fix(hnsw): correct the vector dead-node lifecycle (record on delete, clear on re-insert)

[shaunpatterson]

Summary

The HNSW vector index maintains a per-predicate dead-node list — a uid array stored in a single posting at DataKey(<pred>__vector_dead, 1) — that removeDeadNodes uses to strip edges pointing at deleted vectors. Its lifecycle has two correctness bugs that this PR fixes.

Bug A — deletes are never recorded

In addIndexMutations, the delete branch reads the deleted value from pl.AllValues(txn.StartTs):

data, _ := pl.AllValues(txn.StartTs)
if info.op == DEL && len(data) > 0 && data[0].Tid == types.VFloatID { /* record dead */ }

But addMutationHelper has already applied the delete before this runs, so AllValues(txn.StartTs) is empty (len(data)==0) and the dead node is never recorded. The value being deleted is already available as info.val and was simply ignored. Net effect: the dead list stays empty, removeDeadNodes is a no-op, and edges to deleted vectors are never cleaned.

Bug B — the list is append-only (never cleared on re-insert)

Re-inserting (re-embedding) a previously deleted uid never removed it from the dead list. Once A is fixed, a re-inserted vector would stay marked dead, get stripped from every neighbour's edge list, and end up orphaned — present but unreachable in search.

This is not hypothetical: a SET that overwrites an existing vector runs addIndexMutations with op=DEL (old value) then op=SET (new value) in a single mutation. So without B, every in-place re-embed would orphan its own node.

Fix

• Record on delete using info.val (the value actually being deleted).
• Clear on (re-)insert so a revived uid is no longer treated as dead.
• Both go through updateDeadNodes, an atomic read-modify-write under the posting's write lock. A multi-edge mutation applies its edges in parallel goroutines sharing one txn (worker/draft.go), and the index rebuilder streams with 16 goroutines — all touching this single posting. The previous unlocked delete-path RMW could lose updates under that concurrency; the locked RMW serializes them and reads the txn's own pending writes, so a delete + re-insert within one mutation nets out correctly.
• recordDeadNode is idempotent (no duplicate growth on repeated deletes / Raft re-apply).

Tests (posting/index_vector_dead_test.go)

All drive the real AddMutationWithIndex → addIndexMutations path:

• TestDeletedVectorRecordedAsDead — a vfloat delete records the uid (bug A).
• TestReinsertedVectorClearedFromDeadList — re-insert clears it (bug B).
• TestReinsertedVectorIsSearchable — a deleted-then-reinserted vector is reachable in HNSW search (end-to-end).
• TestDeadNodeConcurrentRecord — concurrent records don't lose updates (passes under -race).
• TestDeadNodeSameTxnRecordThenClear — record + clear within one txn nets out (covers the SET-overwrite path).

go test ./posting/ ./tok/... passes; gofmt/go vet clean.

Note / future work

The dead list is a single JSON-blob posting. This PR makes its lifecycle correct and concurrency-safe, but the blob still grows O(distinct deletes) and is re-marshalled on each change. A more scalable representation (per-uid postings / a uid bitmap, enabling index.Remove(...)) is left as a follow-up — the existing // TODO ... index.Remove is preserved.

🤖 Generated with Claude Code