-
Notifications
You must be signed in to change notification settings - Fork 553
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
rptest: fix hwm computation in topic_recovery_test #16764
Conversation
We were previously validating the HWM with the assumption that non-data records were exactly 1:1 with the number of archival segments. While such an assumption isn't unreasonable, it isn't very future proof. I ran into a test failure where we the highest_producer_id record included to archival made this condition fail: ``` AssertionError: Unexpected high watermark 5025 (min expected: 5026, max expected: 5029) ``` This commit updates the check be more future-proof, simply validating how HWM is computed: hwm = rp_last_offset + 1 - delta
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.
assumption that non-data records were exactly 1:1 with the number of archival segments. While such an assumption isn't unreasonable
IIUC this assumption isn't reasonable, right? Like there are non-data batches that don't correspond to segment rolls is what we are talking about?
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45425#018dedcf-fa14-4b81-b4a6-2854e179b2ef ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45425#018dedcf-fa10-4966-87c4-87f8480fa7b1 |
More or less, yeah. We were placing an upper bound on the number of non-data batches we could have that was tied to the number of archival |
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.
dope
btw, did this resolve a CI failure or something? |
Yeah, I just opened #16769, which I hit in another PR but didn't see it on GH |
We were previously validating the HWM with the assumption that non-data records were exactly 1:1 with the number of archival segments. While such an assumption isn't unreasonable, it isn't very future proof.
I ran into a test failure where we the highest_producer_id record included to archival made this condition fail:
This commit updates the check be more future-proof, simply validating how HWM is computed:
hwm = rp_last_offset + 1 - delta
Fixes #16769
Backports Required
Release Notes