Skip to content

Commit f1dc6da

Browse files
committed
fix(postgres): scope generation witnesses to each watcher
1 parent 0c8cee6 commit f1dc6da

3 files changed

Lines changed: 157 additions & 35 deletions

File tree

internal/postgres/vector_admin.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,25 @@ func vectorGenerationMachines(
129129
if err := rows.Scan(&m); err != nil {
130130
return nil, fmt.Errorf("scanning generation machine: %w", err)
131131
}
132-
machines = append(machines, m)
132+
display := vectorGenerationMachineDisplayName(m)
133+
if len(machines) > 0 && machines[len(machines)-1] == display {
134+
continue
135+
}
136+
machines = append(machines, display)
133137
}
134138
if err := rows.Err(); err != nil {
135139
return nil, fmt.Errorf("iterating generation machines: %w", err)
136140
}
137141
return machines, nil
138142
}
139143

144+
func vectorGenerationMachineDisplayName(raw string) string {
145+
if i := strings.Index(raw, "|"+pushMarkerKeyPrefix); i >= 0 {
146+
return raw[:i]
147+
}
148+
return raw
149+
}
150+
140151
// DropVectorGeneration removes a generation and all of its data: its chunk
141152
// table, its vector_push_state and vector_generation_machines rows, and its
142153
// vector_generations row, then prunes vector_documents rows referenced by no

internal/postgres/vector_push.go

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,12 @@ const vectorProgressStride = 2000
123123
// the schema-qualified halfvec type. The type must be qualified because the
124124
// connection's search_path is the target schema only, while pgvector's types
125125
// live in whichever schema first installed the extension.
126-
// machineRecorded reports whether this machine's push record already existed
127-
// for the generation before this push touched it: recreating the vector
128-
// tables restarts the id sequence, so a reused id can satisfy the memo
129-
// comparison while the recreated generation is empty, and the machine record
130-
// — wiped in the same reset — is the witness that survives id reuse.
126+
// machineRecorded reports whether this pusher's scoped witness record already
127+
// existed for the generation before this push touched it: recreating the
128+
// vector tables restarts the id sequence, so a reused id can satisfy the memo
129+
// comparison while the recreated generation is empty, and the witness record
130+
// — keyed by local push marker plus sync/filter scope and wiped in the same
131+
// reset — is what survives id reuse safely.
131132
type vectorGeneration struct {
132133
id int64
133134
halfvecType string
@@ -270,26 +271,29 @@ func (s *Sync) pushVectors(
270271
res.Skipped, res.SkippedReason = true, unavailable
271272
return res, nil
272273
}
274+
witnessKey, err := s.vectorGenerationWitnessKey()
275+
if err != nil {
276+
return res, err
277+
}
273278
// A scoped push writes only its changed sessions' chunks, which is safe
274279
// only within the exact generation instance this process last reconciled
275280
// generation-wide. Two signals identify that instance. The PG generation
276281
// id catches every recreation that keeps the tables: row deletion never
277282
// resets the sequence, so a re-embed (new fingerprint), reset, or admin
278283
// drop yields a new id, whoever recreates it. Recreating the tables
279284
// themselves restarts the sequence, so a reused id can match a stale
280-
// memo; this machine's push record, wiped in the same reset and written
281-
// only after a clean generation-wide reconciliation, is the witness for
282-
// that case — a scoped push always follows a generation-wide push in the
283-
// same process, which recorded it. Either signal failing means the prior
284-
// reconciliation no longer covers this generation and scoping would
285-
// leave search reading an incomplete one until the interval floor.
285+
// memo; this pusher's scoped witness record, wiped in the same reset and
286+
// written only after a clean generation-wide reconciliation, is the
287+
// witness for that case. Either signal failing means the prior
288+
// reconciliation no longer covers this generation and scoping would leave
289+
// search reading an incomplete one until the interval floor.
286290
// Promote to a generation-wide read. A zero memo means no reconciliation
287291
// to trust yet, so the reconcile bit already forces this push
288292
// generation-wide and the id check must not fire.
289293
requestedScope := scope
290294
var resolved vectorGeneration
291295
if scope != nil {
292-
initialProbe, found, err := s.lookupVectorGeneration(ctx, gen.Fingerprint)
296+
initialProbe, found, err := s.lookupVectorGeneration(ctx, gen.Fingerprint, witnessKey)
293297
if err != nil {
294298
return res, err
295299
}
@@ -298,8 +302,8 @@ func (s *Sync) pushVectors(
298302
gen.Fingerprint)
299303
scope = nil
300304
} else if !initialProbe.machineRecorded {
301-
log.Printf("vector push: no prior push record for machine %q against generation %d; promoting scoped push to generation-wide reconciliation",
302-
s.machine, initialProbe.id)
305+
log.Printf("vector push: no prior scoped witness against generation %d; promoting scoped push to generation-wide reconciliation",
306+
initialProbe.id)
303307
scope = nil
304308
} else if lastReconciledGeneration != 0 &&
305309
initialProbe.id != lastReconciledGeneration {
@@ -312,7 +316,7 @@ func (s *Sync) pushVectors(
312316
s.afterVectorGenerationLookup = nil
313317
hook()
314318
}
315-
currentProbe, found, err := s.lookupVectorGeneration(ctx, gen.Fingerprint)
319+
currentProbe, found, err := s.lookupVectorGeneration(ctx, gen.Fingerprint, witnessKey)
316320
if err != nil {
317321
return res, err
318322
}
@@ -321,8 +325,8 @@ func (s *Sync) pushVectors(
321325
gen.Fingerprint)
322326
scope = nil
323327
} else if !currentProbe.machineRecorded {
324-
log.Printf("vector push: no prior push record for machine %q against generation %d before scoped reconciliation; promoting scoped push to generation-wide reconciliation",
325-
s.machine, currentProbe.id)
328+
log.Printf("vector push: no prior scoped witness against generation %d before scoped reconciliation; promoting scoped push to generation-wide reconciliation",
329+
currentProbe.id)
326330
scope = nil
327331
} else if currentProbe.id != initialProbe.id {
328332
log.Printf("vector push: active generation id changed from %d to %d before scoped reconciliation; promoting scoped push to generation-wide reconciliation",
@@ -358,7 +362,7 @@ func (s *Sync) pushVectors(
358362
gen = export.Generation()
359363
}
360364
if scope == nil {
361-
resolved, err = s.resolveVectorGeneration(ctx, gen)
365+
resolved, err = s.resolveVectorGeneration(ctx, gen, witnessKey)
362366
if err != nil {
363367
if s.skipVectorsOnPrivilegeError(err, &res) {
364368
return res, nil
@@ -413,7 +417,7 @@ func (s *Sync) pushVectors(
413417
ctx, full, nil, lastReconciledGeneration, failedSessions, onProgress,
414418
)
415419
}
416-
finalProbe, found, err := s.lookupVectorGeneration(ctx, gen.Fingerprint)
420+
finalProbe, found, err := s.lookupVectorGeneration(ctx, gen.Fingerprint, witnessKey)
417421
if err != nil {
418422
return res, err
419423
}
@@ -425,8 +429,8 @@ func (s *Sync) pushVectors(
425429
}
426430
if !finalProbe.machineRecorded {
427431
return retryGenerationWide(
428-
"vector push: no prior push record for machine %q against generation %d after scoped reconciliation; retrying generation-wide",
429-
s.machine, finalProbe.id,
432+
"vector push: no prior scoped witness against generation %d after scoped reconciliation; retrying generation-wide",
433+
finalProbe.id,
430434
)
431435
}
432436
if finalProbe.id != resolved.id {
@@ -437,7 +441,7 @@ func (s *Sync) pushVectors(
437441
}
438442
}
439443
if scope == nil && res.SessionsDeferred == 0 {
440-
if err := s.recordVectorGenerationMachine(ctx, resolved.id); err != nil {
444+
if err := s.recordVectorGenerationMachine(ctx, resolved.id, witnessKey); err != nil {
441445
return res, err
442446
}
443447
}
@@ -448,7 +452,7 @@ func (s *Sync) pushVectors(
448452
}
449453

450454
func (s *Sync) lookupVectorGeneration(
451-
ctx context.Context, fingerprint string,
455+
ctx context.Context, fingerprint, witnessKey string,
452456
) (vectorGeneration, bool, error) {
453457
genID, _, ok, err := LookupVectorGeneration(ctx, s.pg, fingerprint)
454458
if err != nil {
@@ -466,7 +470,7 @@ func (s *Sync) lookupVectorGeneration(
466470
SELECT EXISTS (
467471
SELECT 1 FROM vector_generation_machines
468472
WHERE generation_id = $1 AND machine = $2)`,
469-
genID, s.machine).Scan(&machineRecorded); err != nil {
473+
genID, witnessKey).Scan(&machineRecorded); err != nil {
470474
return vectorGeneration{}, false, fmt.Errorf(
471475
"reading vector push machine record: %w", err,
472476
)
@@ -801,11 +805,11 @@ func vectorOutOfScopeQuery(
801805
// against it. Its id is the generation
802806
// instance's identity, which a scoped push compares against the last one it
803807
// reconciled generation-wide to decide whether to promote; whether this
804-
// machine's push record predated this call is captured first, because that
805-
// record is the incarnation witness the promotion check falls back on when a
806-
// recreated id sequence hands the new generation the memoized id.
808+
// scoped witness predated this call is captured first, because that record is
809+
// the incarnation witness the promotion check falls back on when a recreated
810+
// id sequence hands the new generation the memoized id.
807811
func (s *Sync) resolveVectorGeneration(
808-
ctx context.Context, gen VectorGenerationInfo,
812+
ctx context.Context, gen VectorGenerationInfo, witnessKey string,
809813
) (vectorGeneration, error) {
810814
genID, err := ensureVectorGeneration(
811815
ctx, s.pg, gen.Fingerprint, gen.Model, gen.Dimension,
@@ -825,7 +829,7 @@ func (s *Sync) resolveVectorGeneration(
825829
SELECT EXISTS (
826830
SELECT 1 FROM vector_generation_machines
827831
WHERE generation_id = $1 AND machine = $2)`,
828-
genID, s.machine).Scan(&machineRecorded); err != nil {
832+
genID, witnessKey).Scan(&machineRecorded); err != nil {
829833
return vectorGeneration{}, fmt.Errorf("reading vector push machine record: %w", err)
830834
}
831835
return vectorGeneration{
@@ -835,14 +839,22 @@ SELECT EXISTS (
835839
}, nil
836840
}
837841

842+
func (s *Sync) vectorGenerationWitnessKey() (string, error) {
843+
markerID, err := s.pushMarkerID()
844+
if err != nil {
845+
return "", err
846+
}
847+
return s.machine + "|" + s.pushMarkerMetadataKey(pushMarkerKeyPrefix, markerID), nil
848+
}
849+
838850
func (s *Sync) recordVectorGenerationMachine(
839-
ctx context.Context, genID int64,
851+
ctx context.Context, genID int64, witnessKey string,
840852
) error {
841853
if _, err := s.pg.ExecContext(ctx, `
842854
INSERT INTO vector_generation_machines (generation_id, machine, last_push_at)
843855
VALUES ($1, $2, now())
844856
ON CONFLICT (generation_id, machine) DO UPDATE SET last_push_at = now()`,
845-
genID, s.machine); err != nil {
857+
genID, witnessKey); err != nil {
846858
return fmt.Errorf("recording vector push machine: %w", err)
847859
}
848860
return nil

internal/postgres/vector_push_pg_test.go

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,8 @@ func TestVectorPushDeferredFullPassDoesNotRecordMachineWitness(t *testing.T) {
787787
require.NoError(t, err, "baseline Push")
788788
genID := res.Vectors.GenerationID
789789
require.NotZero(t, genID)
790+
witnessKey, err := sync.vectorGenerationWitnessKey()
791+
require.NoError(t, err, "vectorGenerationWitnessKey")
790792
_, err = pg.Exec(
791793
`DELETE FROM vector_generation_machines WHERE generation_id = $1`, genID,
792794
)
@@ -805,7 +807,7 @@ func TestVectorPushDeferredFullPassDoesNotRecordMachineWitness(t *testing.T) {
805807
assert.Equal(t, 1, vres.SessionsPushed)
806808
assert.Equal(t, 0, countRows(t, pg, `
807809
SELECT COUNT(*) FROM vector_generation_machines
808-
WHERE generation_id = $1 AND machine = $2`, genID, "test-machine"),
810+
WHERE generation_id = $1 AND machine = $2`, genID, witnessKey),
809811
"deferred generation-wide work must not witness the generation")
810812

811813
src.genScopes = nil
@@ -823,14 +825,109 @@ SELECT COUNT(*) FROM vector_generation_machines
823825
"the promoted retry repairs B's deferred vector state")
824826
assert.Equal(t, 1, countRows(t, pg, `
825827
SELECT COUNT(*) FROM vector_generation_machines
826-
WHERE generation_id = $1 AND machine = $2`, genID, "test-machine"),
828+
WHERE generation_id = $1 AND machine = $2`, genID, witnessKey),
827829
"the clean generation-wide retry records the witness")
828830
assert.Equal(t, 1, countRows(t, pg, `
829831
SELECT COUNT(*) FROM vector_push_state
830832
WHERE generation_id = $1 AND session_id = 'B' AND doc_agg_hash = 'b2'`, genID),
831833
"the promoted retry must repair the deferred out-of-scope session")
832834
}
833835

836+
// TestVectorPushFilteredWitnessDoesNotCrossScopesAfterTableRecreation pins the
837+
// filtered-watcher incarnation bug: after vector tables are recreated on the
838+
// same machine, one filter's generation-wide witness must not let another
839+
// filter trust a reused generation id for a scoped push.
840+
func TestVectorPushFilteredWitnessDoesNotCrossScopesAfterTableRecreation(t *testing.T) {
841+
pgURL := testPGURL(t)
842+
_, localDB, pg := newVectorPushTestSync(
843+
t, pgURL, "agentsview_vector_push_filtered_witness_test")
844+
ctx := context.Background()
845+
846+
seedVectorSessionProject(t, localDB, "alpha", "alpha")
847+
seedVectorSessionProject(t, localDB, "beta-1", "beta")
848+
seedVectorSessionProject(t, localDB, "beta-2", "beta")
849+
850+
src := &fakeVectorSource{
851+
gen: VectorGenerationInfo{Fingerprint: "fp-filtered", Model: "m", Dimension: 4},
852+
hasGen: true,
853+
hashes: map[string]string{"alpha": "ha", "beta-1": "hb1", "beta-2": "hb2"},
854+
docs: map[string][]VectorPushDoc{
855+
"alpha": {vdoc("alpha", "alpha#0", 0, "a", "ha", []float32{1, 0, 0, 0})},
856+
"beta-1": {vdoc("beta-1", "beta-1#0", 0, "b1", "hb1", []float32{0, 1, 0, 0})},
857+
"beta-2": {vdoc("beta-2", "beta-2#0", 0, "b2", "hb2", []float32{0, 0, 1, 0})},
858+
},
859+
}
860+
filterScopeAlpha := pushSyncStateScope("work", []string{"alpha"}, nil)
861+
filterScopeBeta := pushSyncStateScope("work", []string{"beta"}, nil)
862+
targetState := newScopedSyncStateStore(localDB, "work", false)
863+
syncAlpha := &Sync{
864+
pg: pg,
865+
local: localDB,
866+
syncState: newScopedSyncStateStore(localDB, filterScopeAlpha, false),
867+
aliasBackfillState: targetState,
868+
machine: "test-machine",
869+
schema: "agentsview_vector_push_filtered_witness_test",
870+
targetFingerprint: "target-fp",
871+
syncStateTarget: filterScopeAlpha,
872+
projects: []string{"alpha"},
873+
vectorSource: src,
874+
schemaDone: true,
875+
}
876+
syncBeta := &Sync{
877+
pg: pg,
878+
local: localDB,
879+
syncState: newScopedSyncStateStore(localDB, filterScopeBeta, false),
880+
aliasBackfillState: targetState,
881+
machine: "test-machine",
882+
schema: "agentsview_vector_push_filtered_witness_test",
883+
targetFingerprint: "target-fp",
884+
syncStateTarget: filterScopeBeta,
885+
projects: []string{"beta"},
886+
vectorSource: src,
887+
schemaDone: true,
888+
}
889+
890+
base, err := syncBeta.Push(ctx, false, nil)
891+
require.NoError(t, err, "baseline beta Push")
892+
gen1 := base.Vectors.GenerationID
893+
require.NotZero(t, gen1)
894+
895+
for _, q := range []string{
896+
`DROP TABLE IF EXISTS ` + vectorChunkTable(gen1),
897+
`DROP TABLE IF EXISTS vector_push_state`,
898+
`DROP TABLE IF EXISTS vector_generation_machines`,
899+
`DROP TABLE IF EXISTS vector_documents`,
900+
`DROP TABLE IF EXISTS vector_generations`,
901+
} {
902+
_, err := pg.Exec(q)
903+
require.NoError(t, err, q)
904+
}
905+
906+
alphaRes, err := syncAlpha.Push(ctx, false, nil)
907+
require.NoError(t, err, "alpha Push after table recreation")
908+
require.Equal(t, gen1, alphaRes.Vectors.GenerationID,
909+
"table recreation reuses the generation id in the case under test")
910+
911+
src.genScopes = nil
912+
src.hashScopes = nil
913+
vres, err := syncBeta.pushVectors(ctx, false, []string{"beta-1"}, gen1, nil, nil)
914+
require.NoError(t, err, "beta scoped push after alpha witness replay")
915+
assert.Equal(t, 2, vres.SessionsPushed,
916+
"beta scope must repopulate every in-scope session generation-wide")
917+
require.Len(t, src.genScopes, 2)
918+
assert.Equal(t, []string{"beta-1"}, src.genScopes[0],
919+
"the first pass starts scoped")
920+
assert.Nil(t, src.genScopes[1],
921+
"the beta watcher must reopen generation-wide instead of trusting alpha's witness")
922+
require.Len(t, src.hashScopes, 1)
923+
assert.Nil(t, src.hashScopes[0],
924+
"the replay must read every beta session, not only beta-1")
925+
assert.Equal(t, 2, countRows(t, pg, `
926+
SELECT COUNT(*) FROM vector_push_state
927+
WHERE generation_id = $1 AND session_id IN ('beta-1', 'beta-2')`, gen1),
928+
"both beta sessions have state rows under the recreated generation")
929+
}
930+
834931
func TestVectorPushRoundTrip(t *testing.T) {
835932
pgURL := testPGURL(t)
836933
sync, localDB, pg := newVectorPushTestSync(
@@ -884,6 +981,8 @@ func TestVectorPushRoundTrip(t *testing.T) {
884981
require.NoError(t, err, "LookupVectorGeneration")
885982
require.True(t, ok, "generation registered")
886983
assert.Equal(t, 4, dim)
984+
witnessKey, err := sync.vectorGenerationWitnessKey()
985+
require.NoError(t, err, "vectorGenerationWitnessKey")
887986

888987
assert.Equal(t, 3,
889988
countRows(t, pg, `SELECT COUNT(*) FROM vector_documents`))
@@ -893,7 +992,7 @@ func TestVectorPushRoundTrip(t *testing.T) {
893992
`SELECT COUNT(*) FROM vector_push_state WHERE generation_id = $1`, genID))
894993
assert.Equal(t, 1, countRows(t, pg,
895994
`SELECT COUNT(*) FROM vector_generation_machines
896-
WHERE generation_id = $1 AND machine = $2`, genID, "test-machine"))
995+
WHERE generation_id = $1 AND machine = $2`, genID, witnessKey))
897996
}
898997

899998
func TestVectorPushDeltaNoop(t *testing.T) {

0 commit comments

Comments
 (0)