kvserver: merge undersized ranges into adequately-sized neighbors - #174808
Open
NAVEENKUMARKR777 wants to merge 1 commit into
Open
kvserver: merge undersized ranges into adequately-sized neighbors#174808NAVEENKUMARKR777 wants to merge 1 commit into
NAVEENKUMARKR777 wants to merge 1 commit into
Conversation
The merge queue previously required both the LHS and RHS of a candidate merge to be individually below the RangeMinBytes threshold before merging them. This meant that a range wedged between two neighbors that were each at or above the threshold could never merge with either of them and would persist indefinitely, even if it held zero or near-zero bytes (eg, after all of its rows were deleted). Change the merge queue to merge whenever either side is below the threshold, as long as the combined range would stay below twice the threshold. This preserves the existing anti-thrashing behavior (an undersized range is never merged into a neighbor that's already close to its max size, since the split queue already guards against producing an over-max-size range, and the doubled-threshold check avoids producing a range that's likely to need re-splitting soon) while letting genuinely small ranges merge away instead of persisting forever. This was discussed at length in the issue below without a fix ever landing; this commit implements the minimal, backportable version of the fix that commenters converged on. Resolves: cockroachdb#100443 Epic: none Release note (bug fix): Fixed a bug where a range could become permanently unmergeable if its immediate neighbor's size was at or above the configured minimum range size, even if the range itself was empty or nearly empty. The range merge queue now merges such ranges as long as the resulting merged range would stay comfortably below the minimum size threshold. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This file contains hidden or 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
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.
Summary
The merge queue previously required both the LHS and RHS of a candidate
merge to be individually below the
RangeMinBytesthreshold before mergingthem (see
mergeQueue.processinmerge_queue.go). This meant a rangewedged between two neighbors that were each at or above the threshold could
never merge with either of them and would persist indefinitely — even if it
held zero or near-zero bytes, e.g. after all of its rows were deleted.
This was discussed at length in #100443 (kvoli, andrewbaptist, erikgrinaker),
including simulations comparing a few different fix strategies, but no fix
ever landed. This PR implements the minimal, backportable fix that the
discussion converged on: merge whenever either side is below the
threshold, as long as the combined range stays below twice the threshold.
This preserves the existing anti-thrashing behavior — an undersized range is
never merged into a neighbor that's already close to its max size, since
shouldSplitRangealready guards against producing an over-max-size range,and the doubled-threshold check avoids producing a range that's likely to
need re-splitting soon — while letting genuinely small ranges merge away
instead of persisting forever.
Test plan
Added two new subtests to
TestMergeQueuecovering both the newly-allowedcase (RHS above threshold, but combined size stays well under 2x the
threshold) and the still-disallowed case (RHS above threshold, combined size
would not stay comfortably below 2x). Ran the full
TestMergeQueuesuite(including all pre-existing subtests) to confirm no regressions.
Resolves: #100443
Epic: none
Release note (bug fix): Fixed a bug where a range could become permanently
unmergeable if its immediate neighbor's size was at or above the configured
minimum range size, even if the range itself was empty or nearly empty. The
range merge queue now merges such ranges as long as the resulting merged
range would stay comfortably below the minimum size threshold.
🤖 Generated with Claude Code