storage: add MVCCStats for range keys
#76860
Closed
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
|
This change is |
erikgrinaker
force-pushed
the
mvccstats-range-tombstones
branch
from
1494d12 to
1fbb393
Compare
This patch adds an iteration key type `IterKeyTypePointsWithRanges` for `MVCCIterator` which iterates only over point keys, but surfaces range keys overlapping those points. Release note: None
This patch automatically clears all range keys (at any timestamp) in the `Engine` methods `ClearMVCCRangeAndIntents` and `ClearIterRange`. Range keys are not affected by `ClearRawRange` (which clears raw engine keys) and `ClearMVCCRange` (which is intended for clearing a subset of versions for a single key). This is implemented via a new `Engine.ExperimentalClearMVCCRangeKeys` method which calls through to Pebble's `RangeKeyDelete`, efficiently deleting all range keys in a key span. By extension, the `ClearRange` RPC method now also clears range keys, both when using point deletions and range deletions. Release note: None
This patch adds basic support for MVCC range tombstones in garbage collection. It garbage collects points below a range tombstone, as well as the range tombstones themselves. However, `MVCCStats` do not currently take range tombstones into account for garbage statistics -- this will be addressed separately. Garbage collection below range tombstones does not do anything fancy like dropping a Pebble range tombstone when there are no newer versions above the range tombstone -- it still uses point clears for every GCed key. This can be optimized later. Release note: None
This adds a parameter `ExperimentalRanges` to `GCRequest`, and a corresponding `ExperimentalGCRanges` version gate, which allows GCing large swathes of keys using Pebble range tombstones. This parameter is not yet in use, but is added preemptively to allow garbage collection to make use of it in the future when GCing below an MVCC range tombstone with no keys above it. Since MVCC range tombstones are experimental, this can possibly be added in a 22.1 patch release. Release note: None
This patch split out `MVCCRangeKeyDefragmenter` from `MVCCRangeKeyIterator`. Keeping defragmentation separate from the iterator allows for ad hoc range key defragmentation in other contexts, e.g. during combined point/range key iteration. Release note: None
This patch adds `MVCCStats` tracking for range keys. It only considers range tombstones for now, since the semantics of other range keys are still unclear (e.g. how should overlapping range keys be interpreted?), and will error if encountering non-tombstone range keys. Two new fields are added to `MVCCStats`: * `RangeKeyCount`: the number of individual defragmented logical range keys, regardless of any overlap with other range keys. Multiple versions count as separate range keys, even if they overlap exactly. * `RangeKeyBytes`: the logical encoded byte size of all range keys, excluding value. This ignores fragmentation, and counts the key bounds separately for each version even if multiple range keys overlap exactly. Unlike point keys, which for historical reasons use a fixed-size timestamp contribution, this uses the actual variable-length timestamp encoding contribution. `ComputeStatsForRange()` has been extended to calculate the above quantities, and additionally account for range tombstones themselves in `GCBytesAge` along with their effect on point keys. However, these statistics are not yet updated during range key mutations and GC, nor on CRDB range splits and merges -- this will be addressed separately. All relevant call sites have been updated to surface range keys for the MVCC iterators passed to `ComputeStatsForRange()`. Release note: None
erikgrinaker
force-pushed
the
mvccstats-range-tombstones
branch
from
1fbb393 to
d58c6b4
Compare
sumeerbhola
requested changes
Feb 22, 2022
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aliher1911, @erikgrinaker, and @jbowens)
pkg/storage/enginepb/mvcc.proto, line 209 at r6 (raw file):
// range key (assuming they have the same value). However, a range key that // straddles a range split boundary will become two separate logical range // keys (one in each range), and merge back to one when the ranges merge.
(drive by comment)
We currently sum the stats on a merge, as you are well aware.
I didn't spot the change in cmd_end_transaction.go to recompute these.
What did I miss?
erikgrinaker
commented
Feb 22, 2022
There was a problem hiding this comment.
Reviewable status:
pkg/storage/enginepb/mvcc.proto, line 209 at r6 (raw file):
Previously, sumeerbhola wrote…
(drive by comment)
We currently sum the stats on a merge, as you are well aware.
I didn't spot the change in cmd_end_transaction.go to recompute these.
What did I miss?
Correct. These stats are not updated anywhere, including on range splits/merges or range key mutations -- this PR only implements the new stats quantities and accounts for them in ComputeStatsForRange() as a first step. Updates elsewhere will be addressed separately. For range splits/merges, that will require scanning range keys that straddle the split point and split/merge them accordingly.
|
Closed in favor of #78085. |
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.
Only the last 2 commits should be reviewed here, the others are from #76783.
storage: split out
MVCCRangeKeyDefragmenterThis patch split out
MVCCRangeKeyDefragmenterfromMVCCRangeKeyIterator. Keeping defragmentation separate from theiterator allows for ad hoc range key defragmentation in other contexts,
e.g. during combined point/range key iteration.
Release note: None
storage: add
MVCCStatsfor range keysThis patch adds
MVCCStatstracking for range keys. It only considersrange tombstones for now, since the semantics of other range keys are
still unclear (e.g. how should overlapping range keys be interpreted?),
and will error if encountering non-tombstone range keys.
Two new fields are added to
MVCCStats:RangeKeyCount: the number of individual defragmented logical rangekeys, regardless of any overlap with other range keys. Multiple versions
count as separate range keys, even if they overlap exactly.
RangeKeyBytes: the logical encoded byte size of all range keys,excluding value. This ignores fragmentation, and counts the key bounds
separately for each version even if multiple range keys overlap exactly.
Unlike point keys, which for historical reasons use a fixed-size
timestamp contribution, this uses the actual variable-length timestamp
encoding contribution.
ComputeStatsForRange()has been extended to calculate the abovequantities, and additionally account for range tombstones themselves in
GCBytesAgealong with their effect on point keys. However, thesestatistics are not yet updated during range key mutations and GC, nor on
CRDB range splits and merges -- this will be addressed separately. All
relevant call sites have been updated to surface range keys for the MVCC
iterators passed to
ComputeStatsForRange().Touches #70412.
Release note: None