From ab0e2b38c1462e3f4a39674161c5ea6427e47eed Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Thu, 11 Jan 2024 10:53:59 -0500 Subject: [PATCH] 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 #3082. --- compaction.go | 44 +++--- error_test.go | 4 +- internal/keyspan/defragment_test.go | 6 +- internal/keyspan/interleaving_iter_test.go | 2 +- internal/keyspan/internal_iter_shim.go | 125 ------------------ internal/keyspan/transformer.go | 3 +- range_del_test.go | 2 +- testdata/compaction_delete_only_hints | 4 +- ...l_compaction_set_with_del_sstable_Pebblev4 | 10 +- testdata/range_del | 28 ++-- 10 files changed, 50 insertions(+), 178 deletions(-) delete mode 100644 internal/keyspan/internal_iter_shim.go diff --git a/compaction.go b/compaction.go index 4394a4bebd..0863409bae 100644 --- a/compaction.go +++ b/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,6 +1375,15 @@ 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 @@ -1384,29 +1391,18 @@ func (c *compaction) newInputIter( // 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 diff --git a/error_test.go b/error_test.go index e525afd9ac..97cac7dcd3 100644 --- a/error_test.go +++ b/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) diff --git a/internal/keyspan/defragment_test.go b/internal/keyspan/defragment_test.go index b9856da2c0..c9a7b01afb 100644 --- a/internal/keyspan/defragment_test.go +++ b/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)) diff --git a/internal/keyspan/interleaving_iter_test.go b/internal/keyspan/interleaving_iter_test.go index 116f037614..99202325bf 100644 --- a/internal/keyspan/interleaving_iter_test.go +++ b/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}) diff --git a/internal/keyspan/internal_iter_shim.go b/internal/keyspan/internal_iter_shim.go deleted file mode 100644 index bb9e37bdf9..0000000000 --- a/internal/keyspan/internal_iter_shim.go +++ /dev/null @@ -1,125 +0,0 @@ -// Copyright 2022 The LevelDB-Go and Pebble Authors. All rights reserved. Use -// of this source code is governed by a BSD-style license that can be found in -// the LICENSE file. - -package keyspan - -import ( - "context" - - "github.com/cockroachdb/pebble/internal/base" -) - -// InternalIteratorShim is a temporary iterator type used as a shim between -// keyspan.MergingIter and base.InternalIterator. It's used temporarily for -// range deletions during compactions, allowing range deletions to be -// interleaved by a compaction input iterator. -// -// TODO(jackson): This type should be removed, and the usages converted to using -// an InterleavingIterator type that interleaves keyspan.Spans from a -// keyspan.FragmentIterator with point keys. -type InternalIteratorShim struct { - miter MergingIter - mbufs MergingBuffers - span *Span - iterKey base.InternalKey -} - -// Assert that InternalIteratorShim implements InternalIterator. -var _ base.InternalIterator = &InternalIteratorShim{} - -// Init initializes the internal iterator shim to merge the provided fragment -// iterators. -func (i *InternalIteratorShim) Init(cmp base.Compare, iters ...FragmentIterator) { - i.miter.Init(cmp, noopTransform, &i.mbufs, iters...) -} - -// Span returns the span containing the full set of keys over the key span at -// the current iterator position. -func (i *InternalIteratorShim) Span() *Span { - return i.span -} - -// SeekGE implements (base.InternalIterator).SeekGE. -func (i *InternalIteratorShim) SeekGE( - key []byte, flags base.SeekGEFlags, -) (*base.InternalKey, base.LazyValue) { - panic("unimplemented") -} - -// SeekPrefixGE implements (base.InternalIterator).SeekPrefixGE. -func (i *InternalIteratorShim) SeekPrefixGE( - prefix, key []byte, flags base.SeekGEFlags, -) (*base.InternalKey, base.LazyValue) { - panic("unimplemented") -} - -// SeekLT implements (base.InternalIterator).SeekLT. -func (i *InternalIteratorShim) SeekLT( - key []byte, flags base.SeekLTFlags, -) (*base.InternalKey, base.LazyValue) { - panic("unimplemented") -} - -// First implements (base.InternalIterator).First. -func (i *InternalIteratorShim) First() (*base.InternalKey, base.LazyValue) { - i.span = i.miter.First() - for i.span != nil && i.span.Empty() { - i.span = i.miter.Next() - } - if i.span == nil { - return nil, base.LazyValue{} - } - i.iterKey = base.InternalKey{UserKey: i.span.Start, Trailer: i.span.Keys[0].Trailer} - return &i.iterKey, base.MakeInPlaceValue(i.span.End) -} - -// Last implements (base.InternalIterator).Last. -func (i *InternalIteratorShim) Last() (*base.InternalKey, base.LazyValue) { - panic("unimplemented") -} - -// Next implements (base.InternalIterator).Next. -func (i *InternalIteratorShim) Next() (*base.InternalKey, base.LazyValue) { - i.span = i.miter.Next() - for i.span != nil && i.span.Empty() { - i.span = i.miter.Next() - } - if i.span == nil { - return nil, base.LazyValue{} - } - i.iterKey = base.InternalKey{UserKey: i.span.Start, Trailer: i.span.Keys[0].Trailer} - return &i.iterKey, base.MakeInPlaceValue(i.span.End) -} - -// NextPrefix implements (base.InternalIterator).NextPrefix. -func (i *InternalIteratorShim) NextPrefix([]byte) (*base.InternalKey, base.LazyValue) { - panic("unimplemented") -} - -// Prev implements (base.InternalIterator).Prev. -func (i *InternalIteratorShim) Prev() (*base.InternalKey, base.LazyValue) { - panic("unimplemented") -} - -// Error implements (base.InternalIterator).Error. -func (i *InternalIteratorShim) Error() error { - return i.miter.Error() -} - -// Close implements (base.InternalIterator).Close. -func (i *InternalIteratorShim) Close() error { - return i.miter.Close() -} - -// SetBounds implements (base.InternalIterator).SetBounds. -func (i *InternalIteratorShim) SetBounds(lower, upper []byte) { -} - -// SetContext implements (base.InternalIterator).SetContext. -func (i *InternalIteratorShim) SetContext(_ context.Context) {} - -// String implements fmt.Stringer. -func (i *InternalIteratorShim) String() string { - return i.miter.String() -} diff --git a/internal/keyspan/transformer.go b/internal/keyspan/transformer.go index e0152cf4d6..e886c90c2f 100644 --- a/internal/keyspan/transformer.go +++ b/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 diff --git a/range_del_test.go b/range_del_test.go index 07251ab33a..7d7f4c0ac0 100644 --- a/range_del_test.go +++ b/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] `) diff --git a/testdata/compaction_delete_only_hints b/testdata/compaction_delete_only_hints index e11d120295..00d15ea03a 100644 --- a/testdata/compaction_delete_only_hints +++ b/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 diff --git a/testdata/manual_compaction_set_with_del_sstable_Pebblev4 b/testdata/manual_compaction_set_with_del_sstable_Pebblev4 index 5807ec5287..16b63b14d1 100644 --- a/testdata/manual_compaction_set_with_del_sstable_Pebblev4 +++ b/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] diff --git a/testdata/range_del b/testdata/range_del index 53cca9d90e..a565e532d8 100644 --- a/testdata/range_del +++ b/testdata/range_del @@ -1011,13 +1011,13 @@ mem: 1 compact a-e ---- 1: - 000007:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000007:[a#11,SET-c#inf,RANGEDEL] 000008:[c#12,SET-e#inf,RANGEDEL] compact d-e ---- 1: - 000007:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000007:[a#11,SET-c#inf,RANGEDEL] 2: 000008:[c#12,SET-e#inf,RANGEDEL] @@ -1049,7 +1049,7 @@ mem: 1 compact a-e ---- 1: - 000007:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000007:[a#11,SET-c#inf,RANGEDEL] 000008:[c#12,SET-e#inf,RANGEDEL] compact a-b @@ -1057,7 +1057,7 @@ compact a-b 1: 000008:[c#12,SET-e#inf,RANGEDEL] 2: - 000007:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000007:[a#11,SET-c#inf,RANGEDEL] iter seq=13 seek-lt d @@ -1097,7 +1097,7 @@ mem: 1 compact a-b ---- 1: - 000008:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000008:[a#11,SET-c#inf,RANGEDEL] 000009:[c#12,SET-d#inf,RANGEDEL] 000010:[d#10,RANGEDEL-e#inf,RANGEDEL] 2: @@ -1106,7 +1106,7 @@ compact a-b compact d-e ---- 1: - 000008:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000008:[a#11,SET-c#inf,RANGEDEL] 000009:[c#12,SET-d#inf,RANGEDEL] 3: 000011:[d#10,RANGEDEL-e#inf,RANGEDEL] @@ -1121,7 +1121,7 @@ compact a-b L1 1: 000009:[c#12,SET-d#inf,RANGEDEL] 2: - 000008:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000008:[a#11,SET-c#inf,RANGEDEL] 3: 000011:[d#10,RANGEDEL-e#inf,RANGEDEL] @@ -1160,7 +1160,7 @@ compact a-b 0.0: 000007:[f#13,SET-f#13,SET] 1: - 000009:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000009:[a#11,SET-c#inf,RANGEDEL] 000010:[c#12,SET-d#inf,RANGEDEL] 000011:[d#10,RANGEDEL-e#inf,RANGEDEL] 2: @@ -1171,7 +1171,7 @@ compact d-e 0.0: 000007:[f#13,SET-f#13,SET] 1: - 000009:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000009:[a#11,SET-c#inf,RANGEDEL] 000010:[c#12,SET-d#inf,RANGEDEL] 3: 000012:[d#10,RANGEDEL-e#inf,RANGEDEL] @@ -1184,7 +1184,7 @@ c:v compact f-f L0 ---- 1: - 000009:[a#11,SETWITHDEL-c#inf,RANGEDEL] + 000009:[a#11,SET-c#inf,RANGEDEL] 000010:[c#12,SET-d#inf,RANGEDEL] 000007:[f#13,SET-f#13,SET] 3: @@ -1193,8 +1193,8 @@ compact f-f L0 compact a-f L1 ---- 2: - 000013:[a#11,SETWITHDEL-c#inf,RANGEDEL] - 000014:[c#12,SETWITHDEL-d#inf,RANGEDEL] + 000013:[a#11,SET-c#inf,RANGEDEL] + 000014:[c#12,SET-d#inf,RANGEDEL] 000015:[f#13,SET-f#13,SET] 3: 000012:[d#10,RANGEDEL-e#inf,RANGEDEL] @@ -1255,8 +1255,8 @@ range-deletions-bytes-estimate: 1398 wait-pending-table-stats 000005 ---- -num-entries: 2 -num-deletions: 2 +num-entries: 1 +num-deletions: 1 num-range-key-sets: 0 point-deletions-bytes-estimate: 0 range-deletions-bytes-estimate: 1398