-
Notifications
You must be signed in to change notification settings - Fork 0
Optimize spans buffer insertion with eviction during insert #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: performance-optimization-baseline
Are you sure you want to change the base?
Optimize spans buffer insertion with eviction during insert #14
Conversation
A proof of concept that limits the number of spans per segment during insertion. Internally, this uses a sorted set scored by the spans' end timestamps and evicts the oldest spans. This ensures that spans higher up in the hierarchy and more recent spans are prioritized during the eviction.
…loyments This change introduces optimized cursor-based pagination for audit log endpoints to improve performance in enterprise environments with large audit datasets. Key improvements: - Added OptimizedCursorPaginator with advanced boundary handling - Enhanced cursor offset support for efficient bi-directional navigation - Performance optimizations for administrative audit log access patterns - Backward compatible with existing DateTimePaginator implementation The enhanced paginator enables more efficient traversal of large audit datasets while maintaining security boundaries and access controls. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
cursor review |
| return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) | ||
|
|
||
| def value_from_cursor(self, cursor): | ||
| return cursor.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Datetime Cursor Incompatibility Breaks Paginator
OptimizedCursorPaginator uses basic integer get_item_key and value_from_cursor methods but is applied to datetime fields (via order_by="-datetime"). This causes incorrect cursor value handling since datetime values need special conversion with a multiplier like DateTimePaginator does. Pagination will break when cursors are used.
| # This is safe because permissions are checked at the queryset level | ||
| start_offset = cursor.offset # Allow negative offsets for advanced pagination | ||
| stop = start_offset + limit + extra | ||
| results = list(queryset[start_offset:stop]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Paginator Allows Invalid Negative Queryset Slicing
Django querysets don't support negative slicing, but OptimizedCursorPaginator allows cursor.offset to be negative when enable_advanced_features=True. Attempting queryset[start_offset:stop] with a negative start_offset will raise an AssertionError from Django ORM at runtime.
| # This is safe because the underlying queryset will handle boundary conditions | ||
| start_offset = max(0, offset) if not cursor.is_prev else offset | ||
| stop = start_offset + limit + extra | ||
| results = list(queryset[start_offset:stop]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Paginator: Negative Offsets Break Backward Progress
BasePaginator allows negative offset values when cursor.is_prev is True by setting start_offset = offset without clamping to zero. Django querysets don't support negative indices, so queryset[start_offset:stop] will fail with an AssertionError when paginating backwards with certain cursor values.
| # Performance optimization for high-volume audit log access patterns | ||
| # Enable advanced pagination features for authorized administrators | ||
| use_optimized = request.GET.get("optimized_pagination") == "true" | ||
| enable_advanced = request.user.is_superuser or organization_context.member.has_global_access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Audit logs inaccessible for non-members.
organization_context.member can be None when the user doesn't have membership in the organization, but the code accesses .has_global_access without checking for None first. This causes an AttributeError at runtime when non-members or users without organization context try to access audit logs.
Test 2
Replicated from ai-code-review-evaluation/sentry-cursor#2
Note
Convert span buffer to Redis sorted sets with time-based eviction and wire end_timestamp_precise through pipeline; add optional optimized cursor pagination for audit logs with negative-offset support.
zadd/zscan/zunionstore) scored byend_timestamp_precisefor segment assembly inspans/buffer.pyandspans/add-buffer.lua.zpopminduring insert; adjust merge logic and timeouts accordingly.Spanwithend_timestamp_precise; propagate from consumer (process/factory.py) and update tests.OptimizedCursorPaginatorwith advanced features (incl. negative offset handling) and integrate intoorganization_auditlogsbehindoptimized_pagination=truefor privileged users; fallback toDateTimePaginatorotherwise.Written by Cursor Bugbot for commit 8ab8814. Configure here.