Reviewed 14 of 14 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jbowens)

compaction.go line 366 at r1 (raw file):

	// Referenced by `compactionIter` which uses it to check whether keys are deleted.
	rangeDelFrag keyspan.Fragmenter
	// The range deletion tombstone interleaving iterator that interleaves range

Nit: probably worth mentioning here that this iterator is the outermost returned iterator if there are any rangedels.

compaction_iter.go line 458 at r1 (raw file):

	i.iterKey, i.iterValue = i.iter.Next()
	// We should never see an exclusive sentinel in the compaction input.
	if i.iterKey != nil && i.iterKey.IsExclusiveSentinel() {

If I understand this correctly, we had to remove this because every key will appear like an exclusive sentinel, correct?

Reviewed 14 of 14 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @jbowens)

compaction_iter.go line 495 at r1 (raw file):

	switch key.Kind() {
	case InternalKeyKindRangeDelete:
		// Range tombstones are always interleaved at the infinite sequence

This didn't immediately sink in. Could the first sentence end with something like ", and therefore a range tombstone (if present) will be the first key encountered.". This would underscore the fact that we should never see a range del in the switch statement.

testdata/compaction_iter line 476 at r2 (raw file):

c-d#3
d-e#2
e-f#1

The last fragment is gone? What's the explanation there?

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @itsbilal, @jbowens, and @nicktrav)

compaction_iter.go line 458 at r1 (raw file):

Yeah, that's right.

testdata/compaction_iter line 476 at r2 (raw file):

We don't get as far through the input tombstones with the same number of nexts, a consequence of the MergingIter fragmenting before the range deletions are interleaved. In the old test file, we encountered all three unfragmented tombstones a-d, b-e and c-f. Now we encounter the fragments a-b, b-c, c-d, d-e. A subsequent next would surface e-f.

Reviewed 4 of 5 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @itsbilal and @jbowens)

testdata/compaction_iter line 476 at r2 (raw file):

Ah. So those fragments aren't in the fragmenter yet as they haven't been interleaved. And given that tombstones flushes the current state of the fragmenter, we don't see it. 👍

What do you think about either having one test case that "nexts" through all fragments for completeness, or updating the test cases where we removed e-f in this PR to add an additional "next" operation so that e-f is emitted? Or just a comment. No strong opinion.

Reviewable status: 9 of 16 files reviewed, 1 unresolved discussion (waiting on @itsbilal and @nicktrav)

testdata/compaction_iter line 476 at r2 (raw file):

Updated to emit e-f in this instance. I want to preserve a combination of cases that exhaust input tombstones and case that do not.

Did another pass of the latest revs. 👍

Reviewed 7 of 7 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itsbilal and @sumeerbhola)