-
Notifications
You must be signed in to change notification settings - Fork 21
CNDB-15435: Don't purge rows in the replicas with bytes-sized paging #2009
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
Conversation
Checklist before you submit for review
|
f2925cb
to
5f39850
Compare
|
✔️ Build ds-cassandra-pr-gate/PR-2009 approved by ButlerApproved by Butler |
// We only purge rows before sending them to the coordinator if the limit isn't in bytes. | ||
// Otherwise, the replica can send rows with a size lower than the requested page size, | ||
// making the coordinator think that there is no more data and wrongly end paging. | ||
// See CNDB-15435 for further details. |
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.
Does it affect performance?
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.
Indeed it does, both PRs reduce protection against tombstone overwhelming.
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.
Approving because correctness > performance.
However, I don't like the fact it introduces yet another special case (purging works in one case, but not in another) and I agree that a flag-based approach (passing information to the coordinator) would be less fragile.
Closed in favor of #2010 |
Don't purge rows in the replicas when we are using bytes-sized limits. Otherwise, the coordinator can get rows with a size lower than the requested page size and think that there is no more data.