batcheval: add MVCC-compliant RevertRange variant#76478
Merged
craig[bot] merged 3 commits intocockroachdb:masterfrom Feb 22, 2022
Merged
batcheval: add MVCC-compliant RevertRange variant#76478craig[bot] merged 3 commits intocockroachdb:masterfrom
RevertRange variant#76478craig[bot] merged 3 commits intocockroachdb:masterfrom
Conversation
Member
|
This change is |
2c982e7 to
61df93e
Compare
dt
reviewed
Feb 16, 2022
Contributor
dt
left a comment
There was a problem hiding this comment.
Just one API question from me
61df93e to
7f437df
Compare
aliher1911
reviewed
Feb 17, 2022
Contributor
aliher1911
left a comment
There was a problem hiding this comment.
Mostly nits about comments, but I think there's some room for improvement in ExperimentalMVCCRevertRange which we should address before in becomes non-experimental.
612dc9a to
093180b
Compare
dt
approved these changes
Feb 18, 2022
Contributor
dt
left a comment
There was a problem hiding this comment.
API as it'll be available to bulk/sql clients LGTM!
093180b to
d22650c
Compare
Since `RevertRange` mutates MVCC history, we want them to be alone in a batch. The DistSender will split any batches that have multiple such requests. Release note: None
This patch adds a method `NextKeyIgnoringTime()` for `MVCCIncrementalIterator`. This can be used to find the next key (as opposed to version) of the iterator, ignoring the time bounds. It's similar to `NextIgnoringTime()`, but calls `NextKey()` instead of `Next()` on the underlying iterator. Release note: None
This adds a new parameter `ExperimentalPreserveHistory` to `RevertRange` which, rather than clearing keys above the target time, will write new values or tombstones that reflect the state at the target time. For long runs of deleted keys, this will instead drop an MVCC range tombstone. This makes the command respect e.g. MVCC immutability, the closed timestamp, and timestamp cache. Note that MVCC range tombstones are currently experimental, and as such this parameter is also experimental. Callers must call `storage.CanUseExperimentalMVCCRangeTombstones()` before using it. Release note: None
d22650c to
e1acd75
Compare
Contributor
Author
|
TFTRs! bors r=aliher1911,dt |
Contributor
|
Build failed: |
Contributor
Author
|
Some weird agent failure for bors retry |
Contributor
|
Build failed: |
Contributor
Author
|
Unrelated flake. Third time's the charm! bors retry |
Contributor
|
Build succeeded: |
craig bot
pushed a commit
that referenced
this pull request
Feb 23, 2022
76921: storage: revert experimental MVCC range tombstones r=aliher1911 a=erikgrinaker This reverts most of #76131, #76203, and #76478 -- except minor changes that were unrelated to the range tombstones themselves. This leaves a gap for cluster version `Internal:78` -- I think that's probably fine, but I've left a comment. Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
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
4 participants
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.
roachpb: add
isAloneforRevertRangeRequestSince
RevertRangemutates MVCC history, we want them to be alone in abatch. The DistSender will split any batches that have multiple such
requests.
Release note: None
storage: add
NextKeyIgnoringTime()forMVCCIncrementalIteratorThis patch adds a method
NextKeyIgnoringTime()forMVCCIncrementalIterator. This can be used to find the next key (asopposed to version) of the iterator, ignoring the time bounds. It's
similar to
NextIgnoringTime(), but callsNextKey()instead ofNext()on the underlying iterator.Release note: None
batcheval: add MVCC-compliant
RevertRangevariantThis adds a new parameter
ExperimentalPreserveHistorywhich, ratherthan clearing keys above the target time, will write new values or
tombstones that reflect the state at the target time. For long runs of
new keys, this will instead drop an MVCC range tombstone. This makes the
command respect e.g. MVCC immutability, the closed timestamp, and
timestamp cache.
Note that MVCC range tombstones are currently experimental, and as such
this parameter is also experimental. Callers must call
storage.CanUseExperimentalMVCCRangeTombstones()before using it.Resolves #70416.
Release note: None