Skip to content

Commit 2b51428

Browse files
fix(lore): discard stale index snapshots that raced a concurrent splice (#102)
LoadFromDB read meta.vector_epoch after scanning rows and installed the snapshot unconditionally. Two broken interleavings under concurrent writers: a commit landing between the row scan and the epoch read got a pre-commit snapshot stamped with a post-commit epoch, so CheckAndReload trusted stale state indefinitely; and a snapshot built before a concurrent Splice clobbered the spliced vector on install. Read the epoch before the scan so it lower-bounds the snapshot, and discard installs whose snapshot epoch is older than cachedEpoch. Co-authored-by: Kunal Lanjewar <5488221+kunallanjewar@users.noreply.github.com>
1 parent a946d52 commit 2b51428

2 files changed

Lines changed: 67 additions & 7 deletions

File tree

internal/lore/embed/index.go

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,17 @@ func (i *Index) LoadFromDB(ctx context.Context, db *sql.DB) (int, error) {
231231
}
232232
}
233233

234+
// Read the epoch BEFORE scanning rows so it is a lower bound for
235+
// the snapshot: every commit visible to the SELECT below happened
236+
// at or after this epoch. Reading it after the scan inverts that
237+
// bound and lets a concurrent writer's commit land between the two
238+
// reads, stamping a pre-commit row snapshot with a post-commit
239+
// epoch; CheckAndReload then trusts the stale snapshot forever.
240+
epoch, err := readEpoch(ctx, db, i.corpus.MetaKey(FieldVectorEpoch))
241+
if err != nil {
242+
return 0, err
243+
}
244+
234245
// Stream the corpus's vector table in one SELECT. entry_id is the
235246
// PK so the default ordering is by id; that is fine for parallel-
236247
// slice layout and gives tests a deterministic load order.
@@ -290,14 +301,22 @@ func (i *Index) LoadFromDB(ctx context.Context, db *sql.DB) (int, error) {
290301
return 0, fmt.Errorf("embed/index: iterate %s: %w", i.corpus.VectorTable(), err)
291302
}
292303

293-
// Refresh epoch from meta using the corpus's meta key. A missing
294-
// row (fresh DB) yields 0.
295-
epoch, err := readEpoch(ctx, db, i.corpus.MetaKey(FieldVectorEpoch))
296-
if err != nil {
297-
return 0, err
298-
}
299-
300304
i.bindMu.Lock()
305+
if i.loaded && epoch < i.cachedEpoch {
306+
// A concurrent Splice advanced the index past this snapshot
307+
// (its writer committed after our epoch read). Installing the
308+
// snapshot would silently drop that newer vector. Discard;
309+
// the in-memory state is already at least as fresh as the DB
310+
// state this load observed.
311+
cached := i.cachedEpoch
312+
i.bindMu.Unlock()
313+
i.logger.Debug("embed/index: discarded stale load snapshot",
314+
"snapshot_epoch", epoch,
315+
"cached_epoch", cached,
316+
"rows", len(newVecs),
317+
)
318+
return len(newVecs), nil
319+
}
301320
i.vectors = newVecs
302321
i.entries = newEntries
303322
i.byEntry = newByID

internal/lore/embed/index_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,47 @@ func TestIndex_CheckAndReload_ReloadsOnEpochBump(t *testing.T) {
316316
}
317317
}
318318

319+
// TestIndex_LoadFromDB_DiscardsSnapshotOlderThanSplice: a load whose
320+
// DB snapshot predates a concurrent Splice must not clobber the newer
321+
// in-memory state. Deterministic reconstruction of the interleaving
322+
// behind the Test_ConcurrentInscribeAndAppraise lost-Splice flake: the
323+
// reader's SELECT ran before the writer's commit, the writer's Splice
324+
// landed first, then the reader's stale snapshot tried to install.
325+
func TestIndex_LoadFromDB_DiscardsSnapshotOlderThanSplice(t *testing.T) {
326+
ctx := context.Background()
327+
db := openEmbedTestDB(t)
328+
mustSeedEntry(t, db, 1, "e1")
329+
mustInsertVec(t, db, 1, canonModelID, Quantize(deterministicUnitVec(1)))
330+
mustSetEpoch(t, db, 1)
331+
332+
idx := NewIndex(LoreCorpus{}, canonModelID)
333+
if _, err := idx.LoadFromDB(ctx, db); err != nil {
334+
t.Fatalf("LoadFromDB: %v", err)
335+
}
336+
337+
// Writer path: Splice entry 2 at epoch 2. The DB row + epoch bump
338+
// are deliberately NOT written, so the DB still holds the epoch-1
339+
// single-row state, i.e. the stale snapshot a slow reader observes
340+
// when its reads ran before the writer's commit.
341+
if err := idx.Splice(2, Quantize(deterministicUnitVec(2)), 2); err != nil {
342+
t.Fatalf("Splice: %v", err)
343+
}
344+
if idx.Len() != 2 {
345+
t.Fatalf("post-splice Len = %d, want 2", idx.Len())
346+
}
347+
348+
// The late reader's load must be discarded, not installed.
349+
if _, err := idx.LoadFromDB(ctx, db); err != nil {
350+
t.Fatalf("LoadFromDB (stale): %v", err)
351+
}
352+
if idx.Len() != 2 {
353+
t.Fatalf("stale snapshot clobbered the index: Len = %d, want 2", idx.Len())
354+
}
355+
if idx.Epoch() != 2 {
356+
t.Fatalf("stale snapshot regressed epoch: %d, want 2", idx.Epoch())
357+
}
358+
}
359+
319360
// TestIndex_Splice_InsertNew: Splice on an unknown entry_id appends a
320361
// new slot and bumps cachedEpoch.
321362
func TestIndex_Splice_InsertNew(t *testing.T) {

0 commit comments

Comments
 (0)