Great catch! It's been a while since we caught an instance of Pebble not special-casing rangedel sentinel keys, but I'm not surprised there's one more.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jbowens and @sumeerbhola)

internal/manifest/version.go, line 517 at r1 (raw file):

// The returned files are a subsequence of the input files, i.e., the ordering
// is not changed.
func (v *Version) Overlaps(level int, cmp Compare, start, end []byte) LevelSlice {

You're handling the f.Largest.UserKey == start && f.Largest.Trailer = RangeDelSentinel case correctly, but we can do better and also handle the end == RangeDelSentinel && f.Smallest.UserKey == end case. But since end is a user key, you'd have to add an exclusiveEnd bool in the signature and make the relevant signature changes everywhere.

What do you think? I think that'd be a good idea to also fix while we're addressing this, but if you feel otherwise, I'm good with -ing this as-is.

nice!

Reviewed 3 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @itsbilal and @jbowens)

internal/manifest/version.go, line 517 at r1 (raw file):

+1 to handling the end being exclusive case. There seem to be enough callers who have an [InternalKey, InternalKey] and we are losing information by passing in only the UserKey.

testdata/range_del, line 1269 at r1 (raw file):

----
1:
  000008:[a-c]

can this test be changed to print the InternalKey bounds. It will make it easier to see that the test cases are actually correct.

Added the exclusiveEnd parameter. I also added a (*InternalKey).IsExclusiveSentinel helper, because range keys will also have exclusive sentinel key kinds. Some of this logic will need to further change with range keys, for example, levelIter and mergingIter will only care if the largest key that affects points (eg, point key or range del) is a sentinel. I've deferred that for now.

Reviewable status: 0 of 23 files reviewed, 2 unresolved discussions (waiting on @itsbilal and @sumeerbhola)

internal/manifest/version.go, line 517 at r1 (raw file):

Done.

testdata/range_del, line 1269 at r1 (raw file):

Done.

Reviewed 6 of 17 files at r2, 17 of 17 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sumeerbhola)

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sumeerbhola)

Reviewed 4 of 17 files at r2, 16 of 17 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jbowens and @sumeerbhola)

internal/manifest/version.go, line 275 at r3 (raw file):

	if !endIter.iter.valid() {
		endIter.Prev()
	} else if c := cmp(endIter.Current().Smallest.UserKey, end); c > 0 || c == 0 && exclusiveEnd {

nit: we don't need to do the comparison if !exclusiveEnd, so may as well avoid it.

internal/manifest/version_test.go, line 264 at r3 (raw file):

		// Level 2: empty.
		{2, "a", "z", false, ""},
	}

I'm being paranoid here, but curious if coverprofile shows that we've covered all the code paths.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @sumeerbhola)

internal/manifest/version.go, line 275 at r3 (raw file):

I might be misunderstanding, but if !exclusiveEnd the above for loop might've Nexted to a file with Smallest.UserKey > end, and we need the comparison to check for that.

internal/manifest/version_test.go, line 264 at r3 (raw file):

Just checked and it does 👍

Reviewable status: 16 of 23 files reviewed, 3 unresolved discussions (waiting on @itsbilal, @jbowens, and @sumeerbhola)

internal/manifest/version.go, line 275 at r3 (raw file):

I see. Could you add a tiny example in a code comment -- it will make future readers very happy :)

internal/manifest/version_test.go, line 264 at r3 (raw file):

Thanks

TFTRs!

Reviewable status: 15 of 23 files reviewed, 2 unresolved discussions (waiting on @itsbilal, @jbowens, and @sumeerbhola)

internal/manifest/version.go, line 275 at r3 (raw file):

Done!