Skip to content

Commit c2217d1

Browse files
committed
fix(data): populate write-version index for vector engine writes
Vector write handlers (HNSW/IVF insert, direct upsert, batch insert, multi-vector insert/delete, sparse insert/delete, delete-by-surrogate) applied writes to storage without recording their LSN in the per-key/per-collection write-version index, so cross-shard OCC read-set validation couldn't see them. Record per-surrogate versions where a cross-engine surrogate identity exists, and fall back to the collection floor for doc_id-keyed sparse ops and for the HNSW node-id-keyed delete path where the surrogate isn't the write key. Extend the transaction-replay fork in record_vector_version to mirror the same per-op-kind resolution.
1 parent 427c5bf commit c2217d1

6 files changed

Lines changed: 793 additions & 14 deletions

File tree

nodedb/src/data/executor/handlers/transaction/write_version.rs

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::types::{Lsn, TenantId};
1818
use nodedb_physical::physical_plan::{
1919
ColumnarOp, CrdtOp, DocumentOp, GraphOp, KvOp, SpatialOp, TextOp, TimeseriesOp, VectorOp,
2020
};
21+
use nodedb_types::Surrogate;
2122

2223
impl CoreLoop {
2324
/// Record the version of every key written by a committed transaction batch.
@@ -138,26 +139,108 @@ impl CoreLoop {
138139
);
139140
}
140141

142+
/// Records the version of a vector-engine write within a committed
143+
/// transaction batch.
144+
///
145+
/// Mirrors the fork resolution the live write handlers apply (see
146+
/// `handlers/vector*.rs`): surrogate-carrying ops record `KeyRepr::Surrogate`
147+
/// per surrogate (a superset of the collection floor, since vector SEARCH
148+
/// reads always record `ReadKey::Predicate`, the collection floor); sparse
149+
/// ops (keyed by `doc_id: String`, no cross-engine surrogate) and
150+
/// `Delete` (whose `vector_id` is the internal HNSW node id, NOT the
151+
/// surrogate — recording it as `KeyRepr::Surrogate` would be a wrong
152+
/// identity) record the collection floor only. Read/config/query ops
153+
/// write nothing and are named explicitly below so the match stays
154+
/// exhaustive.
141155
fn record_vector_version(
142156
&mut self,
143157
db: crate::types::DatabaseId,
144158
tenant: TenantId,
145159
op: &VectorOp,
146160
lsn: Lsn,
147161
) {
148-
if let VectorOp::Insert {
149-
collection,
150-
surrogate,
151-
..
152-
} = op
153-
{
154-
self.note_write_lsn(
155-
db,
156-
tenant,
162+
match op {
163+
VectorOp::Insert {
164+
collection,
165+
surrogate,
166+
..
167+
}
168+
| VectorOp::DirectUpsert {
169+
collection,
170+
surrogate,
171+
..
172+
}
173+
| VectorOp::DeleteBySurrogate {
174+
collection,
175+
surrogate,
176+
..
177+
} => {
178+
self.note_write_lsn(
179+
db,
180+
tenant,
181+
collection,
182+
Some(KeyRepr::Surrogate(surrogate.as_u32())),
183+
lsn,
184+
);
185+
}
186+
VectorOp::MultiVectorInsert {
157187
collection,
158-
Some(KeyRepr::Surrogate(surrogate.as_u32())),
159-
lsn,
160-
);
188+
document_surrogate,
189+
..
190+
}
191+
| VectorOp::MultiVectorDelete {
192+
collection,
193+
document_surrogate,
194+
..
195+
} => {
196+
self.note_write_lsn(
197+
db,
198+
tenant,
199+
collection,
200+
Some(KeyRepr::Surrogate(document_surrogate.as_u32())),
201+
lsn,
202+
);
203+
}
204+
VectorOp::BatchInsert {
205+
collection,
206+
surrogates,
207+
..
208+
} => {
209+
let mut any_surrogate_recorded = false;
210+
for s in surrogates {
211+
if *s != Surrogate::ZERO {
212+
self.note_write_lsn(
213+
db,
214+
tenant,
215+
collection,
216+
Some(KeyRepr::Surrogate(s.as_u32())),
217+
lsn,
218+
);
219+
any_surrogate_recorded = true;
220+
}
221+
}
222+
if !any_surrogate_recorded {
223+
self.note_write_lsn(db, tenant, collection, None, lsn);
224+
}
225+
}
226+
// Sparse (doc_id-keyed, no surrogate) and `Delete` (vector_id is
227+
// the internal HNSW node id, not the surrogate): collection floor
228+
// only.
229+
VectorOp::SparseInsert { collection, .. }
230+
| VectorOp::SparseDelete { collection, .. }
231+
| VectorOp::Delete { collection, .. } => {
232+
self.note_write_lsn(db, tenant, collection, None, lsn);
233+
}
234+
// Read / config / query ops: nothing written, no version to record.
235+
VectorOp::Search { .. }
236+
| VectorOp::MultiSearch { .. }
237+
| VectorOp::SetParams { .. }
238+
| VectorOp::QueryStats { .. }
239+
| VectorOp::Seal { .. }
240+
| VectorOp::CompactIndex { .. }
241+
| VectorOp::Rebuild { .. }
242+
| VectorOp::SparseSearch { .. }
243+
| VectorOp::MultiVectorScoreSearch { .. } => {}
161244
}
162245
}
163246

nodedb/src/data/executor/handlers/vector.rs

Lines changed: 156 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ impl CoreLoop {
198198
&& cfg.index_type == crate::engine::vector::index_config::IndexType::IvfPq
199199
{
200200
let key = index_key.clone();
201-
return self.ivf_insert(task, &key, vector, dim, surrogate);
201+
return self.ivf_insert(task, tid, &key, vector, dim, surrogate);
202202
}
203203

204204
// Default: HNSW (with or without PQ).
@@ -222,6 +222,15 @@ impl CoreLoop {
222222
warn!(core = self.core_id, error = %e, "failed to send HNSW build request");
223223
}
224224
self.checkpoint_coordinator.mark_dirty("vector", 1);
225+
// Record this write's version so cross-shard OCC read-set
226+
// validation (predicate reads always record the collection
227+
// floor) sees this insert. `ZERO` means no surrogate binding
228+
// was made (headless insert) — floor-only.
229+
if surrogate == Surrogate::ZERO {
230+
self.note_collection_write_lsn(task, collection);
231+
} else {
232+
self.note_surrogate_write_lsn(task, tid, collection, surrogate.as_u32());
233+
}
225234
self.response_ok(task)
226235
}
227236
Err(err) => self.response_error(task, err),
@@ -232,6 +241,7 @@ impl CoreLoop {
232241
fn ivf_insert(
233242
&mut self,
234243
task: &ExecutionTask,
244+
tid: u64,
235245
index_key: &(DatabaseId, TenantId, String),
236246
vector: &[f32],
237247
dim: usize,
@@ -274,6 +284,14 @@ impl CoreLoop {
274284
}
275285

276286
self.checkpoint_coordinator.mark_dirty("vector", 1);
287+
// Record this write's version so cross-shard OCC read-set validation
288+
// sees this insert, same as the HNSW insert path above. `ZERO` means
289+
// no surrogate binding was made (headless insert) — floor-only.
290+
if surrogate == Surrogate::ZERO {
291+
self.note_collection_write_lsn(task, &index_key.2);
292+
} else {
293+
self.note_surrogate_write_lsn(task, tid, &index_key.2, surrogate.as_u32());
294+
}
277295
self.response_ok(task)
278296
}
279297

@@ -375,11 +393,147 @@ impl CoreLoop {
375393
.and_then(|c| c.surrogate_to_local.get(&surrogate).copied());
376394

377395
match node_id {
378-
Some(vid) => self.execute_vector_delete(task, tid, collection, vid),
396+
Some(vid) => {
397+
let response = self.execute_vector_delete(task, tid, collection, vid);
398+
if response.status == crate::bridge::envelope::Status::Ok {
399+
// Record this write's version keyed by the cross-engine
400+
// surrogate (a superset of `execute_vector_delete`'s
401+
// node-id-scoped floor-only record below — correct and
402+
// more precise since the surrogate identity is known here).
403+
self.note_surrogate_write_lsn(task, tid, collection, surrogate.as_u32());
404+
}
405+
response
406+
}
379407
None => {
380408
// Surrogate not present — idempotent.
381409
self.response_ok(task)
382410
}
383411
}
384412
}
385413
}
414+
415+
#[cfg(test)]
416+
mod tests {
417+
use super::*;
418+
use crate::bridge::envelope::{PhysicalPlan, Priority, Request};
419+
use crate::data::executor::core_loop::write_index::WriteKey;
420+
use crate::types::{Lsn, RequestId, TraceId, VShardId};
421+
use nodedb_bridge::buffer::RingBuffer;
422+
use nodedb_physical::physical_plan::VectorOp;
423+
use std::time::{Duration, Instant};
424+
425+
struct CoreHarness {
426+
core: CoreLoop,
427+
_req_tx: nodedb_bridge::buffer::Producer<crate::bridge::dispatch::BridgeRequest>,
428+
_resp_rx: nodedb_bridge::buffer::Consumer<crate::bridge::dispatch::BridgeResponse>,
429+
_dir: tempfile::TempDir,
430+
}
431+
432+
fn make_core() -> CoreHarness {
433+
use crate::bridge::dispatch::{BridgeRequest, BridgeResponse};
434+
435+
let dir = tempfile::tempdir().expect("tempdir");
436+
let (req_tx, req_rx) = RingBuffer::channel::<BridgeRequest>(64);
437+
let (resp_tx, resp_rx) = RingBuffer::channel::<BridgeResponse>(64);
438+
let core = CoreLoop::open(
439+
0,
440+
req_rx,
441+
resp_tx,
442+
dir.path(),
443+
std::sync::Arc::new(nodedb_types::OrdinalClock::new()),
444+
)
445+
.expect("open core");
446+
CoreHarness {
447+
core,
448+
_req_tx: req_tx,
449+
_resp_rx: resp_rx,
450+
_dir: dir,
451+
}
452+
}
453+
454+
/// A task carrying `wal_lsn` so the handler's `note_*_write_lsn` calls
455+
/// (gated on `task.wal_lsn().is_some()`) actually fire, mirroring a live
456+
/// write dispatched with an allocated WAL LSN.
457+
fn make_task_with_lsn(lsn: u64) -> ExecutionTask {
458+
ExecutionTask::new(Request {
459+
request_id: RequestId::new(1),
460+
tenant_id: TenantId::new(1),
461+
database_id: DatabaseId::DEFAULT,
462+
vshard_id: VShardId::new(0),
463+
plan: PhysicalPlan::Vector(VectorOp::Search {
464+
collection: "docs".to_string(),
465+
query_vector: Vec::new(),
466+
top_k: 0,
467+
ef_search: 0,
468+
metric: nodedb_types::vector_distance::DistanceMetric::L2,
469+
filter_bitmap: None,
470+
field_name: String::new(),
471+
rls_filters: Vec::new(),
472+
inline_prefilter_plan: None,
473+
ann_options: Default::default(),
474+
skip_payload_fetch: false,
475+
payload_filters: Vec::new(),
476+
}),
477+
deadline: Instant::now() + Duration::from_secs(5),
478+
priority: Priority::Normal,
479+
trace_id: TraceId::ZERO,
480+
consistency: crate::types::ReadConsistency::Strong,
481+
idempotency_key: None,
482+
event_source: crate::event::EventSource::User,
483+
user_roles: Vec::new(),
484+
user_id: None,
485+
statement_digest: None,
486+
txn_id: None,
487+
wal_lsn: Some(Lsn::new(lsn)),
488+
resolved_now_ms: None,
489+
admission: crate::bridge::envelope::Admission::Exempt(
490+
crate::bridge::envelope::ExemptReason::Read,
491+
),
492+
})
493+
}
494+
495+
#[test]
496+
fn vector_insert_populates_write_version_index_surrogate_and_floor() {
497+
let mut h = make_core();
498+
let task = make_task_with_lsn(11);
499+
let surrogate = Surrogate::new(42);
500+
501+
let response = h.core.execute_vector_insert(VectorInsertParams {
502+
task: &task,
503+
tid: 1,
504+
collection: "docs",
505+
vector: &[1.0, 2.0, 3.0],
506+
dim: 3,
507+
field_name: "",
508+
surrogate,
509+
provenance: None,
510+
});
511+
assert_eq!(response.status, crate::bridge::envelope::Status::Ok);
512+
513+
let key = WriteKey {
514+
db: DatabaseId::DEFAULT,
515+
tenant: TenantId::new(1),
516+
collection: Box::from("docs"),
517+
key: crate::data::executor::core_loop::write_index::KeyRepr::Surrogate(
518+
surrogate.as_u32(),
519+
),
520+
};
521+
assert_eq!(
522+
h.core.write_index.key_write_lsn(&key),
523+
Some(Lsn::new(11)),
524+
"vector insert must populate the per-key (surrogate) write-version index"
525+
);
526+
527+
let coll_key = crate::data::executor::core_loop::write_index::CollKey {
528+
db: DatabaseId::DEFAULT,
529+
tenant: TenantId::new(1),
530+
collection: Box::from("docs"),
531+
};
532+
assert_eq!(
533+
h.core.write_index.collection_write_lsn(&coll_key),
534+
Some(Lsn::new(11)),
535+
"vector insert must advance the collection write-version floor \
536+
(predicate reads validate against the floor)"
537+
);
538+
}
539+
}

0 commit comments

Comments
 (0)