[v17] Fix large event handling for DynamoDB backend#63562
Merged
kshi36 merged 1 commit intobranch/v17from Feb 9, 2026
Merged
Conversation
kshi36
commented
Feb 5, 2026
|
|
||
| // saveCheckpointAtEvent generates a sub-page checkpoint at the event causing a page break. | ||
| // Subsequent processing will resume from this exact event. | ||
| func (l *eventsFetcher) saveCheckpointAtEvent(e event, oldIterator string) error { |
Contributor
Author
There was a problem hiding this comment.
Revised function saveCheckpointAtEvent to envelope the old sub-page checkpoint logic
kshi36
commented
Feb 5, 2026
Comment on lines
-1478
to
1502
| return out, true, nil | ||
| // We reached the response size limit. | ||
| // Either we reach the fetch limit (l.left == 0) or we have more items to process. | ||
| // For the latter, we must loop one more time to save the sub-page checkpoint at the next event, | ||
| // where we will resume future processing. | ||
| data = trimmedData | ||
| } | ||
| l.totalSize += len(data) | ||
| out = append(out, e) |
Contributor
Author
There was a problem hiding this comment.
Because of the way sub-page checkpoints work in branch/v17 (old sub-pagination solution), we must save at the next event, not the last processed event in out slice. We simply perform one more loop iteration, which will trigger this logic to save at next event:
if l.totalSize+len(data) > events.MaxEventBytesInResponse {
// Encountered an event that would push the total page over the size limit.
// Return all processed events, and the next event will be picked up with the old iterator and saved sub-page checkpoint.
if len(out) > 0 {
if err := l.saveCheckpointAtEvent(e, oldIterator); err != nil {
return nil, false, trace.Wrap(err)
}
return out, true, nil
}
kshi36
commented
Feb 5, 2026
Comment on lines
+861
to
+868
| mockResponses: map[EventKey]mockResponse{ | ||
| {}: { | ||
| events: []apievents.AuditEvent{event1, event2, event3, bigUntrimmableEvent}, | ||
| }, | ||
| }, | ||
| // we don't expect bigUntrimmableEvent because it should go to next batch | ||
| wantEvents: []apievents.AuditEvent{event1, event2, event3}, | ||
| wantKey: keyUntrimmable, |
Contributor
Author
There was a problem hiding this comment.
All tests expect key of the next processed event (the big trimmable/untrimmable events)
tigrato
approved these changes
Feb 6, 2026
bernardjkim
approved these changes
Feb 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport #62899 to branch/v17
Manual backport to use dynamoevents old sub-pagination solution.
changelog: Fixed bug where event handler would get stuck on DynamoDB backend when handling large events