Skip to content

Commit e542217

Browse files
committed
fix: assert errors when relative positioning functions are called
For functions that require positioning, assert errors instead of attempting to load index again. The caller must comply with the contract.
1 parent 1ba33f5 commit e542217

File tree

4 files changed

+31
-12
lines changed

4 files changed

+31
-12
lines changed

sstable/reader_iter_single_lvl.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -439,9 +439,8 @@ func (i *singleLevelIterator[I, PI, P, PD]) SetContext(ctx context.Context) {
439439
// unpositioned. If unsuccessful, it sets i.err to any error encountered, which
440440
// may be nil if we have simply exhausted the entire table.
441441
func (i *singleLevelIterator[I, PI, P, PD]) loadDataBlock(dir int8) loadBlockResult {
442-
if !i.ensureIndexLoaded() {
443-
// Ensure the data block iterator is invalidated
444-
PD(&i.data).Invalidate()
442+
if !i.indexLoaded {
443+
i.err = errors.AssertionFailedf("index block is not loaded")
445444
return loadBlockFailed
446445
}
447446
if !PI(&i.index).Valid() {
@@ -608,7 +607,7 @@ func (i *singleLevelIterator[I, PI, D, PD]) trySeekLTUsingPrevWithinBlock(
608607
key []byte,
609608
) (kv *base.InternalKV, done bool) {
610609
kv = PD(&i.data).KV()
611-
for j := 0; j < numStepsBeforeSeek; j++ {
610+
for range numStepsBeforeSeek {
612611
curKeyCmp := i.cmp(kv.K.UserKey, key)
613612
if curKeyCmp < 0 {
614613
if i.blockLower != nil && i.cmp(kv.K.UserKey, i.blockLower) < 0 {
@@ -1284,7 +1283,8 @@ func (i *singleLevelIterator[I, PI, D, PD]) NextPrefix(succKey []byte) *base.Int
12841283
// Did not find prefix in the existing data block. This is the slow-path
12851284
// where we effectively seek the iterator.
12861285
// The key is likely to be in the next data block, so try one step.
1287-
if !i.ensureIndexLoaded() {
1286+
if !i.indexLoaded {
1287+
i.err = errors.AssertionFailedf("index block is not loaded")
12881288
return nil
12891289
}
12901290
if !PI(&i.index).Next() {
@@ -1358,7 +1358,8 @@ func (i *singleLevelIterator[I, PI, D, PD]) Prev() *base.InternalKV {
13581358

13591359
func (i *singleLevelIterator[I, PI, D, PD]) skipForward() *base.InternalKV {
13601360
for {
1361-
if !i.ensureIndexLoaded() {
1361+
if !i.indexLoaded {
1362+
i.err = errors.AssertionFailedf("index block is not loaded")
13621363
return nil
13631364
}
13641365
if !PI(&i.index).Next() {
@@ -1439,7 +1440,8 @@ func (i *singleLevelIterator[I, PI, D, PD]) skipForward() *base.InternalKV {
14391440

14401441
func (i *singleLevelIterator[I, PI, D, PD]) skipBackward() *base.InternalKV {
14411442
for {
1442-
if !i.ensureIndexLoaded() {
1443+
if !i.indexLoaded {
1444+
i.err = errors.AssertionFailedf("index block is not loaded")
14431445
return nil
14441446
}
14451447
if !PI(&i.index).Prev() {

sstable/reader_iter_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,9 +444,12 @@ func TestLazyLoadingStressOperations(t *testing.T) {
444444
require.NoError(t, err)
445445
defer iter.Close()
446446

447+
// Ensure iterator is positioned before operations
448+
iter.First()
449+
447450
// Perform many repeated operations
448451
const numOperations = 5000
449-
for i := 0; i < numOperations; i++ {
452+
for i := range numOperations {
450453
switch i % 5 {
451454
case 0:
452455
iter.First()
@@ -523,7 +526,10 @@ func performConcurrentOps(reader *Reader, keys [][]byte, numOps int, goroutineID
523526
}
524527
defer iter.Close()
525528

526-
for i := 0; i < numOps; i++ {
529+
// Ensure iterator is positioned before any operations
530+
iter.First()
531+
532+
for i := range numOps {
527533
// Vary operations based on goroutine ID and iteration
528534
switch (goroutineID + i) % 4 {
529535
case 0:

sstable/reader_iter_two_lvl.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,8 @@ func (i *twoLevelIterator[I, PI, D, PD]) NextPrefix(succKey []byte) *base.Intern
854854
// Did not find prefix in the existing second-level index block. This is the
855855
// slow-path where we seek the iterator.
856856

857-
if !i.ensureTopLevelIndexLoaded() {
857+
if !i.topLevelIndexLoaded {
858+
i.secondLevel.err = errors.AssertionFailedf("top level index is not loaded")
858859
return nil
859860
}
860861

@@ -905,7 +906,8 @@ func (i *twoLevelIterator[I, PI, D, PD]) skipForward() *base.InternalKV {
905906
return nil
906907
}
907908

908-
if !i.ensureTopLevelIndexLoaded() {
909+
if !i.topLevelIndexLoaded {
910+
i.secondLevel.err = errors.AssertionFailedf("top level index is not loaded")
909911
return nil
910912
}
911913

@@ -987,7 +989,8 @@ func (i *twoLevelIterator[I, PI, D, PD]) skipBackward() *base.InternalKV {
987989
return nil
988990
}
989991

990-
if !i.ensureTopLevelIndexLoaded() {
992+
if !i.topLevelIndexLoaded {
993+
i.secondLevel.err = errors.AssertionFailedf("top level index is not loaded")
991994
return nil
992995
}
993996

sstable/reader_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,8 @@ func (rw *readerWorkload) handleInvalid(callType readCallType, iter Iterator) *b
11391139
switch callType {
11401140
case SeekGE, Next, Last:
11411141
if len(rw.seekKeyAfterInvalid) == 0 {
1142+
// Ensure iterator is positioned before calling Prev()
1143+
iter.Last()
11421144
return iter.Prev()
11431145
}
11441146
return iter.SeekLT(rw.seekKeyAfterInvalid, base.SeekLTFlagsNone)
@@ -1149,6 +1151,8 @@ func (rw *readerWorkload) handleInvalid(callType readCallType, iter Iterator) *b
11491151
return iter.SeekLT(rw.seekKeyAfterInvalid, base.SeekLTFlagsNone)
11501152
case SeekLT, Prev, First:
11511153
if len(rw.seekKeyAfterInvalid) == 0 {
1154+
// Ensure iterator is positioned before calling Next()
1155+
iter.First()
11521156
return iter.Next()
11531157
}
11541158
return iter.SeekGE(rw.seekKeyAfterInvalid, base.SeekGEFlagsNone)
@@ -1386,6 +1390,10 @@ func TestRandomizedPrefixSuffixRewriter(t *testing.T) {
13861390
iter, cleanup := createIter(fileName, syntheticSuffix, syntheticPrefix)
13871391
defer cleanup()
13881392

1393+
// Position both iterators to ensure index blocks are loaded
1394+
eIter.First()
1395+
iter.First()
1396+
13891397
w := createReadWorkload(t, rng, callCount, ks, maxTs+2, syntheticPrefix)
13901398
workloadChecker := checker{
13911399
t: t,

0 commit comments

Comments
 (0)