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: refine start offset after downscaling #1185

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Conversation

pvlugter
Copy link
Contributor

Refining the selected timestamp offset after projection scaling. Before if it saw there were multiple projection keys, it would use the earliest of the latest by slice. But if just one of the slices has not had recent activity and is far behind, we go back to that timestamp offset on each projection scaling or restart, replaying many events.

Update this to use the earliest of the latest grouped by projection key (by slice range), to still handle the case where a previous projection instance may have been falling behind. Also recheck that there are multiple projection keys, within the latest offsets by slice.

Tested and this improves things a lot. But there can still be cases where a particular slice is far behind, and with projection scaling, happened to be processed by a projection instance for a different key. Then it will still continue to go back to that timestamp offset, until that slice moves forward. Would need something else to address this — maybe updating that offset to the current projection key, so it will eventually all be for one projection instance and not need this special case of using the earliest.

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

@pvlugter
Copy link
Contributor Author

Not sure there's a straightforward way to solve the issue with one of the slice and projection key combinations being far behind, because that slice hasn't been updated, and there are no newer latest offsets for that particular projection key. Only idea currently is to have the new projection instance "adopt" that timestamp offset, so that it doesn't repeat on subsequent restarts. Maybe something to come back to. The current fix covers the more usual downscaling cases.

@pvlugter pvlugter marked this pull request as ready for review August 29, 2024 07:34
@pvlugter
Copy link
Contributor Author

Will merge as is and we can possibly revisit later.

@pvlugter pvlugter merged commit d57d2c2 into main Aug 29, 2024
22 checks passed
@pvlugter pvlugter deleted the downscaling-earliest branch August 29, 2024 07:35
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.

2 participants