storage: minor iterator tweaks#79744
storage: minor iterator tweaks#79744erikgrinaker merged 2 commits intocockroachdb:mvcc-range-tombstonesfrom
Conversation
|
This change is |
d63e143 to
53aa367
Compare
768f563 to
4178a11
Compare
53aa367 to
004eab2
Compare
Release note: None
Release note: None
4178a11 to
19748e3
Compare
nicktrav
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @aliher1911 and @jbowens)
|
TFTR! |
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/storage/pebble_iterator.go, line 41 at r2 (raw file):
// use two slices for each of the bounds since this caller should not change // the slice holding the current bounds, that the callee (pebble.MVCCIterator) // is currently using, until after the caller has made the SetBounds call.
Why is the removal of this 2 slice scheme correct? I assumed this meant we are making a copy somewhere in the Iterator.SetOptions code path but I can't spot it in the Pebble code.
erikgrinaker
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/storage/pebble_iterator.go, line 41 at r2 (raw file):
Previously, sumeerbhola wrote…
Why is the removal of this 2 slice scheme correct? I assumed this meant we are making a copy somewhere in the Iterator.SetOptions code path but I can't spot it in the Pebble code.
Good catch, thanks. I don't think I fully grasped the need for this, and tests passed without it. Reintroduced in #79993.
This is for the
mvcc-range-tombstonesbranch.These are minor fixes/tweaks following #79291, which will be squashed into the respective commits on
mvcc-range-tombstones. Submitting a PR just to keep the history.storage: fix batch reads with multiple concurrent iterators
Release note: None
storage: minor code cleanups
Release note: None