[release-21.2] kvserver: Clean up empty range directories after snapshots #88142
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
Previously, we were creating subdirectories for ranges and range snapshots in the auxiliary directory every time we accepted a snapshot, but only cleaning up the snapshot subdirectories after a snapshot scratch space closed. This left empty parent range directories around on the FS, slowing down future calls to Pebble.Capacity() and indirectly slowing down AddSSTable in the future. This change adds code to clean up empty range directories in the aux directory if they're not being used. Some coordination and synchronization code had to be added to ensure we wouldn't remove a directory that was just created by a concurrent snapshot. Fixes cockroachdb#83137. Release note (bug fix, performance improvement): Addresses issue where imports and rebalances were being slowed down due to the accumulation of empty directories from range snapshot applications.
Previously, in rare cases where we had two SSTSnapshotStorageScratch for a given range, we'd have not incremented the refcount for the second scratch creation, leading to an inconsistent refcount panic down the line. This change fixes it and adds a test to exercise the concurrent case. Bug popped up in cockroachdb#84100. Release note: None.
|
This change is |
nicktrav
approved these changes
Sep 19, 2022
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @nicktrav)
nicktrav
reviewed
Sep 19, 2022
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status:
|
TFTR! |
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.
Backport of #84100
Previously, we were creating subdirectories for ranges and
range snapshots in the auxiliary directory every time we
accepted a snapshot, but only cleaning up the snapshot
subdirectories after a snapshot scratch space closed. This
left empty parent range directories around on the FS,
slowing down future calls to Pebble.Capacity() and indirectly
slowing down AddSSTable in the future.
This change adds code to clean up empty range directories
in the aux directory if they're not being used. Some coordination
and synchronization code had to be added to ensure we wouldn't
remove a directory that was just created by a concurrent snapshot.
Fixes #83137.
Release note (bug fix, performance improvement): Addresses issue where
imports and rebalances were being slowed down due to the accumulation
of empty directories from range snapshot applications.