Skip to content

Commit 2b15eec

Browse files
fix(table): advance ConcatIterator past fully version-pruned first table
Block-level version skipping (SetVersionBounds) can leave a table iterator invalid after Rewind when every block falls outside the window (seekToFirst/seekToLast set io.EOF). ConcatIterator.Rewind only positioned on the first table without looping, so if that table was fully pruned the ConcatIterator — and its MergeIterator level — looked exhausted and stranded every later table, silently dropping in-window entries (reachable at L1+ where a level uses a ConcatIterator; e.g. a table with old keys in early blocks and recent keys in later blocks, scanned with a mid version window). Add skipInvalidTables(), called from Rewind, mirroring the empty-table loop already in Next(). No-op when version bounds are disabled. Seek is unaffected: seekFrom does not version-skip blocks and the binary search guarantees the chosen table holds a key >= the seek key. Adds TestConcatIteratorSkipsFullyPrunedFirstTable (fails without the fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e4d44cf commit 2b15eec

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

table/iterator.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,27 @@ func (s *ConcatIterator) Rewind() {
559559
s.setIdx(len(s.iters) - 1)
560560
}
561561
s.cur.Rewind()
562+
s.skipInvalidTables()
563+
}
564+
565+
// skipInvalidTables advances to the next table (in iteration direction) whose
566+
// iterator is valid, after Rewind/Seek positioned us on one that is not. Version-range
567+
// block skipping (SetVersionBounds) can leave a table iterator invalid when all its
568+
// blocks fall outside the window; without this, the ConcatIterator — and hence its
569+
// MergeIterator level — would look exhausted and strand later tables' in-window keys.
570+
// Mirrors the empty-table loop in Next(). When version bounds are disabled this is a
571+
// no-op, since a non-empty table is always valid after Rewind.
572+
func (s *ConcatIterator) skipInvalidTables() {
573+
for s.cur != nil && !s.cur.Valid() {
574+
if s.options&REVERSED == 0 {
575+
s.setIdx(s.idx + 1)
576+
} else {
577+
s.setIdx(s.idx - 1)
578+
}
579+
if s.cur != nil {
580+
s.cur.Rewind()
581+
}
582+
}
562583
}
563584

564585
// Valid implements y.Interface

table/version_range_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,38 @@ func TestOldFormatTableNeverSkipped(t *testing.T) {
344344
}
345345
require.Equal(t, 50, count)
346346
}
347+
348+
// TestConcatIteratorSkipsFullyPrunedFirstTable is a regression test for the
349+
// ConcatIterator stranding later tables when the first table's blocks are all
350+
// outside the version window. Table 1 (keys a1,a2) holds only old versions and is
351+
// fully block-pruned by the window [5,10]; table 2 (key m1) holds an in-window
352+
// version. A version-bounded ConcatIterator's Rewind must still surface table 2's key
353+
// instead of treating the whole level as exhausted, which dropped in-window data
354+
// before the fix. (Seek can't hit this: seekFrom does not version-skip blocks, and the
355+
// binary search guarantees the chosen table holds a key >= the seek key.)
356+
func TestConcatIteratorSkipsFullyPrunedFirstTable(t *testing.T) {
357+
lower, upper := uint64(5), uint64(10)
358+
359+
t1 := buildVersionTable(t, []vkv{
360+
{key: "a1", version: 1, value: "x"},
361+
{key: "a2", version: 2, value: "x"},
362+
}, 512)
363+
defer func() { require.NoError(t, t1.DecrRef()) }()
364+
t2 := buildVersionTable(t, []vkv{
365+
{key: "m1", version: 7, value: "y"},
366+
}, 512)
367+
defer func() { require.NoError(t, t2.DecrRef()) }()
368+
369+
require.Less(t, t1.MaxVersion(), lower, "table1 must lie entirely below the window")
370+
371+
ci := NewConcatIterator([]*Table{t1, t2}, 0)
372+
ci.SetVersionBounds(lower, upper)
373+
defer func() { require.NoError(t, ci.Close()) }()
374+
375+
var got []string
376+
for ci.Rewind(); ci.Valid(); ci.Next() {
377+
got = append(got, string(y.ParseKey(ci.Key())))
378+
}
379+
require.Equal(t, []string{"m1"}, got,
380+
"Rewind must surface the in-window key in table 2 despite table 1 being fully pruned")
381+
}

0 commit comments

Comments
 (0)