compaction_manager: run_offstrategy_compaction: retrieve owned_ranges from compaction_state #15043
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please elaborate, why must it piggyback cleanup on reshape? |
… from compaction_state perform_offstrategy is called from try_perform_cleanup when there are sstables in the maintenance set that require cleanup. The input sstables are inserted into the compaction_state `sstables_requiring_cleanup` and `try_perform_cleanup` expects offstrategy compaction to clean them up along with reshape compaction. Otherwise, the maintenance sstables that require cleanup are not cleaned up by cleanup compaction, since the reshape output sstable(s) are not analyzed again after reshape compaction, where that would insert the output sstable(s) into `sstables_requiring_cleanup` and trigger their cleanup in the subsequent cleanup compaction. The latter method is viable too, but it is less effficient since we can do reshape+cleanup in one pass, vs. reshape first and cleanup later. Fixes scylladb#15041 Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
bhalevy
force-pushed
the
compaction-offstrategy-piggyback-cleanup
branch
from
81dd87b
to
f9935e7
Compare
Elaborated patch change log, and respective PR description. |
|
Please update the issue too. An issue must describe something that is wrong to deserve being fixed. "Should" just moves the responsibility to figuring out what's wrong to the reader. |
avikivity
approved these changes
Aug 14, 2023
Done |
|
CI state |
|
How did you get CI to succeed? |
Spells and sorcery |
A witch! |
bhalevy
added a commit
to bhalevy/scylla
that referenced
this pull request
Aug 15, 2023
… from compaction_state perform_offstrategy is called from try_perform_cleanup when there are sstables in the maintenance set that require cleanup. The input sstables are inserted into the compaction_state `sstables_requiring_cleanup` and `try_perform_cleanup` expects offstrategy compaction to clean them up along with reshape compaction. Otherwise, the maintenance sstables that require cleanup are not cleaned up by cleanup compaction, since the reshape output sstable(s) are not analyzed again after reshape compaction, where that would insert the output sstable(s) into `sstables_requiring_cleanup` and trigger their cleanup in the subsequent cleanup compaction. The latter method is viable too, but it is less effficient since we can do reshape+cleanup in one pass, vs. reshape first and cleanup later. Fixes scylladb#15041 Signed-off-by: Benny Halevy <bhalevy@scylladb.com> Closes scylladb#15043
bhalevy
added a commit
to bhalevy/scylla
that referenced
this pull request
Aug 31, 2023
… from compaction_state perform_offstrategy is called from try_perform_cleanup when there are sstables in the maintenance set that require cleanup. The input sstables are inserted into the compaction_state `sstables_requiring_cleanup` and `try_perform_cleanup` expects offstrategy compaction to clean them up along with reshape compaction. Otherwise, the maintenance sstables that require cleanup are not cleaned up by cleanup compaction, since the reshape output sstable(s) are not analyzed again after reshape compaction, where that would insert the output sstable(s) into `sstables_requiring_cleanup` and trigger their cleanup in the subsequent cleanup compaction. The latter method is viable too, but it is less effficient since we can do reshape+cleanup in one pass, vs. reshape first and cleanup later. \Fixes scylladb#15041 Signed-off-by: Benny Halevy <bhalevy@scylladb.com> \Closes scylladb#15043 (cherry picked from commit 9f77a32)
bhalevy
added a commit
to bhalevy/scylla
that referenced
this pull request
Sep 19, 2023
… from compaction_state perform_offstrategy is called from try_perform_cleanup when there are sstables in the maintenance set that require cleanup. The input sstables are inserted into the compaction_state `sstables_requiring_cleanup` and `try_perform_cleanup` expects offstrategy compaction to clean them up along with reshape compaction. Otherwise, the maintenance sstables that require cleanup are not cleaned up by cleanup compaction, since the reshape output sstable(s) are not analyzed again after reshape compaction, where that would insert the output sstable(s) into `sstables_requiring_cleanup` and trigger their cleanup in the subsequent cleanup compaction. The latter method is viable too, but it is less effficient since we can do reshape+cleanup in one pass, vs. reshape first and cleanup later. \Fixes scylladb#15041 Signed-off-by: Benny Halevy <bhalevy@scylladb.com> \Closes scylladb#15043 (cherry picked from commit 9f77a32)
bhalevy
added a commit
to bhalevy/scylla
that referenced
this pull request
Nov 13, 2023
… from compaction_state perform_offstrategy is called from try_perform_cleanup when there are sstables in the maintenance set that require cleanup. The input sstables are inserted into the compaction_state `sstables_requiring_cleanup` and `try_perform_cleanup` expects offstrategy compaction to clean them up along with reshape compaction. Otherwise, the maintenance sstables that require cleanup are not cleaned up by cleanup compaction, since the reshape output sstable(s) are not analyzed again after reshape compaction, where that would insert the output sstable(s) into `sstables_requiring_cleanup` and trigger their cleanup in the subsequent cleanup compaction. The latter method is viable too, but it is less effficient since we can do reshape+cleanup in one pass, vs. reshape first and cleanup later. \Fixes scylladb#15041 Signed-off-by: Benny Halevy <bhalevy@scylladb.com> \Closes scylladb#15043 (cherry picked from commit 9f77a32)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
perform_offstrategy is called from try_perform_cleanup
when there are sstables in the maintenance set that require
cleanup.
The input sstables are inserted into the compaction_state
sstables_requiring_cleanupandtry_perform_cleanupexpects offstrategy compaction to clean them up along
with reshape compaction.
Otherwise, the maintenance sstables that require cleanup
are not cleaned up by cleanup compaction, since
the reshape output sstable(s) are not analyzed again
after reshape compaction, where that would insert
the output sstable(s) into
sstables_requiring_cleanupand trigger their cleanup in the subsequent cleanup compaction.
The latter method is viable too, but it is less effficient
since we can do reshape+cleanup in one pass, vs.
reshape first and cleanup later.
Fixes #15041