-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
#77762
batcheval: add range tombstone support for DeleteRange
#77762
Conversation
171e3cd
to
0c93f12
Compare
19a02e0
to
430743b
Compare
fc6b90f
to
d2c629b
Compare
b050b4a
to
bb5aa54
Compare
e06f38c
to
a4abc6c
Compare
bb5aa54
to
ba8eb78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is GH description correct regarding which commits belong to this PR? It looks only the last one is relevant.
Some nits as I think I already went through this code before.
ba8eb78
to
34e6ab0
Compare
Ah, it's because I rebased the base branch |
a4abc6c
to
5d5aecb
Compare
34e6ab0
to
4798ae2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 20 of 20 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @aliher1911 and @erikgrinaker)
pkg/storage/testdata/mvcc_histories/range_tombstone_mutations, line 1 at r4 (raw file):
# Set up some point keys, point/range tombstones, and intents.
nit: diagram would be useful here.
2097ac5
to
3ad25e4
Compare
5d5aecb
to
53f4f4c
Compare
cf525db
to
413b5bc
Compare
cb77b2e
to
49aacac
Compare
413b5bc
to
bc45db8
Compare
49aacac
to
e5bd560
Compare
bc45db8
to
f7fea06
Compare
f7fea06
to
6c3aa50
Compare
441ea16
to
8410a44
Compare
@aliher1911 May want to take a quick look at this again, since there's been a few changes to range keys since it was initially submitted -- nothing major though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave it another read and it looks good.
There's something wrong with the request itself though. I can see latest version with all updates in "Files changed", but neither VS-Code view, nor Reviewable shows it. Both show diff with some older commit which rolls back changes from master massively.
Thanks!
Are you sure you're using the correct base though? Remember, this used to be onto the Using "Show full diff" in Reviewable shows the correct changes, as does comparing the branch to |
8410a44
to
363ce8d
Compare
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. Release note: None
363ce8d
to
3d71082
Compare
TFTR! bors r=aliher1911 |
Build succeeded: |
This patch adds the parameter
UseExperimentalRangeTombstone
forDeleteRange
, which deletes the span using an MVCC range tombstone.The new version gate
MVCCRangeTombstones
must be checked before usingit.
storage.ExperimentalMVCCDeleteRangeUsingTombstone()
is added tocarry 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