diff --git a/external_iterator.go b/external_iterator.go index 77e2015704..1d8f0d5217 100644 --- a/external_iterator.go +++ b/external_iterator.go @@ -6,8 +6,6 @@ package pebble import ( "context" - "fmt" - "sort" "github.com/cockroachdb/errors" "github.com/cockroachdb/pebble/internal/base" @@ -189,7 +187,6 @@ func createExternalPointIter(ctx context.Context, it *Iterator) (topLevelIterato seqNum += len(readers) } for _, readers := range it.externalReaders { - var combinedIters []internalIterator for _, r := range readers { var ( rangeDelIter keyspan.FragmentIterator @@ -220,21 +217,6 @@ func createExternalPointIter(ctx context.Context, it *Iterator) (topLevelIterato rangeDelIter: rangeDelIter, }) } - if len(combinedIters) == 1 { - mlevels = append(mlevels, mergingIterLevel{ - iter: combinedIters[0], - }) - } else if len(combinedIters) > 1 { - sli := &simpleLevelIter{ - cmp: it.cmp, - iters: combinedIters, - } - sli.init(it.opts) - mlevels = append(mlevels, mergingIterLevel{ - iter: sli, - rangeDelIter: nil, - }) - } } it.alloc.merging.init(&it.opts, &it.stats.InternalStats, it.comparer.Compare, it.comparer.Split, mlevels...) @@ -331,199 +313,3 @@ func openExternalTables( } return readers, err } - -// simpleLevelIter is similar to a levelIter in that it merges the points -// from multiple point iterators that are non-overlapping in the key ranges -// they return. It is only expected to support forward iteration and forward -// regular seeking; reverse iteration and prefix seeking is not supported. -// Intended to be a low-overhead, non-FileMetadata dependent option for -// NewExternalIter. To optimize seeking and forward iteration, it maintains -// two slices of child iterators; one of all iterators, and a subset of it that -// contains just the iterators that contain point keys within the current -// bounds. -// -// Note that this levelIter does not support pausing at file boundaries -// in case of range tombstones in this file that could apply to points outside -// of this file (and outside of this level). This is sufficient for optimizing -// the main use cases of NewExternalIter, however for completeness it would make -// sense to build this pausing functionality in. -type simpleLevelIter struct { - cmp Compare - err error - lowerBound []byte - iters []internalIterator - filtered []internalIterator - firstKeys [][]byte - firstKeysBuf []byte - currentIdx int -} - -var _ internalIterator = &simpleLevelIter{} - -// init initializes this simpleLevelIter. -func (s *simpleLevelIter) init(opts IterOptions) { - s.currentIdx = 0 - s.lowerBound = opts.LowerBound - s.resetFilteredIters() -} - -func (s *simpleLevelIter) resetFilteredIters() { - s.filtered = s.filtered[:0] - s.firstKeys = s.firstKeys[:0] - s.firstKeysBuf = s.firstKeysBuf[:0] - s.err = nil - for i := range s.iters { - var iterKV *base.InternalKV - if s.lowerBound != nil { - iterKV = s.iters[i].SeekGE(s.lowerBound, base.SeekGEFlagsNone) - } else { - iterKV = s.iters[i].First() - } - if iterKV != nil { - s.filtered = append(s.filtered, s.iters[i]) - bufStart := len(s.firstKeysBuf) - s.firstKeysBuf = append(s.firstKeysBuf, iterKV.K.UserKey...) - s.firstKeys = append(s.firstKeys, s.firstKeysBuf[bufStart:bufStart+len(iterKV.K.UserKey)]) - } else if err := s.iters[i].Error(); err != nil { - s.err = err - } - } -} - -func (s *simpleLevelIter) SeekGE(key []byte, flags base.SeekGEFlags) *base.InternalKV { - if s.err != nil { - return nil - } - // Find the first file that is entirely >= key. The file before that could - // contain the key we're looking for. - n := sort.Search(len(s.firstKeys), func(i int) bool { - return s.cmp(key, s.firstKeys[i]) <= 0 - }) - if n > 0 { - s.currentIdx = n - 1 - } else { - s.currentIdx = n - } - if s.currentIdx < len(s.filtered) { - if iterKV := s.filtered[s.currentIdx].SeekGE(key, flags); iterKV != nil { - return iterKV - } - if err := s.filtered[s.currentIdx].Error(); err != nil { - s.err = err - } - s.currentIdx++ - } - return s.skipEmptyFileForward(key, flags) -} - -func (s *simpleLevelIter) skipEmptyFileForward( - seekKey []byte, flags base.SeekGEFlags, -) *base.InternalKV { - var iterKV *base.InternalKV - for s.currentIdx >= 0 && s.currentIdx < len(s.filtered) && s.err == nil { - if seekKey != nil { - iterKV = s.filtered[s.currentIdx].SeekGE(seekKey, flags) - } else if s.lowerBound != nil { - iterKV = s.filtered[s.currentIdx].SeekGE(s.lowerBound, flags) - } else { - iterKV = s.filtered[s.currentIdx].First() - } - if iterKV != nil { - return iterKV - } - if err := s.filtered[s.currentIdx].Error(); err != nil { - s.err = err - } - s.currentIdx++ - } - return nil -} - -func (s *simpleLevelIter) SeekPrefixGE( - prefix, key []byte, flags base.SeekGEFlags, -) *base.InternalKV { - panic("unimplemented") -} - -func (s *simpleLevelIter) SeekLT(key []byte, flags base.SeekLTFlags) *base.InternalKV { - panic("unimplemented") -} - -func (s *simpleLevelIter) First() *base.InternalKV { - if s.err != nil { - return nil - } - s.currentIdx = 0 - return s.skipEmptyFileForward(nil /* seekKey */, base.SeekGEFlagsNone) -} - -func (s *simpleLevelIter) Last() *base.InternalKV { - panic("unimplemented") -} - -func (s *simpleLevelIter) Next() *base.InternalKV { - if s.err != nil { - return nil - } - if s.currentIdx < 0 || s.currentIdx >= len(s.filtered) { - return nil - } - if iterKV := s.filtered[s.currentIdx].Next(); iterKV != nil { - return iterKV - } - s.currentIdx++ - return s.skipEmptyFileForward(nil /* seekKey */, base.SeekGEFlagsNone) -} - -func (s *simpleLevelIter) NextPrefix(succKey []byte) *base.InternalKV { - if s.err != nil { - return nil - } - if s.currentIdx < 0 || s.currentIdx >= len(s.filtered) { - return nil - } - if iterKV := s.filtered[s.currentIdx].NextPrefix(succKey); iterKV != nil { - return iterKV - } - s.currentIdx++ - return s.skipEmptyFileForward(succKey /* seekKey */, base.SeekGEFlagsNone) -} - -func (s *simpleLevelIter) Prev() *base.InternalKV { - panic("unimplemented") -} - -func (s *simpleLevelIter) Error() error { - if s.currentIdx >= 0 && s.currentIdx < len(s.filtered) { - s.err = firstError(s.err, s.filtered[s.currentIdx].Error()) - } - return s.err -} - -func (s *simpleLevelIter) Close() error { - var err error - for i := range s.iters { - err = firstError(err, s.iters[i].Close()) - } - return err -} - -func (s *simpleLevelIter) SetBounds(lower, upper []byte) { - s.currentIdx = -1 - s.lowerBound = lower - for i := range s.iters { - s.iters[i].SetBounds(lower, upper) - } - s.resetFilteredIters() -} - -func (s *simpleLevelIter) SetContext(_ context.Context) {} - -func (s *simpleLevelIter) String() string { - if s.currentIdx < 0 || s.currentIdx >= len(s.filtered) { - return "simpleLevelIter: current=" - } - return fmt.Sprintf("simpleLevelIter: current=%s", s.filtered[s.currentIdx]) -} - -var _ internalIterator = &simpleLevelIter{} diff --git a/external_iterator_test.go b/external_iterator_test.go index 438e1d3489..f503303e39 100644 --- a/external_iterator_test.go +++ b/external_iterator_test.go @@ -10,8 +10,6 @@ import ( "testing" "github.com/cockroachdb/datadriven" - "github.com/cockroachdb/errors" - "github.com/cockroachdb/pebble/internal/itertest" "github.com/cockroachdb/pebble/internal/testkeys" "github.com/cockroachdb/pebble/objstorage/objstorageprovider" "github.com/cockroachdb/pebble/sstable" @@ -70,70 +68,6 @@ func TestExternalIterator(t *testing.T) { }) } -func TestSimpleLevelIter(t *testing.T) { - mem := vfs.NewMem() - o := &Options{ - FS: mem, - Comparer: testkeys.Comparer, - } - o.testingRandomized(t) - o.EnsureDefaults() - d, err := Open("", o) - require.NoError(t, err) - defer func() { require.NoError(t, d.Close()) }() - - datadriven.RunTest(t, "testdata/simple_level_iter", func(t *testing.T, td *datadriven.TestData) string { - switch td.Cmd { - case "reset": - mem = vfs.NewMem() - return "" - case "build": - if err := runBuildCmd(td, d, mem); err != nil { - return err.Error() - } - return "" - case "iter": - var files []sstable.ReadableFile - var filenames []string - td.ScanArgs(t, "files", &filenames) - for _, name := range filenames { - f, err := mem.Open(name) - require.NoError(t, err) - files = append(files, f) - } - readers, err := openExternalTables(o, files, 0, o.MakeReaderOptions()) - require.NoError(t, err) - defer func() { - for i := range readers { - _ = readers[i].Close() - } - }() - var internalIters []internalIterator - for i := range readers { - iter, err := readers[i].NewIter(sstable.NoTransforms, nil, nil) - require.NoError(t, err) - internalIters = append(internalIters, iter) - } - it := &simpleLevelIter{cmp: o.Comparer.Compare, iters: internalIters} - it.init(IterOptions{}) - - response := itertest.RunInternalIterCmd(t, td, it) - require.NoError(t, it.Close()) - return response - default: - return fmt.Sprintf("unknown command: %s", td.Cmd) - } - }) -} - -func TestSimpleIterError(t *testing.T) { - s := simpleLevelIter{cmp: DefaultComparer.Compare, iters: []internalIterator{&errorIter{err: errors.New("injected")}}} - s.init(IterOptions{}) - defer s.Close() - require.Nil(t, s.First()) - require.Error(t, s.Error()) -} - func BenchmarkExternalIter_NonOverlapping_Scan(b *testing.B) { ks := testkeys.Alpha(6) opts := (&Options{Comparer: testkeys.Comparer}).EnsureDefaults() diff --git a/testdata/simple_level_iter b/testdata/simple_level_iter deleted file mode 100644 index 76cd65825a..0000000000 --- a/testdata/simple_level_iter +++ /dev/null @@ -1,77 +0,0 @@ -build 1 -set b b -set c c ----- - - -iter files=(1) -first -next -next ----- -b:b -c:c -. - -build 2 -set d d -set f f ----- - -iter files=(1, 2) -first -next -next -next ----- -b:b -c:c -d:d -f:f - -# Test seeks within files. - -iter files=(1, 2) -seek-ge bb -next -next -next ----- -c:c -d:d -f:f -. - -iter files=(1, 2) -seek-ge a -next -next -next ----- -b:b -c:c -d:d -f:f - -iter files=(1, 2) -seek-ge d -next -next ----- -d:d -f:f -. - -iter files=(1, 2) -seek-ge f -next ----- -f:f -. - -iter files=(1, 2) -seek-ge ff -next ----- -. -.