db: use InterleavingIter to interleave range deletions in compactions

Remove the keyspan.InternalIteratorShim that was intended to be a temporary
bridge to allow range deletions to be surfaced within the compaction iterator,
replacing it with the keyspan.InterleavingIter. This mirrors the mechanics of
range keys. Follow up work will be able to simplify the compaction iterator
logic now that range deletions are always interleaved at the maximal sequence
number.

Informs cockroachdb#3082.

compaction.go

@@ -628,9 +628,7 @@ type compaction struct {
 	// The range deletion tombstone iterator, that merges and fragments
 	// tombstones across levels. This iterator is included within the compaction
 	// input iterator as a single level.
-	// TODO(jackson): Remove this when the refactor of FragmentIterator,
-	// InterleavingIterator, etc is complete.
-	rangeDelIter keyspan.InternalIteratorShim
+	rangeDelInterleaving keyspan.InterleavingIter
 	// rangeKeyInterleaving is the interleaving iter for range keys.
 	rangeKeyInterleaving keyspan.InterleavingIter
 
@@ -1377,36 +1375,34 @@ func (c *compaction) newInputIter(
 		}
 	}
 
+	// If there's only one constituent point iterator, we can avoid the overhead
+	// of a *mergingIter. This is possible, for example, when performing a flush
+	// of a single memtable. Otherwise, combine all the iterators into a merging
+	// iter.
+	iter := iters[0]
+	if len(iters) > 1 {
+		iter = newMergingIter(c.logger, &c.stats, c.cmp, nil, iters...)
+	}
+
 	// In normal operation, levelIter iterates over the point operations in a
 	// level, and initializes a rangeDelIter pointer for the range deletions in
 	// each table. During compaction, we want to iterate over the merged view of
 	// point operations and range deletions. In order to do this we create one
 	// levelIter per level to iterate over the point operations, and collect up
 	// all the range deletion files.
 	//
-	// The range deletion levels are first combined with a keyspan.MergingIter
-	// (currently wrapped by a keyspan.InternalIteratorShim to satisfy the
-	// internal iterator interface). The resulting merged rangedel iterator is
-	// then included with the point levels in a single mergingIter.
-	//
-	// Combine all the rangedel iterators using a keyspan.MergingIterator and a
-	// InternalIteratorShim so that the range deletions may be interleaved in
-	// the compaction input.
-	// TODO(jackson): Replace the InternalIteratorShim with an interleaving
-	// iterator.
+	// The range deletion levels are combined with a keyspan.MergingIter and
+	// defragmented by a keyspan.DefragmentingIter. The resulting merged
+	// rangedel iterator is then included using an InterleavingIter.
 	if len(rangeDelIters) > 0 {
-		c.rangeDelIter.Init(c.cmp, rangeDelIters...)
-		iters = append(iters, &c.rangeDelIter)
+		mi := &keyspan.MergingIter{}
+		mi.Init(c.cmp, keyspan.NoopTransform, new(keyspan.MergingBuffers), rangeDelIters...)
+		di := &keyspan.DefragmentingIter{}
+		di.Init(c.comparer, mi, keyspan.DefragmentInternal, keyspan.StaticDefragmentReducer, new(keyspan.DefragmentingBuffers))
+		c.rangeDelInterleaving.Init(c.comparer, iter, di, keyspan.InterleavingIterOpts{})
+		iter = &c.rangeDelInterleaving
 	}
 
-	// If there's only one constituent point iterator, we can avoid the overhead
-	// of a *mergingIter. This is possible, for example, when performing a flush
-	// of a single memtable. Otherwise, combine all the iterators into a merging
-	// iter.
-	iter := iters[0]
-	if len(iters) > 0 {
-		iter = newMergingIter(c.logger, &c.stats, c.cmp, nil, iters...)
-	}
 	// If there are range key iterators, we need to combine them using
 	// keyspan.MergingIter, and then interleave them among the points.
 	if len(rangeKeyIters) > 0 {
@@ -3401,7 +3397,7 @@ func (d *DB) runCompaction(
 				// The interleaved range deletion might only be one of many with
 				// these bounds. Some fragmenting is performed ahead of time by
 				// keyspan.MergingIter.
-				if s := c.rangeDelIter.Span(); !s.Empty() {
+				if s := c.rangeDelInterleaving.Span(); !s.Empty() {
 					// The memory management here is subtle. Range deletions
 					// blocks do NOT use prefix compression, which ensures that
 					// range deletion spans' memory is available as long we keep

error_test.go

@@ -198,7 +198,7 @@ func TestRequireReadError(t *testing.T) {
 		require.NoError(t, d.Flush())
 		expectLSM(`
 0.0:
-  000007:[a1#13,SETWITHDEL-a2#inf,RANGEDEL]
+  000007:[a1#13,SET-a2#inf,RANGEDEL]
 6:
   000005:[a1#10,SET-a2#11,SET]
 `, d, t)
@@ -292,7 +292,7 @@ func TestCorruptReadError(t *testing.T) {
 		require.NoError(t, d.Flush())
 		expectLSM(`
 0.0:
-  000007:[a1#13,SETWITHDEL-a2#inf,RANGEDEL]
+  000007:[a1#13,SET-a2#inf,RANGEDEL]
 6:
   000005:[a1#10,SET-a2#11,SET]
 `, d, t)

internal/keyspan/defragment_test.go

@@ -80,7 +80,7 @@ func TestDefragmentingIter(t *testing.T) {
 				}
 			}
 			var miter MergingIter
-			miter.Init(cmp, noopTransform, new(MergingBuffers), NewIter(cmp, spans))
+			miter.Init(cmp, NoopTransform, new(MergingBuffers), NewIter(cmp, spans))
 			innerIter := attachProbes(&miter, probeContext{log: &buf}, probes...)
 			var iter DefragmentingIter
 			iter.Init(comparer, innerIter, equal, reducer, new(DefragmentingBuffers))
@@ -164,9 +164,9 @@ func testDefragmentingIteRandomizedOnce(t *testing.T, seed int64) {
 	fragmented = fragment(cmp, formatKey, fragmented)
 
 	var originalInner MergingIter
-	originalInner.Init(cmp, noopTransform, new(MergingBuffers), NewIter(cmp, original))
+	originalInner.Init(cmp, NoopTransform, new(MergingBuffers), NewIter(cmp, original))
 	var fragmentedInner MergingIter
-	fragmentedInner.Init(cmp, noopTransform, new(MergingBuffers), NewIter(cmp, fragmented))
+	fragmentedInner.Init(cmp, NoopTransform, new(MergingBuffers), NewIter(cmp, fragmented))
 
 	var referenceIter, fragmentedIter DefragmentingIter
 	referenceIter.Init(comparer, &originalInner, DefragmentInternal, StaticDefragmentReducer, new(DefragmentingBuffers))

internal/keyspan/interleaving_iter_test.go

@@ -108,7 +108,7 @@ func runInterleavingIterTest(t *testing.T, filename string) {
 			for _, line := range lines {
 				spans = append(spans, ParseSpan(line))
 			}
-			keyspanIter.Init(cmp, noopTransform, new(MergingBuffers), NewIter(cmp, spans))
+			keyspanIter.Init(cmp, NoopTransform, new(MergingBuffers), NewIter(cmp, spans))
 			hooks.maskSuffix = nil
 			iter.Init(testkeys.Comparer, &pointIter, &keyspanIter,
 				InterleavingIterOpts{Mask: &hooks})

internal/keyspan/transformer.go

@@ -24,7 +24,8 @@ func (tf TransformerFunc) Transform(cmp base.Compare, in Span, out *Span) error
 	return tf(cmp, in, out)
 }
 
-var noopTransform Transformer = TransformerFunc(func(_ base.Compare, s Span, dst *Span) error {
+// NoopTransform is a Transformer that performs no mutations.
+var NoopTransform Transformer = TransformerFunc(func(_ base.Compare, s Span, dst *Span) error {
 	dst.Start, dst.End = s.Start, s.End
 	dst.Keys = append(dst.Keys[:0], s.Keys...)
 	return nil

range_del_test.go

@@ -356,7 +356,7 @@ func TestRangeDelCompactionTruncation(t *testing.T) {
 1:
   000008:[a#12,RANGEDEL-b#inf,RANGEDEL]
 2:
-  000012:[b#13,SETWITHDEL-c#inf,RANGEDEL]
+  000012:[b#13,SET-c#inf,RANGEDEL]
 3:
   000013:[c#14,SET-d#inf,RANGEDEL]
 `)

testdata/compaction_delete_only_hints

@@ -221,7 +221,7 @@ L6
 a.SET.20:b a.RANGEDEL.15:z
 ----
 6:
-  000004:[a#20,SETWITHDEL-z#inf,RANGEDEL]
+  000004:[a#20,SET-z#inf,RANGEDEL]
 
 # Note that this test depends on stats being present on the sstables, so we
 # collect hints here. We expect none, as the table is in L6.
@@ -255,7 +255,7 @@ L0.000001 a-z seqnums(tombstone=5-27, file-smallest=0, type=point-key-only)
 close-snapshot
 10
 ----
-[JOB 100] compacted(elision-only) L6 [000004] (741B) Score=0.00 + L6 [] (0B) Score=0.00 -> L6 [000005] (662B), in 1.0s (2.0s total), output rate 662B/s
+[JOB 100] compacted(elision-only) L6 [000004] (825B) Score=0.00 + L6 [] (0B) Score=0.00 -> L6 [000005] (746B), in 1.0s (2.0s total), output rate 746B/s
 
 # The deletion hint was removed by the elision-only compaction.
 get-hints

testdata/manual_compaction_set_with_del_sstable_Pebblev4

@@ -93,7 +93,7 @@ range-deletions-bytes-estimate: 1334
 compact a-e L1
 ----
 2:
-  000008:[a#3,SETWITHDEL-c#inf,RANGEDEL]
+  000008:[a#3,SET-c#inf,RANGEDEL]
   000009:[c#2,RANGEDEL-e#inf,RANGEDEL]
 3:
   000006:[a#0,SET-b#0,SET]
@@ -141,7 +141,7 @@ L3
 compact a-e L1
 ----
 2:
-  000010:[a#3,SETWITHDEL-c#inf,RANGEDEL]
+  000010:[a#3,SET-c#inf,RANGEDEL]
   000011:[c#2,RANGEDEL-e#inf,RANGEDEL]
   000012:[e#2,RANGEDEL-g#inf,RANGEDEL]
 3:
@@ -181,7 +181,7 @@ L3
 compact a-e L1
 ----
 2:
-  000009:[a#3,SETWITHDEL-c#inf,RANGEDEL]
+  000009:[a#3,SET-c#inf,RANGEDEL]
   000010:[c#2,RANGEDEL-h#3,SET]
 3:
   000006:[a#0,SET-b#0,SET]
@@ -354,7 +354,7 @@ compact a-e L1
 0.0:
   000004:[c#4,SET-c#4,SET]
 2:
-  000008:[a#3,SETWITHDEL-b#inf,RANGEDEL]
+  000008:[a#3,SET-b#inf,RANGEDEL]
   000009:[b#2,RANGEDEL-e#inf,RANGEDEL]
 3:
   000007:[b#1,SET-b#1,SET]
@@ -702,7 +702,7 @@ compact a-q L1
 ----
 2:
   000011:[a#4,RANGEDEL-d#inf,RANGEDEL]
-  000012:[k#5,RANGEDEL-m#inf,RANGEDEL]
+  000012:[k#5,RANGEDEL-q#inf,RANGEDEL]
 3:
   000008:[a#1,SET-c#1,SET]
   000009:[ff#1,SET-ff#1,SET]