Skip to content

Commit

Permalink
sstable: remove compaction iterator wrappers
Browse files Browse the repository at this point in the history 
The `compactionIterator` and `twoLevelCompactionIterator` wrappers no
longer serve any purpose.
  • Loading branch information
@RaduBerinde
RaduBerinde committed Jul 30, 2024
1 parent 1cc8f4c commit a6580b1
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 200 deletions.
14 changes: 8 additions & 6 deletions sstable/reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,12 @@ func (r *Reader) newIterWithBlockPropertyFiltersAndContext(
return res, nil
}

// NewIter returns an iterator for the contents of the table. If an error
// occurs, NewIter cleans up after itself and returns a nil iterator. NewIter
// must only be used when the Reader is guaranteed to outlive any LazyValues
// returned from the iter.
// NewIter returns an iterator for the point keys in the table. It is a
// simplified version of NewIterWithBlockPropertyFiltersAndContextEtc and
// should only be used for tests and tooling.
//
// NewIter must only be used when the Reader is guaranteed to outlive any
// LazyValues returned from the iter.
func (r *Reader) NewIter(transforms IterTransforms, lower, upper []byte) (Iterator, error) {
// TODO(radu): we should probably not use bloom filters in this case, as there
// likely isn't a cache set up.
Expand Down Expand Up @@ -268,7 +270,7 @@ func (r *Reader) newCompactionIter(
return nil, err
}
i.setupForCompaction()
return &twoLevelCompactionIterator{twoLevelIterator: i}, nil
return i, nil
}
i, err := newSingleLevelIterator(
context.Background(), r, vState, transforms, nil /* lower */, nil, /* upper */
Expand All @@ -278,7 +280,7 @@ func (r *Reader) newCompactionIter(
return nil, err
}
i.setupForCompaction()
return &compactionIterator{singleLevelIterator: i}, nil
return i, nil
}

// NewRawRangeDelIter returns an internal iterator for the contents of the
Expand Down
97 changes: 0 additions & 97 deletions sstable/reader_iter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,100 +149,3 @@ func checkTwoLevelIterator(obj interface{}) {
os.Exit(1)
}
}

// compactionIterator is similar to Iterator but it increments the number of
// bytes that have been iterated through.
type compactionIterator struct {
*singleLevelIterator
}

// compactionIterator implements the base.InternalIterator interface.
var _ base.InternalIterator = (*compactionIterator)(nil)

func (i *compactionIterator) String() string {
if i.vState != nil {
return i.vState.fileNum.String()
}
return i.reader.cacheOpts.FileNum.String()
}

func (i *compactionIterator) SeekGE(key []byte, flags base.SeekGEFlags) *base.InternalKV {
panic("pebble: SeekGE unimplemented")
}

func (i *compactionIterator) SeekPrefixGE(
prefix, key []byte, flags base.SeekGEFlags,
) *base.InternalKV {
panic("pebble: SeekPrefixGE unimplemented")
}

func (i *compactionIterator) SeekLT(key []byte, flags base.SeekLTFlags) *base.InternalKV {
panic("pebble: SeekLT unimplemented")
}

func (i *compactionIterator) First() *base.InternalKV {
i.err = nil // clear cached iteration error
return i.skipForward(i.singleLevelIterator.First())
}

func (i *compactionIterator) Last() *base.InternalKV {
panic("pebble: Last unimplemented")
}

// Note: compactionIterator.Next mirrors the implementation of Iterator.Next
// due to performance. Keep the two in sync.
func (i *compactionIterator) Next() *base.InternalKV {
if i.err != nil {
return nil
}
return i.skipForward(i.data.Next())
}

func (i *compactionIterator) NextPrefix(succKey []byte) *base.InternalKV {
panic("pebble: NextPrefix unimplemented")
}

func (i *compactionIterator) Prev() *base.InternalKV {
panic("pebble: Prev unimplemented")
}

func (i *compactionIterator) skipForward(kv *base.InternalKV) *base.InternalKV {
if kv == nil {
for {
if kv := i.index.Next(); kv == nil {
break
}
result := i.loadBlock(+1)
if result != loadBlockOK {
if i.err != nil {
break
}
switch result {
case loadBlockFailed:
// We checked that i.index was at a valid entry, so
// loadBlockFailed could not have happened due to to i.index
// being exhausted, and must be due to an error.
panic("loadBlock should not have failed with no error")
case loadBlockIrrelevant:
panic("compactionIter should not be using block intervals for skipping")
default:
panic(fmt.Sprintf("unexpected case %d", result))
}
}
// result == loadBlockOK
if kv = i.data.First(); kv != nil {
break
}
}
}

// We have an upper bound when the table is virtual.
if i.upper != nil && kv != nil {
cmp := i.cmp(kv.K.UserKey, i.upper)
if cmp > 0 || (!i.endKeyInclusive && cmp == 0) {
return nil
}
}

return kv
}
94 changes: 0 additions & 94 deletions sstable/reader_iter_two_lvl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -964,97 +964,3 @@ func (i *twoLevelIterator) Close() error {
twoLevelIterPool.Put(i)
return err
}

// Note: twoLevelCompactionIterator and compactionIterator are very similar but
// were separated due to performance.
type twoLevelCompactionIterator struct {
*twoLevelIterator
}

// twoLevelCompactionIterator implements the base.InternalIterator interface.
var _ base.InternalIterator = (*twoLevelCompactionIterator)(nil)

func (i *twoLevelCompactionIterator) Close() error {
return i.twoLevelIterator.Close()
}

func (i *twoLevelCompactionIterator) SeekGE(key []byte, flags base.SeekGEFlags) *base.InternalKV {
panic("pebble: SeekGE unimplemented")
}

func (i *twoLevelCompactionIterator) SeekPrefixGE(
prefix, key []byte, flags base.SeekGEFlags,
) *base.InternalKV {
panic("pebble: SeekPrefixGE unimplemented")
}

func (i *twoLevelCompactionIterator) SeekLT(key []byte, flags base.SeekLTFlags) *base.InternalKV {
panic("pebble: SeekLT unimplemented")
}

func (i *twoLevelCompactionIterator) First() *base.InternalKV {
i.err = nil // clear cached iteration error
return i.skipForward(i.twoLevelIterator.First())
}

func (i *twoLevelCompactionIterator) Last() *base.InternalKV {
panic("pebble: Last unimplemented")
}

// Note: twoLevelCompactionIterator.Next mirrors the implementation of
// twoLevelIterator.Next due to performance. Keep the two in sync.
func (i *twoLevelCompactionIterator) Next() *base.InternalKV {
if i.err != nil {
return nil
}
return i.skipForward(i.singleLevelIterator.Next())
}

func (i *twoLevelCompactionIterator) NextPrefix(succKey []byte) *base.InternalKV {
panic("pebble: NextPrefix unimplemented")
}

func (i *twoLevelCompactionIterator) Prev() *base.InternalKV {
panic("pebble: Prev unimplemented")
}

func (i *twoLevelCompactionIterator) skipForward(kv *base.InternalKV) *base.InternalKV {
if kv == nil {
for {
if key := i.topLevelIndex.Next(); key == nil {
break
}
result := i.loadIndex(+1)
if result != loadBlockOK {
if i.err != nil {
break
}
switch result {
case loadBlockFailed:
// We checked that i.index was at a valid entry, so
// loadBlockFailed could not have happened due to to i.index
// being exhausted, and must be due to an error.
panic("loadBlock should not have failed with no error")
case loadBlockIrrelevant:
panic("compactionIter should not be using block intervals for skipping")
default:
panic(fmt.Sprintf("unexpected case %d", result))
}
}
// result == loadBlockOK
if kv = i.singleLevelIterator.First(); kv != nil {
break
}
}
}

// We have an upper bound when the table is virtual.
if i.upper != nil && kv != nil {
cmp := i.cmp(kv.K.UserKey, i.upper)
if cmp > 0 || (!i.endKeyInclusive && cmp == 0) {
return nil
}
}

return kv
}
6 changes: 3 additions & 3 deletions sstable/reader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -980,12 +980,12 @@ func TestCompactionIteratorSetupForCompaction(t *testing.T) {
NoTransforms, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r), &pool)
require.NoError(t, err)
switch i := citer.(type) {
case *compactionIterator:
case *singleLevelIterator:
require.True(t, objstorageprovider.TestingCheckMaxReadahead(i.dataRH))
// Each key has one version, so no value block, regardless of
// sstable version.
require.Nil(t, i.vbRH)
case *twoLevelCompactionIterator:
case *twoLevelIterator:
require.True(t, objstorageprovider.TestingCheckMaxReadahead(i.dataRH))
// Each key has one version, so no value block, regardless of
// sstable version.
Expand Down Expand Up @@ -1036,7 +1036,7 @@ func TestReadaheadSetupForV3TablesWithMultipleVersions(t *testing.T) {
NoTransforms, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r), &pool)
require.NoError(t, err)
defer citer.Close()
i := citer.(*compactionIterator)
i := citer.(*singleLevelIterator)
require.True(t, objstorageprovider.TestingCheckMaxReadahead(i.dataRH))
require.True(t, objstorageprovider.TestingCheckMaxReadahead(i.vbRH))
}
Expand Down

0 comments on commit a6580b1

Please sign in to comment.