-
Notifications
You must be signed in to change notification settings - Fork 597
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
storage
: reset compaction window start offset when offset map isn't built
#24323
storage
: reset compaction window start offset when offset map isn't built
#24323
Conversation
… built Consider the following situation: [0] [1] [2] [3] [4] [5] ^ |----- _last_compaction_window_start_offset In the event that segment [2] cannot have its keys completely added to the offset map due to memory constraints, if we do not reset the window start offset, compaction will become somewhat stuck as it repeatedly tries to index segment [2] in future compaction rounds. By resetting the window's value in this situation, we can allow compaction to continue to make forward progression.
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/58828#01936a7e-ff7b-4c6d-8aa3-93d21eec7fd6 |
This may be superseded by #24367 |
/backport v24.3.x |
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.
nice.
By resetting the window's value in this situation, we can allow
compaction to continue to make forward progression.
how did you find this issue? did you see it happen in practice/test system?
segment_set segs(std::move(buf)); | ||
if (segs.empty()) { | ||
return segs; | ||
} | ||
|
||
segment_set segs(std::move(buf)); |
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.
😃
The idea struck me when I was on a walk actually 🚶 The test case came next to confirm the theory, then the fix |
The behavior of the sliding window considered for compaction was changed in this PR: #23380.
However, consider the following situation:
In the event that segment [2] cannot have its keys completely added to the offset map due to memory constraints, if we do not reset the window start offset, compaction will become somewhat stuck as it repeatedly tries to index segment [2] in future compaction rounds.
By resetting the window's value in this situation, we can allow compaction to continue to make forward progression.
Backports Required
Release Notes
Bug Fixes