Skip to content

Commit f35e9f2

Browse files
jbowensjbowens
committed
sstable: add IterOptions
Add a IterOptions for propagating point iterator configuration options. The set of possible configuration options has ballooned and the struct makes call sites a little more legible.
1 parent 0e42024 commit f35e9f2

14 files changed

+191
-232
lines changed

‎cockroachkvs/cockroachkvs_bench_test.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,11 @@ func benchmarkRandSeekInSST(
130130
// Iterate through the entire table to warm up the cache.
131131
var stats base.InternalIteratorStats
132132
rp := sstable.MakeTrivialReaderProvider(reader)
133-
iter, err := reader.NewPointIter(
134-
ctx, sstable.NoTransforms, nil, nil, nil, sstable.NeverUseFilterBlock,
135-
block.ReadEnv{Stats: &stats, IterStats: nil}, rp)
133+
iter, err := reader.NewPointIter(ctx, sstable.IterOptions{
134+
FilterBlockSizeLimit: sstable.NeverUseFilterBlock,
135+
Env: block.ReadEnv{Stats: &stats, IterStats: nil},
136+
ReaderProvider: rp,
137+
})
136138
require.NoError(b, err)
137139
n := 0
138140
for kv := iter.First(); kv != nil; kv = iter.Next() {
@@ -146,9 +148,11 @@ func benchmarkRandSeekInSST(
146148
b.ResetTimer()
147149
for i := 0; i < b.N; i++ {
148150
key := queryKeys[i%numQueryKeys]
149-
iter, err := reader.NewPointIter(
150-
ctx, sstable.NoTransforms, nil, nil, nil, sstable.NeverUseFilterBlock,
151-
block.ReadEnv{Stats: &stats, IterStats: nil}, rp)
151+
iter, err := reader.NewPointIter(ctx, sstable.IterOptions{
152+
FilterBlockSizeLimit: sstable.NeverUseFilterBlock,
153+
Env: block.ReadEnv{Stats: &stats, IterStats: nil},
154+
ReaderProvider: rp,
155+
})
152156
if err != nil {
153157
b.Fatal(err)
154158
}

‎external_iterator.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,14 @@ func createExternalPointIter(
176176
// BlockPropertiesFilterer that includes obsoleteKeyBlockPropertyFilter.
177177
transforms := sstable.IterTransforms{SyntheticSeqNum: sstable.SyntheticSeqNum(seqNum)}
178178
seqNum--
179-
pointIter, err = r.NewPointIter(
180-
ctx, transforms, it.opts.LowerBound, it.opts.UpperBound, nil, /* BlockPropertiesFilterer */
181-
sstable.NeverUseFilterBlock, readEnv, sstable.MakeTrivialReaderProvider(r))
179+
pointIter, err = r.NewPointIter(ctx, sstable.IterOptions{
180+
Lower: it.opts.LowerBound,
181+
Upper: it.opts.UpperBound,
182+
Transforms: transforms,
183+
FilterBlockSizeLimit: sstable.NeverUseFilterBlock,
184+
Env: readEnv,
185+
ReaderProvider: sstable.MakeTrivialReaderProvider(r),
186+
})
182187
if err == nil {
183188
rangeDelIter, err = r.NewRawRangeDelIter(ctx, sstable.FragmentIterTransforms{
184189
SyntheticSeqNum: sstable.SyntheticSeqNum(seqNum),

‎file_cache.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -679,9 +679,15 @@ func (h *fileCacheHandle) newPointIter(
679679
if internalOpts.compaction {
680680
iter, err = cr.NewCompactionIter(transforms, internalOpts.readEnv, &v.readerProvider)
681681
} else {
682-
iter, err = cr.NewPointIter(
683-
ctx, transforms, opts.GetLowerBound(), opts.GetUpperBound(), filterer,
684-
filterBlockSizeLimit, internalOpts.readEnv, &v.readerProvider)
682+
iter, err = cr.NewPointIter(ctx, sstable.IterOptions{
683+
Lower: opts.GetLowerBound(),
684+
Upper: opts.GetUpperBound(),
685+
Transforms: transforms,
686+
FilterBlockSizeLimit: filterBlockSizeLimit,
687+
Filterer: filterer,
688+
Env: internalOpts.readEnv,
689+
ReaderProvider: &v.readerProvider,
690+
})
685691
}
686692
if err != nil {
687693
return nil, err

‎level_iter_test.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,21 @@ func (lt *levelIterTest) newIters(
183183
transforms := file.IterTransforms()
184184
var set iterSet
185185
if kinds.Point() {
186-
iter, err := lt.readers[file.FileNum].NewPointIter(
187-
ctx, transforms,
188-
opts.LowerBound, opts.UpperBound, nil, sstable.AlwaysUseFilterBlock, iio.readEnv, sstable.MakeTrivialReaderProvider(lt.readers[file.FileNum]))
186+
iter, err := lt.readers[file.FileNum].NewPointIter(ctx, sstable.IterOptions{
187+
Lower: opts.GetLowerBound(),
188+
Upper: opts.GetUpperBound(),
189+
Transforms: transforms,
190+
FilterBlockSizeLimit: sstable.AlwaysUseFilterBlock,
191+
Env: iio.readEnv,
192+
ReaderProvider: sstable.MakeTrivialReaderProvider(lt.readers[file.FileNum]),
193+
})
189194
if err != nil {
190195
return iterSet{}, errors.CombineErrors(err, set.CloseAll())
191196
}
192197
set.point = iter
193198
}
194199
if kinds.RangeDeletion() {
195-
rangeDelIter, err := lt.readers[file.FileNum].NewRawRangeDelIter(context.Background(), file.FragmentIterTransforms(), block.NoReadEnv)
200+
rangeDelIter, err := lt.readers[file.FileNum].NewRawRangeDelIter(ctx, file.FragmentIterTransforms(), block.NoReadEnv)
196201
if err != nil {
197202
return iterSet{}, errors.CombineErrors(err, set.CloseAll())
198203
}

‎merging_iter_test.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,22 +165,26 @@ func TestMergingIterDataDriven(t *testing.T) {
165165
fileNum base.FileNum
166166
)
167167
newIters :=
168-
func(_ context.Context, file *manifest.TableMetadata, opts *IterOptions, iio internalIterOpts, kinds iterKinds,
168+
func(ctx context.Context, file *manifest.TableMetadata, opts *IterOptions, iio internalIterOpts, kinds iterKinds,
169169
) (iterSet, error) {
170170
var set iterSet
171171
var err error
172172
r := readers[file.FileNum]
173173
if kinds.RangeDeletion() {
174-
set.rangeDeletion, err = r.NewRawRangeDelIter(context.Background(), sstable.NoFragmentTransforms, iio.readEnv)
174+
set.rangeDeletion, err = r.NewRawRangeDelIter(ctx, sstable.NoFragmentTransforms, iio.readEnv)
175175
if err != nil {
176176
return iterSet{}, errors.CombineErrors(err, set.CloseAll())
177177
}
178178
}
179179
if kinds.Point() {
180-
set.point, err = r.NewPointIter(
181-
context.Background(),
182-
sstable.NoTransforms,
183-
opts.GetLowerBound(), opts.GetUpperBound(), nil, sstable.AlwaysUseFilterBlock, iio.readEnv, sstable.MakeTrivialReaderProvider(r))
180+
set.point, err = r.NewPointIter(ctx, sstable.IterOptions{
181+
Lower: opts.GetLowerBound(),
182+
Upper: opts.GetUpperBound(),
183+
Transforms: sstable.NoTransforms,
184+
FilterBlockSizeLimit: sstable.AlwaysUseFilterBlock,
185+
Env: iio.readEnv,
186+
ReaderProvider: sstable.MakeTrivialReaderProvider(r),
187+
})
184188
if err != nil {
185189
return iterSet{}, errors.CombineErrors(err, set.CloseAll())
186190
}

‎sstable/block_property_test.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -986,9 +986,15 @@ func TestBlockProperties(t *testing.T) {
986986
} else if !ok {
987987
return "filter excludes entire table"
988988
}
989-
iter, err := r.NewPointIter(
990-
context.Background(),
991-
NoTransforms, lower, upper, filterer, NeverUseFilterBlock, block.ReadEnv{Stats: &stats, IterStats: nil}, MakeTrivialReaderProvider(r))
989+
iter, err := r.NewPointIter(context.Background(), IterOptions{
990+
Lower: lower,
991+
Upper: upper,
992+
Transforms: NoTransforms,
993+
Filterer: filterer,
994+
FilterBlockSizeLimit: NeverUseFilterBlock,
995+
Env: block.ReadEnv{Stats: &stats, IterStats: nil},
996+
ReaderProvider: MakeTrivialReaderProvider(r),
997+
})
992998
if err != nil {
993999
return err.Error()
9941000
}
@@ -1069,9 +1075,15 @@ func TestBlockProperties_BoundLimited(t *testing.T) {
10691075
} else if !ok {
10701076
return "filter excludes entire table"
10711077
}
1072-
iter, err := r.NewPointIter(
1073-
context.Background(),
1074-
NoTransforms, lower, upper, filterer, NeverUseFilterBlock, block.ReadEnv{Stats: &stats, IterStats: nil}, MakeTrivialReaderProvider(r))
1078+
iter, err := r.NewPointIter(context.Background(), IterOptions{
1079+
Lower: lower,
1080+
Upper: upper,
1081+
Transforms: NoTransforms,
1082+
Filterer: filterer,
1083+
FilterBlockSizeLimit: NeverUseFilterBlock,
1084+
Env: block.ReadEnv{Stats: &stats, IterStats: nil},
1085+
ReaderProvider: MakeTrivialReaderProvider(r),
1086+
})
10751087
if err != nil {
10761088
return err.Error()
10771089
}

‎sstable/random_test.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,13 @@ func runErrorInjectionTest(t *testing.T, seed int64) {
105105
if rng.IntN(2) == 1 {
106106
filterBlockSizeLimit = NeverUseFilterBlock
107107
}
108-
it, err := r.NewPointIter(
109-
context.Background(),
110-
NoTransforms,
111-
nil /* lower TODO */, nil, /* upper TODO */
112-
filterer,
113-
filterBlockSizeLimit,
114-
block.ReadEnv{Stats: &stats, IterStats: nil},
115-
MakeTrivialReaderProvider(r),
116-
)
108+
it, err := r.NewPointIter(context.Background(), IterOptions{
109+
Transforms: NoTransforms,
110+
Filterer: filterer,
111+
FilterBlockSizeLimit: filterBlockSizeLimit,
112+
Env: block.ReadEnv{Stats: &stats, IterStats: nil},
113+
ReaderProvider: MakeTrivialReaderProvider(r),
114+
})
117115
require.NoError(t, err)
118116
defer it.Close()
119117

‎sstable/reader.go‎

Lines changed: 39 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -77,23 +77,23 @@ func (r *Reader) Close() error {
7777
return nil
7878
}
7979

80+
// IterOptions defines options for configuring a sstable pointer iterator.
81+
type IterOptions struct {
82+
Lower, Upper []byte
83+
Transforms IterTransforms
84+
Filterer *BlockPropertiesFilterer
85+
FilterBlockSizeLimit FilterBlockSizeLimit
86+
Env block.ReadEnv
87+
ReaderProvider valblk.ReaderProvider
88+
}
89+
8090
// NewPointIter returns an iterator for the point keys in the table.
8191
//
8292
// If transform.HideObsoletePoints is set, the callee assumes that filterer
8393
// already includes obsoleteKeyBlockPropertyFilter. The caller can satisfy this
8494
// contract by first calling TryAddBlockPropertyFilterForHideObsoletePoints.
85-
func (r *Reader) NewPointIter(
86-
ctx context.Context,
87-
transforms IterTransforms,
88-
lower, upper []byte,
89-
filterer *BlockPropertiesFilterer,
90-
filterBlockSizeLimit FilterBlockSizeLimit,
91-
env block.ReadEnv,
92-
rp valblk.ReaderProvider,
93-
) (Iterator, error) {
94-
return r.newPointIter(
95-
ctx, transforms, lower, upper, filterer, filterBlockSizeLimit,
96-
env, rp, nil)
95+
func (r *Reader) NewPointIter(ctx context.Context, opts IterOptions) (Iterator, error) {
96+
return r.newPointIter(ctx, opts, nil /* vState */)
9797
}
9898

9999
// TryAddBlockPropertyFilterForHideObsoletePoints is expected to be called
@@ -113,14 +113,7 @@ func (r *Reader) TryAddBlockPropertyFilterForHideObsoletePoints(
113113
}
114114

115115
func (r *Reader) newPointIter(
116-
ctx context.Context,
117-
transforms IterTransforms,
118-
lower, upper []byte,
119-
filterer *BlockPropertiesFilterer,
120-
filterBlockSizeLimit FilterBlockSizeLimit,
121-
env block.ReadEnv,
122-
rp valblk.ReaderProvider,
123-
vState *virtualState,
116+
ctx context.Context, opts IterOptions, vState *virtualState,
124117
) (Iterator, error) {
125118
// NB: pebble.fileCache wraps the returned iterator with one which performs
126119
// reference counting on the Reader, preventing the Reader from being closed
@@ -130,22 +123,18 @@ func (r *Reader) newPointIter(
130123
if r.Properties.IndexType == twoLevelIndex {
131124
if r.tableFormat.BlockColumnar() {
132125
res, err = newColumnBlockTwoLevelIterator(
133-
ctx, r, vState, transforms, lower, upper, filterer, filterBlockSizeLimit,
134-
env, rp)
126+
ctx, r, vState, opts)
135127
} else {
136128
res, err = newRowBlockTwoLevelIterator(
137-
ctx, r, vState, transforms, lower, upper, filterer, filterBlockSizeLimit,
138-
env, rp)
129+
ctx, r, vState, opts)
139130
}
140131
} else {
141132
if r.tableFormat.BlockColumnar() {
142133
res, err = newColumnBlockSingleLevelIterator(
143-
ctx, r, vState, transforms, lower, upper, filterer, filterBlockSizeLimit,
144-
env, rp)
134+
ctx, r, vState, opts)
145135
} else {
146136
res, err = newRowBlockSingleLevelIterator(
147-
ctx, r, vState, transforms, lower, upper, filterer, filterBlockSizeLimit,
148-
env, rp)
137+
ctx, r, vState, opts)
149138
}
150139
}
151140
if err != nil {
@@ -165,9 +154,16 @@ func (r *Reader) newPointIter(
165154
func (r *Reader) NewIter(transforms IterTransforms, lower, upper []byte) (Iterator, error) {
166155
// TODO(radu): we should probably not use bloom filters in this case, as there
167156
// likely isn't a cache set up.
168-
return r.NewPointIter(
169-
context.TODO(), transforms, lower, upper, nil, AlwaysUseFilterBlock,
170-
block.NoReadEnv, MakeTrivialReaderProvider(r))
157+
opts := IterOptions{
158+
Lower: lower,
159+
Upper: upper,
160+
Transforms: transforms,
161+
Filterer: nil,
162+
FilterBlockSizeLimit: AlwaysUseFilterBlock,
163+
Env: block.NoReadEnv,
164+
ReaderProvider: MakeTrivialReaderProvider(r),
165+
}
166+
return r.NewPointIter(context.TODO(), opts)
171167
}
172168

173169
// NewCompactionIter returns an iterator similar to NewIter but it also increments
@@ -185,42 +181,40 @@ func (r *Reader) newCompactionIter(
185181
if vState != nil && vState.isSharedIngested {
186182
transforms.HideObsoletePoints = true
187183
}
184+
ctx := context.Background()
185+
opts := IterOptions{
186+
Transforms: transforms,
187+
Filterer: nil,
188+
FilterBlockSizeLimit: NeverUseFilterBlock,
189+
Env: env,
190+
ReaderProvider: rp,
191+
}
188192

189193
if r.Properties.IndexType == twoLevelIndex {
190194
if !r.tableFormat.BlockColumnar() {
191-
i, err := newRowBlockTwoLevelIterator(
192-
context.Background(),
193-
r, vState, transforms, nil /* lower */, nil /* upper */, nil,
194-
NeverUseFilterBlock, env, rp)
195+
i, err := newRowBlockTwoLevelIterator(ctx, r, vState, opts)
195196
if err != nil {
196197
return nil, err
197198
}
198199
i.SetupForCompaction()
199200
return i, nil
200201
}
201-
i, err := newColumnBlockTwoLevelIterator(
202-
context.Background(),
203-
r, vState, transforms, nil /* lower */, nil /* upper */, nil,
204-
NeverUseFilterBlock, env, rp)
202+
i, err := newColumnBlockTwoLevelIterator(ctx, r, vState, opts)
205203
if err != nil {
206204
return nil, err
207205
}
208206
i.SetupForCompaction()
209207
return i, nil
210208
}
211209
if !r.tableFormat.BlockColumnar() {
212-
i, err := newRowBlockSingleLevelIterator(
213-
context.Background(), r, vState, transforms, nil /* lower */, nil, /* upper */
214-
nil, NeverUseFilterBlock, env, rp)
210+
i, err := newRowBlockSingleLevelIterator(ctx, r, vState, opts)
215211
if err != nil {
216212
return nil, err
217213
}
218214
i.SetupForCompaction()
219215
return i, nil
220216
}
221-
i, err := newColumnBlockSingleLevelIterator(
222-
context.Background(), r, vState, transforms, nil /* lower */, nil, /* upper */
223-
nil, NeverUseFilterBlock, env, rp)
217+
i, err := newColumnBlockSingleLevelIterator(ctx, r, vState, opts)
224218
if err != nil {
225219
return nil, err
226220
}

‎sstable/reader_common.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,7 @@ type CommonReader interface {
2525
ctx context.Context, transforms FragmentIterTransforms, env block.ReadEnv,
2626
) (keyspan.FragmentIterator, error)
2727

28-
NewPointIter(
29-
ctx context.Context,
30-
transforms IterTransforms,
31-
lower, upper []byte,
32-
filterer *BlockPropertiesFilterer,
33-
filterBlockSizeLimit FilterBlockSizeLimit,
34-
env block.ReadEnv,
35-
rp valblk.ReaderProvider,
36-
) (Iterator, error)
28+
NewPointIter(ctx context.Context, opts IterOptions) (Iterator, error)
3729

3830
NewCompactionIter(
3931
transforms IterTransforms,

0 commit comments

Comments
 (0)