Skip to content

Commit c5c3fd0

Browse files
committed
feat(control): record per-shard watermarks in transaction read-set
Thread per-core watermark LSNs through Gathered/GatherOutcome instead of collapsing them to a single max, and route every read seam (native SQL loop, native direct-ops, pgwire dispatch) through one protocol-neutral read_set::record_read_set capture point. A predicate read fanned over N shards now records one read-set entry per shard, each validated against that shard's own watermark, rather than a single collapsed value — closing a phantom-safety gap where a read from a stale shard could pass commit-time validation against another shard's higher watermark. Point reads now capture exact row identity via the shared KeyRepr type (extracted from the Data Plane's write-version index into a plane-neutral types::identity module) so read keys and write keys compare directly; everything else falls back to a collection-scoped predicate observation.
1 parent f75e705 commit c5c3fd0

20 files changed

Lines changed: 755 additions & 155 deletions

File tree

nodedb-physical/src/physical_plan/kv.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,3 +370,46 @@ pub enum KvOp {
370370
count: usize,
371371
},
372372
}
373+
374+
impl KvOp {
375+
/// The user collection this op targets, if any. Sorted-index ops keyed only
376+
/// by an index name (and no direct collection) return `None`; `TransferItem`
377+
/// reports its source collection.
378+
pub fn collection(&self) -> Option<&str> {
379+
match self {
380+
KvOp::Get { collection, .. }
381+
| KvOp::Put { collection, .. }
382+
| KvOp::Insert { collection, .. }
383+
| KvOp::InsertIfAbsent { collection, .. }
384+
| KvOp::InsertOnConflictUpdate { collection, .. }
385+
| KvOp::Delete { collection, .. }
386+
| KvOp::Scan { collection, .. }
387+
| KvOp::Expire { collection, .. }
388+
| KvOp::Persist { collection, .. }
389+
| KvOp::GetTtl { collection, .. }
390+
| KvOp::BatchGet { collection, .. }
391+
| KvOp::BatchPut { collection, .. }
392+
| KvOp::RegisterIndex { collection, .. }
393+
| KvOp::DropIndex { collection, .. }
394+
| KvOp::FieldGet { collection, .. }
395+
| KvOp::FieldSet { collection, .. }
396+
| KvOp::Truncate { collection, .. }
397+
| KvOp::Incr { collection, .. }
398+
| KvOp::IncrFloat { collection, .. }
399+
| KvOp::Cas { collection, .. }
400+
| KvOp::GetSet { collection, .. }
401+
| KvOp::Transfer { collection, .. }
402+
| KvOp::RegisterSortedIndex { collection, .. }
403+
| KvOp::MaterializeScan { collection, .. } => Some(collection.as_str()),
404+
KvOp::TransferItem {
405+
source_collection, ..
406+
} => Some(source_collection.as_str()),
407+
KvOp::DropSortedIndex { .. }
408+
| KvOp::SortedIndexRank { .. }
409+
| KvOp::SortedIndexTopK { .. }
410+
| KvOp::SortedIndexRange { .. }
411+
| KvOp::SortedIndexCount { .. }
412+
| KvOp::SortedIndexScore { .. } => None,
413+
}
414+
}
415+
}

nodedb/src/control/gateway/dispatcher.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ async fn dispatch_remote(args: RemoteDispatchArgs<'_>) -> Result<Vec<Vec<u8>>, E
233233
{
234234
// A root-level Gather resolved entirely at the coordinator — its merged
235235
// response is ready; return it instead of shipping anything.
236-
crate::control::server::exchange::Resolved::Gathered(resp) => {
236+
crate::control::server::exchange::Resolved::Gathered(resp, _shard_watermarks) => {
237237
return Ok(vec![resp.payload.to_vec()]);
238238
}
239239
crate::control::server::exchange::Resolved::Plan(p) => p,
@@ -359,7 +359,7 @@ async fn dispatch_remote_stream(args: RemoteDispatchArgs<'_>) -> Result<ResultSt
359359
// ready response/stream — re-emit it as a single-batch / forwarded
360360
// stream. These do not occur for the streamable-scan plans routed here,
361361
// but handle exhaustively and behaviour-preservingly.
362-
crate::control::server::exchange::Resolved::Gathered(resp) => {
362+
crate::control::server::exchange::Resolved::Gathered(resp, _shard_watermarks) => {
363363
let batch = RowBatch {
364364
payload: resp.payload.to_vec(),
365365
watermark_lsn: resp.watermark_lsn,

nodedb/src/control/server/dispatch_utils.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,9 @@ async fn dispatch_to_data_plane_inner(
267267
)
268268
.await?
269269
{
270-
crate::control::server::exchange::Resolved::Gathered(resp) => return Ok(resp),
270+
crate::control::server::exchange::Resolved::Gathered(resp, _shard_watermarks) => {
271+
return Ok(resp);
272+
}
271273
crate::control::server::exchange::Resolved::Plan(p) => p,
272274
// Internal funnel callers want a fully-collected Response, not a lazy
273275
// stream: materialize the stream into one merged-array Response,

nodedb/src/control/server/exchange/gather.rs

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,15 @@ pub struct GatherOutcome {
108108
/// Single merged msgpack array of all row elements.
109109
/// Consumed by the pgwire/native response path and `ProviderScan` embedding.
110110
pub merged_array: Vec<u8>,
111-
/// Maximum watermark LSN seen across all responding cores.
111+
/// Maximum watermark LSN seen across all responding cores. Retained as the
112+
/// scalar fence value for Strong-consistency callers that need one LSN.
112113
pub watermark_lsn: Lsn,
114+
/// Per-shard watermark LSNs — one `(vshard, watermark_lsn)` per responding
115+
/// core, NOT collapsed to the max. The transaction read-set records one
116+
/// entry per participating shard from this, so a predicate read fanned over
117+
/// N cores is validated against each core's own version rather than a
118+
/// single global max.
119+
pub shard_watermarks: Vec<(VShardId, Lsn)>,
113120
}
114121

115122
/// Fan `plan` to every Data-Plane core in parallel and gather the results.
@@ -149,43 +156,46 @@ pub async fn gather_all_cores(
149156
let deadline = Duration::from_secs(deadline_secs);
150157
let max_result_bytes = state.tuning.network.max_query_result_bytes as usize;
151158
let response_futures = receivers.into_iter().map(|(core_id, mut rx)| async move {
152-
match tokio::time::timeout(
159+
let result = match tokio::time::timeout(
153160
deadline,
154161
crate::control::server::dispatch_utils::collect_bounded_response(
155162
&mut rx,
156163
max_result_bytes,
157164
),
158165
)
159166
.await
160-
.map_err(|_| crate::Error::Dispatch {
161-
detail: format!("gather timeout on core {core_id}"),
162-
})? {
163-
Ok(resp) => Ok(resp),
164-
Err(crate::control::server::dispatch_utils::DispatchCollectError::OverBudget {
167+
{
168+
Err(_) => Err(crate::Error::Dispatch {
169+
detail: format!("gather timeout on core {core_id}"),
170+
}),
171+
Ok(Ok(resp)) => Ok(resp),
172+
Ok(Err(crate::control::server::dispatch_utils::DispatchCollectError::OverBudget {
165173
bytes,
166-
}) => Err(crate::Error::ExecutionLimitExceeded {
174+
})) => Err(crate::Error::ExecutionLimitExceeded {
167175
detail: format!(
168176
"gather on core {core_id} exceeded max_query_result_bytes \
169177
({bytes} > {max_result_bytes} bytes)"
170178
),
171179
}),
172-
Err(crate::control::server::dispatch_utils::DispatchCollectError::ChannelClosed) => {
173-
Err(crate::Error::Dispatch {
174-
detail: format!("gather channel closed on core {core_id}"),
175-
})
176-
}
177-
}
180+
Ok(Err(
181+
crate::control::server::dispatch_utils::DispatchCollectError::ChannelClosed,
182+
)) => Err(crate::Error::Dispatch {
183+
detail: format!("gather channel closed on core {core_id}"),
184+
}),
185+
};
186+
(core_id, result)
178187
});
179188

180-
let results: Vec<crate::Result<Response>> = join_all(response_futures).await;
189+
let results: Vec<(usize, crate::Result<Response>)> = join_all(response_futures).await;
181190

182191
let mut raw = Vec::new();
183192
let mut all_elements: Vec<Vec<u8>> = Vec::new();
184193
let mut max_lsn = Lsn::ZERO;
194+
let mut shard_watermarks: Vec<(VShardId, Lsn)> = Vec::new();
185195
let mut had_error = false;
186196
let mut error_msg = String::new();
187197

188-
for result in results {
198+
for (core_id, result) in results {
189199
let resp = match result {
190200
Ok(r) => r,
191201
Err(e) => {
@@ -208,6 +218,11 @@ pub async fn gather_all_cores(
208218
continue;
209219
}
210220

221+
// Record this core's own watermark as a participating-shard version,
222+
// even when its payload is empty — an empty scan slice is still a
223+
// validatable observation at that shard's version (phantom safety).
224+
shard_watermarks.push((VShardId::new(core_id as u32), resp.watermark_lsn));
225+
211226
if resp.watermark_lsn > max_lsn {
212227
max_lsn = resp.watermark_lsn;
213228
}
@@ -231,6 +246,7 @@ pub async fn gather_all_cores(
231246
raw,
232247
merged_array,
233248
watermark_lsn: max_lsn,
249+
shard_watermarks,
234250
})
235251
}
236252

@@ -381,8 +397,12 @@ pub async fn gather_all_vshards(
381397
// watermark LSNs back through the gateway response, so Strong-consistency
382398
// LSN fencing degrades to pass-through on this path. This is consistent
383399
// with existing gateway behavior (gateway.execute returns no LSN metadata).
384-
// Tracked as a follow-up: propagate watermark_lsn through GatewayResponse.
400+
// The cross-node per-shard watermark folds into the gateway wire change
401+
// (co-located with cross-shard `txn_id`/`database_id` threading), so no
402+
// per-shard watermarks are surfaced here yet — a ZERO read version is
403+
// over-conservative (never unsafe) and nothing validates it until then.
385404
watermark_lsn: Lsn::ZERO,
405+
shard_watermarks: Vec::new(),
386406
})
387407
}
388408

nodedb/src/control/server/exchange/resolve/exchange.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::bridge::envelope::Response;
2020
use crate::control::security::identity::AuthenticatedIdentity;
2121
use crate::control::state::SharedState;
2222
use crate::data::executor::response_codec::flatten_to_relational_rows;
23-
use crate::types::{DatabaseId, TenantId, TraceId, TxnId};
23+
use crate::types::{DatabaseId, Lsn, TenantId, TraceId, TxnId, VShardId};
2424

2525
use crate::control::server::exchange::gather::{
2626
GatherOutcome, finalize_aggregate, gather_all_cores, gather_all_cores_stream,
@@ -33,8 +33,13 @@ use super::materialize::materialize_providers;
3333
/// Result of `resolve_and_materialize`.
3434
pub enum Resolved {
3535
/// The plan was a root-level `Gather` — the coordinator has already
36-
/// executed it and the response is ready to return to the client.
37-
Gathered(Response),
36+
/// executed it and the response is ready to return to the client. The
37+
/// second field carries the per-shard watermark LSNs the gather observed
38+
/// (one `(vshard, watermark_lsn)` per responding core), so an in-transaction
39+
/// read can record one read-set entry per participating shard rather than a
40+
/// single collapsed max. Empty for cross-node gathers (per-shard watermarks
41+
/// are not yet threaded through the gateway) and for shuffle joins.
42+
Gathered(Response, Vec<(VShardId, Lsn)>),
3843
/// The plan (possibly mutated by catalog materialization or Broadcast
3944
/// embedding) is self-contained and should be dispatched normally.
4045
Plan(PhysicalPlan),
@@ -131,7 +136,7 @@ async fn resolve_exchange(
131136
.await?
132137
{
133138
Resolved::Plan(p) => p,
134-
Resolved::Gathered(resp) => return Ok(Resolved::Gathered(resp)),
139+
Resolved::Gathered(resp, wms) => return Ok(Resolved::Gathered(resp, wms)),
135140
// A nested Exchange that itself resolved to a stream cannot be
136141
// re-wrapped by an outer Gather without materializing first;
137142
// surface it as the stream (the outer Gather is redundant —
@@ -151,7 +156,14 @@ async fn resolve_exchange(
151156
// and merges the per-route streams with the same `select_all`.
152157
//
153158
// Aggregate gathers keep the materialize-then-merge behaviour.
154-
if !as_aggregate && child.is_streamable_unordered_scan() {
159+
//
160+
// An in-transaction read (`txn_id.is_some()`) also keeps the
161+
// materialize path: streaming collapses per-core watermarks into one
162+
// value, but a transaction must record each participating shard's own
163+
// read version for optimistic-concurrency validation, so it takes the
164+
// `gather_all_vshards` branch below whose `GatherOutcome` preserves
165+
// `shard_watermarks`.
166+
if !as_aggregate && txn_id.is_none() && child.is_streamable_unordered_scan() {
155167
let stream = if let Some(gw) = state.gateway.as_ref() {
156168
let ctx = crate::control::gateway::core::QueryContext {
157169
tenant_id,
@@ -176,10 +188,10 @@ async fn resolve_exchange(
176188
} else {
177189
outcome.merged_array
178190
};
179-
Ok(Resolved::Gathered(outcome_to_response(
180-
payload,
181-
outcome.watermark_lsn,
182-
)))
191+
Ok(Resolved::Gathered(
192+
outcome_to_response(payload, outcome.watermark_lsn),
193+
outcome.shard_watermarks,
194+
))
183195
}
184196

185197
// Root-level Broadcast: unusual but treat as Gather without merge.
@@ -189,10 +201,10 @@ async fn resolve_exchange(
189201
})) => {
190202
let outcome =
191203
gather_all_vshards(state, tenant_id, database_id, *child, trace_id, txn_id).await?;
192-
Ok(Resolved::Gathered(outcome_to_response(
193-
outcome.merged_array,
194-
outcome.watermark_lsn,
195-
)))
204+
Ok(Resolved::Gathered(
205+
outcome_to_response(outcome.merged_array, outcome.watermark_lsn),
206+
outcome.shard_watermarks,
207+
))
196208
}
197209

198210
// Root-level Shuffle: orchestrate a real cross-node grace hash join.

nodedb/src/control/server/exchange/resolve/shuffle.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,12 @@ pub async fn resolve_shuffle_join(
298298
// staged sides are consumed; there is no coordinator-side cleanup hook to
299299
// call here. No leak: each `(shuffle_id, part, side)` inbox is dropped by
300300
// the part-owner after its consume completes.
301-
Ok(Resolved::Gathered(outcome_to_response(merged, Lsn::ZERO)))
301+
// Cross-node shuffle join: per-shard watermarks are not threaded through
302+
// the shuffle transport, so no per-shard read versions are surfaced here.
303+
Ok(Resolved::Gathered(
304+
outcome_to_response(merged, Lsn::ZERO),
305+
Vec::new(),
306+
))
302307
}
303308

304309
/// Send one `ShuffleProduceRequest` and map the reply / RPC error to a typed

nodedb/src/control/server/exchange/resolve/shuffle_aggregate.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,12 @@ pub async fn resolve_shuffle_aggregate(
311311
}
312312
let merged = encode_msgpack_array(&elements);
313313

314-
Ok(Resolved::Gathered(outcome_to_response(merged, Lsn::ZERO)))
314+
// Cross-node shuffle aggregate: per-shard watermarks are not threaded
315+
// through the shuffle transport, so no per-shard read versions here.
316+
Ok(Resolved::Gathered(
317+
outcome_to_response(merged, Lsn::ZERO),
318+
Vec::new(),
319+
))
315320
}
316321

317322
/// Send one `ShuffleProduceRequest` and map the reply / RPC error to a typed

nodedb/src/control/server/native/dispatch/direct_ops.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,23 @@ pub(crate) async fn handle_graph_match(
226226
Err(e) => return error_to_native(seq, &e),
227227
};
228228

229+
// A MATCH issued inside a native transaction records a collection-scoped
230+
// predicate read at the shard's watermark, identical to every other read
231+
// seam. Single-shard direct op → one watermark, one entry.
232+
if (resp.status == Status::Ok
233+
|| resp.error_code == Some(crate::bridge::envelope::ErrorCode::NotFound))
234+
&& ctx.sessions.transaction_state(ctx.peer_addr)
235+
== crate::control::server::shared::session::TransactionState::InBlock
236+
{
237+
crate::control::server::shared::session::record_read_set(
238+
ctx.sessions,
239+
ctx.peer_addr,
240+
ctx.tenant_id(),
241+
&plan_for_response,
242+
&[(vshard_id, resp.watermark_lsn)],
243+
);
244+
}
245+
229246
if resp.status == Status::Error {
230247
return data_plane_response_to_native(ctx, seq, &plan_for_response, &resp);
231248
}
@@ -326,10 +343,33 @@ async fn dispatch_single_task(
326343
};
327344

328345
let plan_for_response = task.plan.clone();
346+
let task_vshard = task.vshard_id;
329347
match dispatch_single_task_raw(ctx, task.tenant_id, task.vshard_id, task.plan, task.txn_id)
330348
.await
331349
{
332-
Ok(resp) => data_plane_response_to_native(ctx, seq, &plan_for_response, &resp),
350+
Ok(resp) => {
351+
// Track direct-op reads (PointGet / RangeScan / VectorSearch / KV
352+
// Get) for conflict detection at the protocol-neutral layer, so
353+
// native direct-ops record identically to native SQL and pgwire.
354+
// Absent-key reads record too (a `NotFound` is a validatable phantom
355+
// observation). Direct ops are single-shard, so one watermark → one
356+
// entry.
357+
let records_read = resp.status == Status::Ok
358+
|| resp.error_code == Some(crate::bridge::envelope::ErrorCode::NotFound);
359+
if records_read
360+
&& ctx.sessions.transaction_state(ctx.peer_addr)
361+
== crate::control::server::shared::session::TransactionState::InBlock
362+
{
363+
crate::control::server::shared::session::record_read_set(
364+
ctx.sessions,
365+
ctx.peer_addr,
366+
ctx.tenant_id(),
367+
&plan_for_response,
368+
&[(task_vshard, resp.watermark_lsn)],
369+
);
370+
}
371+
data_plane_response_to_native(ctx, seq, &plan_for_response, &resp)
372+
}
333373
Err(e) => error_to_native(seq, &e),
334374
}
335375
}

0 commit comments

Comments
 (0)