Skip to content
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

fix: [DynamoDB] Ensure that inflight persistence IDs are not evicted #1269

Merged
merged 10 commits into from
Nov 26, 2024

Conversation

leviramsey
Copy link
Contributor

If, when recovering a backlog in an at-least-once projection, the range of event timestamps from a slice in a given batch of records to have offsets saved exceeds the offset store's time-window, the older events will be immediately evicted; since the inflight records are cleaned based on the state after evicting, this implies that the records for those older events will continue to be considered inflight.

If there are no further events for a persistence ID that was immediately evicted, this means a leak in inflight (if there are enough of these situations, the hard limit of 10k records inflight will get breached) eventually resulting in projection failure.

Note that sequence numbers are saved based on the incoming records, so this eviction will not prevent seen sequence numbers from being updated.

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, makes sense that we shouldn't evict those that are in fligt

but there is a change in "use last record for each pid" that I don't see why it's needed?

@patriknw
Copy link
Member

I have incorporated this in #1255 for r2dbc, commit 22c0c26

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@pvlugter pvlugter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Co-authored-by: Peter Vlugter <[email protected]>
@patriknw patriknw added this to the 1.6.4 milestone Nov 26, 2024
@patriknw patriknw merged commit a639151 into akka:main Nov 26, 2024
21 of 22 checks passed
@leviramsey leviramsey deleted the cleanup-fix branch November 28, 2024 16:07
@leviramsey
Copy link
Contributor Author

Refs: akka/akka-persistence-dynamodb#95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants