Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

batcheval: add range tombstone support for DeleteRange #76203

Merged
merged 1 commit into from
Feb 21, 2022

Conversation

erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Feb 7, 2022

This patch adds a parameter UseExperimentalRangeTombstone for
DeleteRange, which deletes the span using an MVCC range tombstone.
This must only be called after checking
storage.CanUseExperimentalMVCCRangeTombstones(), which ensures the
ExperimentalMVCCRangeTombstones version gate and
COCKROACH_EXPERIMENTAL_MVCC_RANGE_TOMBSTONES environment variable are
set.

Did I mention that MVCC range tombstones are experimental? They are
currently under active development, and are not respected by the KV or
MVCC APIs, nor are they persisted. This patch simply sets up the
plumbing for it.

Resolves #70415.
Touches #70427.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@erikgrinaker
Copy link
Contributor Author

Some comments on the first pass while I'm trying to get into iteration logic.

Do you mean the iteration logic in the first commit? Please review that on the corresponding PR (#76131).

Copy link
Contributor

@aliher1911 aliher1911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to other PR

pkg/storage/mvcc_key.go Outdated Show resolved Hide resolved
pkg/storage/mvcc_key_test.go Outdated Show resolved Hide resolved
pkg/storage/mvcc_range_tombstone_iterator.go Outdated Show resolved Hide resolved
pkg/storage/engine.go Outdated Show resolved Hide resolved
pkg/storage/engine.go Outdated Show resolved Hide resolved
@aliher1911 aliher1911 self-requested a review February 8, 2022 16:52
@aliher1911
Copy link
Contributor

LGTM!

@erikgrinaker erikgrinaker force-pushed the delete-range-tombstone branch 2 times, most recently from b8e42d9 to 0ad9646 Compare February 21, 2022 19:25
@erikgrinaker
Copy link
Contributor Author

CI failure appears unrelated, Bazel build is still green. TFTRs!

bors r=aliher1911,dt

@craig
Copy link
Contributor

craig bot commented Feb 21, 2022

Merge conflict.

@erikgrinaker
Copy link
Contributor Author

Whoops, conflict with last master merge.

bors r-

This patch adds a parameter `UseExperimentalRangeTombstone` for
`DeleteRange`, which deletes the span using an MVCC range tombstone.
This must only be called after checking
`storage.CanUseExperimentalMVCCRangeTombstones()`, which ensures the
`ExperimentalMVCCRangeTombstones` version gate and
`COCKROACH_EXPERIMENTAL_MVCC_RANGE_TOMBSTONES` environment variable are
set.

Did I mention that MVCC range tombstones are experimental? They are
currently under active development, and are not respected by the KV or
MVCC APIs, nor are they persisted. This patch simply sets up the
plumbing for it.

Release note: None
@erikgrinaker erikgrinaker force-pushed the delete-range-tombstone branch from 0ad9646 to f07cfd6 Compare February 21, 2022 20:35
@erikgrinaker
Copy link
Contributor Author

Trivial merge conflict, going to yolo this.

bors r=aliher1911,dt

@craig
Copy link
Contributor

craig bot commented Feb 21, 2022

Build succeeded:

@craig craig bot merged commit 9c0d08d into cockroachdb:master Feb 21, 2022
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 <[email protected]>
@erikgrinaker erikgrinaker deleted the delete-range-tombstone branch May 28, 2022 17:10
craig bot pushed a commit that referenced this pull request Jun 9, 2022
77762: batcheval: add range tombstone support for `DeleteRange` r=aliher1911 a=erikgrinaker

This patch adds the parameter `UseExperimentalRangeTombstone` for
`DeleteRange`, which deletes the span using an MVCC range tombstone.
The new version gate `MVCCRangeTombstones` must be checked before using
it. `storage.ExperimentalMVCCDeleteRangeUsingTombstone()` is added to
carry out the actual deletion.

This is a bare-bones implementation to allow writing range keys via the
KV API for testing and development purposes. It has significant
shortcomings, and will be fleshed out at a later time.

Touches #70415.
Replaces #76203.

Release note: None

Co-authored-by: Erik Grinaker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: use MVCC range tombstones in DeleteRange
4 participants