From b6400d6321f64fa1c451954441517516e7d589b7 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Mon, 29 Jul 2024 19:12:37 -0700 Subject: [PATCH] sstable: rename NewIterWithBlockPropertyFiltersAndContextEtc Rename to `NewPointIter`. --- external_iterator.go | 2 +- level_iter_test.go | 2 +- merging_iter_test.go | 2 +- sstable/block_property_test.go | 4 ++-- sstable/random_test.go | 4 ++-- sstable/reader.go | 19 +++++++++---------- sstable/reader_common.go | 2 +- sstable/reader_test.go | 8 ++++---- sstable/reader_virtual.go | 17 +++++++++++------ table_cache.go | 2 +- 10 files changed, 33 insertions(+), 29 deletions(-) diff --git a/external_iterator.go b/external_iterator.go index 7d60adaf8c..08eecca095 100644 --- a/external_iterator.go +++ b/external_iterator.go @@ -156,7 +156,7 @@ func createExternalPointIter(ctx context.Context, it *Iterator) (topLevelIterato // BlockPropertiesFilterer that includes obsoleteKeyBlockPropertyFilter. transforms := sstable.IterTransforms{SyntheticSeqNum: sstable.SyntheticSeqNum(seqNum)} seqNum-- - pointIter, err = r.NewIterWithBlockPropertyFiltersAndContextEtc( + pointIter, err = r.NewPointIter( ctx, transforms, it.opts.LowerBound, it.opts.UpperBound, nil, /* BlockPropertiesFilterer */ sstable.NeverUseFilterBlock, &it.stats.InternalStats, it.opts.CategoryAndQoS, nil, diff --git a/level_iter_test.go b/level_iter_test.go index f23809ddf9..9efed17812 100644 --- a/level_iter_test.go +++ b/level_iter_test.go @@ -173,7 +173,7 @@ func (lt *levelIterTest) newIters( transforms := file.IterTransforms() var set iterSet if kinds.Point() { - iter, err := lt.readers[file.FileNum].NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err := lt.readers[file.FileNum].NewPointIter( ctx, transforms, opts.LowerBound, opts.UpperBound, nil, sstable.AlwaysUseFilterBlock, iio.stats, sstable.CategoryAndQoS{}, nil, sstable.MakeTrivialReaderProvider(lt.readers[file.FileNum])) diff --git a/merging_iter_test.go b/merging_iter_test.go index d167bc9c82..4bc721c9c9 100644 --- a/merging_iter_test.go +++ b/merging_iter_test.go @@ -175,7 +175,7 @@ func TestMergingIterCornerCases(t *testing.T) { } } if kinds.Point() { - set.point, err = r.NewIterWithBlockPropertyFiltersAndContextEtc( + set.point, err = r.NewPointIter( context.Background(), sstable.NoTransforms, opts.GetLowerBound(), opts.GetUpperBound(), nil, sstable.AlwaysUseFilterBlock, iio.stats, diff --git a/sstable/block_property_test.go b/sstable/block_property_test.go index 1d96d4677c..e2c4b646b2 100644 --- a/sstable/block_property_test.go +++ b/sstable/block_property_test.go @@ -971,7 +971,7 @@ func TestBlockProperties(t *testing.T) { } else if !ok { return "filter excludes entire table" } - iter, err := r.NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err := r.NewPointIter( context.Background(), NoTransforms, lower, upper, filterer, NeverUseFilterBlock, &stats, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r)) @@ -1054,7 +1054,7 @@ func TestBlockProperties_BoundLimited(t *testing.T) { } else if !ok { return "filter excludes entire table" } - iter, err := r.NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err := r.NewPointIter( context.Background(), NoTransforms, lower, upper, filterer, NeverUseFilterBlock, &stats, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r)) diff --git a/sstable/random_test.go b/sstable/random_test.go index 981bd85fba..60596eb0da 100644 --- a/sstable/random_test.go +++ b/sstable/random_test.go @@ -92,7 +92,7 @@ func runErrorInjectionTest(t *testing.T, seed int64) { }, nil, nil) } - // TOOD(jackson): NewIterWithBlockPropertyFiltersAndContextEtc returns an + // TOOD(jackson): NewPointIter returns an // iterator over point keys only. Should we add variants of this test that run // random operations on the range deletion and range key iterators? var stats base.InternalIteratorStats @@ -100,7 +100,7 @@ func runErrorInjectionTest(t *testing.T, seed int64) { if rng.Intn(2) == 1 { filterBlockSizeLimit = NeverUseFilterBlock } - it, err := r.NewIterWithBlockPropertyFiltersAndContextEtc( + it, err := r.NewPointIter( context.Background(), NoTransforms, nil /* lower TODO */, nil, /* upper TODO */ diff --git a/sstable/reader.go b/sstable/reader.go index 2c3a0f07cc..afec311fd9 100644 --- a/sstable/reader.go +++ b/sstable/reader.go @@ -151,13 +151,12 @@ func (r *Reader) Close() error { return nil } -// NewIterWithBlockPropertyFiltersAndContextEtc returns an iterator for the -// point keys in the table. +// NewPointIter returns an iterator for the point keys in the table. // // If transform.HideObsoletePoints is set, the callee assumes that filterer // already includes obsoleteKeyBlockPropertyFilter. The caller can satisfy this // contract by first calling TryAddBlockPropertyFilterForHideObsoletePoints. -func (r *Reader) NewIterWithBlockPropertyFiltersAndContextEtc( +func (r *Reader) NewPointIter( ctx context.Context, transforms IterTransforms, lower, upper []byte, @@ -168,14 +167,14 @@ func (r *Reader) NewIterWithBlockPropertyFiltersAndContextEtc( statsCollector *CategoryStatsCollector, rp ReaderProvider, ) (Iterator, error) { - return r.newIterWithBlockPropertyFiltersAndContext( + return r.newPointIter( ctx, transforms, lower, upper, filterer, filterBlockSizeLimit, stats, categoryAndQoS, statsCollector, rp, nil) } // TryAddBlockPropertyFilterForHideObsoletePoints is expected to be called -// before the call to NewIterWithBlockPropertyFiltersAndContextEtc, to get the -// value of hideObsoletePoints and potentially add a block property filter. +// before the call to NewPointIter, to get the value of hideObsoletePoints and +// potentially add a block property filter. func (r *Reader) TryAddBlockPropertyFilterForHideObsoletePoints( snapshotForHideObsoletePoints base.SeqNum, fileLargestSeqNum base.SeqNum, @@ -189,7 +188,7 @@ func (r *Reader) TryAddBlockPropertyFilterForHideObsoletePoints( return hideObsoletePoints, pointKeyFilters } -func (r *Reader) newIterWithBlockPropertyFiltersAndContext( +func (r *Reader) newPointIter( ctx context.Context, transforms IterTransforms, lower, upper []byte, @@ -223,15 +222,15 @@ func (r *Reader) newIterWithBlockPropertyFiltersAndContext( } // 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. +// simplified version of NewPointIter 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. - return r.NewIterWithBlockPropertyFiltersAndContextEtc( + return r.NewPointIter( context.TODO(), transforms, lower, upper, nil, AlwaysUseFilterBlock, nil /* stats */, CategoryAndQoS{}, nil /* statsCollector */, MakeTrivialReaderProvider(r)) } diff --git a/sstable/reader_common.go b/sstable/reader_common.go index 7158c9102b..0587110a06 100644 --- a/sstable/reader_common.go +++ b/sstable/reader_common.go @@ -25,7 +25,7 @@ type CommonReader interface { ctx context.Context, transforms FragmentIterTransforms, ) (keyspan.FragmentIterator, error) - NewIterWithBlockPropertyFiltersAndContextEtc( + NewPointIter( ctx context.Context, transforms IterTransforms, lower, upper []byte, diff --git a/sstable/reader_test.go b/sstable/reader_test.go index 3f771b6afc..6067c132e3 100644 --- a/sstable/reader_test.go +++ b/sstable/reader_test.go @@ -448,7 +448,7 @@ func runVirtualReaderTest(t *testing.T, path string, blockSize, indexBlockSize i } transforms := IterTransforms{SyntheticSuffix: syntheticSuffix} - iter, err := v.NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err := v.NewPointIter( context.Background(), transforms, lower, upper, filterer, NeverUseFilterBlock, &stats, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r)) if err != nil { @@ -827,7 +827,7 @@ func runTestReader(t *testing.T, o WriterOptions, dir string, r *Reader, printVa return "table does not intersect BlockPropertyFilter" } } - iter, err := r.NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err := r.NewPointIter( context.Background(), transforms, nil, /* lower */ @@ -1323,7 +1323,7 @@ func TestRandomizedPrefixSuffixRewriter(t *testing.T) { } eReader, err := newReader(f, opts) require.NoError(t, err) - iter, err := eReader.newIterWithBlockPropertyFiltersAndContext( + iter, err := eReader.newPointIter( context.Background(), block.IterTransforms{SyntheticSuffix: syntheticSuffix, SyntheticPrefix: syntheticPrefix}, nil, nil, nil, @@ -2448,7 +2448,7 @@ func BenchmarkIteratorScanObsolete(b *testing.B) { } } transforms := IterTransforms{HideObsoletePoints: hideObsoletePoints} - iter, err := r.NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err := r.NewPointIter( context.Background(), transforms, nil, nil, filterer, AlwaysUseFilterBlock, nil, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r)) diff --git a/sstable/reader_virtual.go b/sstable/reader_virtual.go index d258251658..771cb6a35b 100644 --- a/sstable/reader_virtual.go +++ b/sstable/reader_virtual.go @@ -101,11 +101,16 @@ func (v *VirtualReader) NewCompactionIter( transforms, categoryAndQoS, statsCollector, rp, &v.vState, bufferPool) } -// NewIterWithBlockPropertyFiltersAndContextEtc wraps -// Reader.NewIterWithBlockPropertyFiltersAndContext. We assume that the passed -// in [lower, upper) bounds will have at least some overlap with the virtual -// sstable bounds. No overlap is not currently supported in the iterator. -func (v *VirtualReader) NewIterWithBlockPropertyFiltersAndContextEtc( +// NewPointIter returns an iterator for the point keys in the table. +// +// If transform.HideObsoletePoints is set, the callee assumes that filterer +// already includes obsoleteKeyBlockPropertyFilter. The caller can satisfy this +// contract by first calling TryAddBlockPropertyFilterForHideObsoletePoints. +// +// We assume that the [lower, upper) bounds (if specified) will have at least +// some overlap with the virtual sstable bounds. No overlap is not currently +// supported in the iterator. +func (v *VirtualReader) NewPointIter( ctx context.Context, transforms IterTransforms, lower, upper []byte, @@ -116,7 +121,7 @@ func (v *VirtualReader) NewIterWithBlockPropertyFiltersAndContextEtc( statsCollector *CategoryStatsCollector, rp ReaderProvider, ) (Iterator, error) { - return v.reader.newIterWithBlockPropertyFiltersAndContext( + return v.reader.newPointIter( ctx, transforms, lower, upper, filterer, filterBlockSizeLimit, stats, categoryAndQoS, statsCollector, rp, &v.vState) } diff --git a/table_cache.go b/table_cache.go index cd3cb225ba..031b53870a 100644 --- a/table_cache.go +++ b/table_cache.go @@ -596,7 +596,7 @@ func (c *tableCacheShard) newPointIter( transforms, categoryAndQoS, dbOpts.sstStatsCollector, rp, internalOpts.bufferPool) } else { - iter, err = cr.NewIterWithBlockPropertyFiltersAndContextEtc( + iter, err = cr.NewPointIter( ctx, transforms, opts.GetLowerBound(), opts.GetUpperBound(), filterer, filterBlockSizeLimit, internalOpts.stats, categoryAndQoS, dbOpts.sstStatsCollector, rp) }