Skip to content

Commit 2bd7d36

Browse files
committed
feat(control): fence uncontended writes on per-vShard deterministic locks
The write-admission gate previously stamped every write Admitted as a no-op seam with no actual locking. It now acquires the exact deterministic lock keys for uncontended POINT writes (Document, KV, Vector, single-home graph edges) against the same per-vShard Arc<Mutex<LockManager>> the Calvin scheduler validates against, and holds the resulting guard across enqueue and response, releasing on drop. A point write whose keys are already held by a pending cross-shard commit, along with any predicate, bulk, or multi-home write, now routes through the deterministic Calvin scheduler instead of the fast path: point writes submit a statically-built TxClass, predicate writes go through dependent reconnaissance to discover their affected surrogates. Both share the same lock table as the fast path, so there is no time-of-check/time-of-use gap between the two paths. TxClass construction is split out of the planner's dispatch module into its own tx_class module to be shared by the new routing path.
1 parent 0cc58a2 commit 2bd7d36

25 files changed

Lines changed: 1540 additions & 478 deletions

File tree

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! A plain (non-RETURNING) write routed through Calvin surfaces its ACTUAL
4+
//! affected-row count, not a synthesized zero.
5+
//!
6+
//! An edge-bearing schemaless collection routes a predicate `DELETE FROM coll
7+
//! WHERE <non-pk>` through the OLLP/Calvin dependent path (rewritten as a
8+
//! `BulkDelete` so mirrored edges are cleaned atomically). The Calvin completion
9+
//! ack carries no payload, so before this fix the applied Data-Plane response —
10+
//! including the affected-row count — was deposited ONLY for a RETURNING write.
11+
//! A plain delete therefore came back as `DELETE 0` even though it removed rows.
12+
//!
13+
//! This test proves the fix: the primary-write participant now deposits its full
14+
//! applied response (count included) for a plain write too, so a multi-row
15+
//! delete routed through Calvin reports the correct `DELETE N`.
16+
//!
17+
//! File name contains "calvin" within the cluster-tests crate so nextest applies
18+
//! the cluster test-group serialization.
19+
20+
mod common;
21+
22+
use std::sync::atomic::Ordering;
23+
use std::time::Duration;
24+
25+
use common::cluster_harness::{TestClusterNode, wait_for};
26+
use tokio_postgres::SimpleQueryMessage;
27+
28+
/// Documents seeded with `mark = 'del'` (all deleted by the predicate) and with
29+
/// `mark = 'keep'` (all retained).
30+
const TO_DELETE: usize = 3;
31+
const TO_KEEP: usize = 3;
32+
33+
/// Count of transactions the single-node sequencer has admitted to an epoch, or
34+
/// `0` if the sequencer metrics handle is not installed yet.
35+
fn sequencer_admitted(node: &TestClusterNode) -> u64 {
36+
node.shared
37+
.sequencer_metrics
38+
.get()
39+
.map(|m| m.admitted_total.load(Ordering::Relaxed))
40+
.unwrap_or(0)
41+
}
42+
43+
/// Whether `coll` is flagged edge-bearing in this node's local catalog. The
44+
/// implicit-edge mark is committed via the replicated metadata path, so the
45+
/// DELETE must wait for it before planning — otherwise the predicate delete
46+
/// lowers to a fast-path bulk delete instead of the Calvin/OLLP `BulkDelete`.
47+
fn collection_edge_bearing(node: &TestClusterNode, coll: &str) -> bool {
48+
node.shared
49+
.credentials
50+
.catalog()
51+
.load_collections_for_tenant(nodedb_types::DatabaseId::DEFAULT, 1)
52+
.map(|v| v.iter().any(|c| c.name == coll && c.has_implicit_edges))
53+
.unwrap_or(false)
54+
}
55+
56+
/// Affected-row count carried by the first `CommandComplete` in a simple-query
57+
/// response (PostgreSQL's `DELETE N` count).
58+
fn command_count(msgs: &[SimpleQueryMessage]) -> Option<u64> {
59+
msgs.iter().find_map(|m| match m {
60+
SimpleQueryMessage::CommandComplete(n) => Some(*n),
61+
_ => None,
62+
})
63+
}
64+
65+
/// A plain predicate `DELETE` on an edge-bearing collection routes through the
66+
/// single-node Calvin OLLP path and reports the correct affected-row count
67+
/// through the completion path (previously reported `DELETE 0`).
68+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
69+
async fn calvin_plain_delete_surfaces_affected_count() {
70+
// 4 Data-Plane cores so the edge endpoints land on distinct vShards: the
71+
// doc-delete participant carries the primary write (and its count) while the
72+
// edge-delete participants carry none — exercising the primary-write deposit
73+
// gate on a genuinely multi-participant transaction.
74+
let node = TestClusterNode::spawn_single_node_calvin(4)
75+
.await
76+
.expect("spawn standalone single-node-calvin server");
77+
78+
// The lone sequencer voter self-elects; wait for it before submitting.
79+
wait_for(
80+
"single-node sequencer leader elected",
81+
Duration::from_secs(10),
82+
Duration::from_millis(50),
83+
|| node.sequencer_leader() == node.node_id,
84+
)
85+
.await;
86+
87+
let coll = "sncalvin_affected_count";
88+
node.client
89+
.simple_query(&format!(
90+
"CREATE COLLECTION {coll} WITH (engine='document_schemaless')"
91+
))
92+
.await
93+
.expect("CREATE COLLECTION");
94+
wait_for(
95+
"collection visible on the node",
96+
Duration::from_secs(10),
97+
Duration::from_millis(50),
98+
|| node.cached_collection_count() >= 1,
99+
)
100+
.await;
101+
102+
// Seed implicit-edge documents (plain docs carrying _from/_to/_type, which
103+
// marks the collection edge-bearing). `mark = 'del'` docs will be matched by
104+
// the predicate delete; `mark = 'keep'` docs must survive.
105+
for i in 0..TO_DELETE {
106+
node.client
107+
.simple_query(&format!(
108+
"INSERT INTO {coll} {{ id: 'del_{i}', _from: 'src_del_{i}', _to: 'hub', _type: 'l', mark: 'del' }}"
109+
))
110+
.await
111+
.unwrap_or_else(|e| panic!("insert deletable edge del_{i}: {e}"));
112+
}
113+
for i in 0..TO_KEEP {
114+
node.client
115+
.simple_query(&format!(
116+
"INSERT INTO {coll} {{ id: 'keep_{i}', _from: 'src_keep_{i}', _to: 'hub', _type: 'l', mark: 'keep' }}"
117+
))
118+
.await
119+
.unwrap_or_else(|e| panic!("insert retained edge keep_{i}: {e}"));
120+
}
121+
122+
// Wait for the implicit-edge mark to land so the predicate delete plans as a
123+
// `BulkDelete` and routes through Calvin (not the fast bulk path).
124+
wait_for(
125+
"collection marked edge-bearing",
126+
Duration::from_secs(10),
127+
Duration::from_millis(50),
128+
|| collection_edge_bearing(&node, coll),
129+
)
130+
.await;
131+
132+
let admitted_before = sequencer_admitted(&node);
133+
134+
// A non-PK predicate DELETE on the edge-bearing collection is a `BulkDelete`
135+
// routed through the OLLP/Calvin dependent path. Plain (no RETURNING): it
136+
// must report the actual number of matched rows.
137+
let msgs = node
138+
.client
139+
.simple_query(&format!("DELETE FROM {coll} WHERE mark = 'del'"))
140+
.await
141+
.expect("plain predicate delete routed through Calvin must complete");
142+
143+
// Proof it traversed the sequencer→scheduler path (not a fast path that
144+
// never touches Calvin): the delete was admitted to a Calvin epoch.
145+
let admitted_after = sequencer_admitted(&node);
146+
assert!(
147+
admitted_after > admitted_before,
148+
"the predicate delete must be admitted to a Calvin epoch \
149+
(before={admitted_before}, after={admitted_after})"
150+
);
151+
152+
// The core assertion: the reported affected count is the number of matched
153+
// rows, carried through the completion sidecar. Before the fix it was 0.
154+
let count = command_count(&msgs).expect("DELETE returns a CommandComplete count");
155+
assert_eq!(
156+
count, TO_DELETE as u64,
157+
"a plain DELETE routed through Calvin must report the actual affected-row \
158+
count ({TO_DELETE}), not a synthesized 0; got {count}"
159+
);
160+
161+
// Sanity: exactly the matched documents are gone and the rest remain.
162+
let remaining = node
163+
.client
164+
.simple_query(&format!("SELECT * FROM {coll}"))
165+
.await
166+
.expect("SELECT all rows");
167+
let remaining_count = remaining
168+
.iter()
169+
.filter(|m| matches!(m, SimpleQueryMessage::Row(_)))
170+
.count();
171+
assert_eq!(
172+
remaining_count, TO_KEEP,
173+
"exactly the mark='del' documents must be removed"
174+
);
175+
176+
node.shutdown().await;
177+
}

nodedb/src/control/cluster/calvin/scheduler/driver/core/dispatch.rs

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -83,36 +83,25 @@ pub(crate) fn plan_vshard(plan: &PhysicalPlan) -> Vec<VShardId> {
8383
)]
8484
}
8585

86-
/// Whether any plan in `plans` carries a RETURNING clause.
86+
/// Whether this vShard's slice carries a PRIMARY user data write — the write
87+
/// whose applied `Response` (affected-count + any RETURNING rows) the
88+
/// coordinator surfaces.
8789
///
88-
/// Only the `Document` DML variants can carry `returning`
89-
/// (`PointUpdate` / `BulkUpdate` / `PointDelete` / `BulkDelete` /
90-
/// `UpdateFromJoin`); every other plan is row-less. Used to gate the applied
91-
/// `Response` deposit so only the RETURNING-bearing participant populates the
92-
/// coordinator's sidecar.
93-
pub(crate) fn plans_have_returning(plans: &[PhysicalPlan]) -> bool {
90+
/// A primary write is a Document / KV / Vector / Timeseries / Columnar / Array
91+
/// write — NOT the implicit graph-edge cleanup (`EdgePut` / `EdgeDelete`) that
92+
/// dual-homes alongside a document delete/update. For a single-collection user
93+
/// DML (plus its implicit edges) exactly ONE participant carries the primary
94+
/// write, so only it deposits the applied `Response` into the coordinator's
95+
/// sidecar and the edge participants never clobber the entry.
96+
///
97+
/// This gate subsumes the RETURNING case (a RETURNING write IS a primary write,
98+
/// so its rows are still deposited) while ALSO carrying the affected-count of a
99+
/// plain (non-RETURNING) write — which a RETURNING-only gate dropped, making a
100+
/// routed plain write report zero rows affected.
101+
pub(crate) fn plans_have_primary_write(plans: &[PhysicalPlan]) -> bool {
94102
plans.iter().any(|plan| {
95-
matches!(
96-
plan,
97-
PhysicalPlan::Document(
98-
DocumentOp::PointUpdate {
99-
returning: Some(_),
100-
..
101-
} | DocumentOp::BulkUpdate {
102-
returning: Some(_),
103-
..
104-
} | DocumentOp::PointDelete {
105-
returning: Some(_),
106-
..
107-
} | DocumentOp::BulkDelete {
108-
returning: Some(_),
109-
..
110-
} | DocumentOp::UpdateFromJoin {
111-
returning: Some(_),
112-
..
113-
}
114-
)
115-
)
103+
crate::control::planner::calvin::is_write_plan(plan)
104+
&& !matches!(plan, PhysicalPlan::Graph(_))
116105
})
117106
}
118107

@@ -215,7 +204,7 @@ impl Scheduler {
215204
return;
216205
}
217206
};
218-
let has_returning = plans_have_returning(&plans);
207+
let has_primary_write = plans_have_primary_write(&plans);
219208
let plan = PhysicalPlan::Meta(MetaOp::CalvinExecuteStatic {
220209
epoch,
221210
position,
@@ -294,7 +283,7 @@ impl Scheduler {
294283
// no-determinism: dispatch_time is scheduler observability, not Calvin WAL data
295284
dispatch_time: dispatch_instant,
296285
lock_acquired_time,
297-
has_returning,
286+
has_primary_write,
298287
},
299288
);
300289
}
@@ -345,7 +334,7 @@ impl Scheduler {
345334
return;
346335
}
347336
};
348-
let has_returning = plans_have_returning(&plans);
337+
let has_primary_write = plans_have_primary_write(&plans);
349338
let plan = PhysicalPlan::Meta(MetaOp::CalvinExecuteActive {
350339
epoch,
351340
position,
@@ -425,7 +414,7 @@ impl Scheduler {
425414
// no-determinism: dispatch_time is scheduler observability, not Calvin WAL data
426415
dispatch_time: dispatch_instant,
427416
lock_acquired_time,
428-
has_returning,
417+
has_primary_write,
429418
},
430419
);
431420
}

nodedb/src/control/cluster/calvin/scheduler/driver/core/process.rs

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! bookkeeping for the Calvin scheduler.
55
66
use std::collections::BTreeMap;
7+
use std::sync::Arc;
78
use std::time::Instant;
89

910
use nodedb_cluster::calvin::types::SequencedTxn;
@@ -37,7 +38,10 @@ impl Scheduler {
3738
keys_count,
3839
)
3940
.entered();
40-
let outcome = self.lock_manager.acquire(txn_id, keys.clone());
41+
let outcome = {
42+
let mut lm = self.lock_manager.lock().unwrap_or_else(|p| p.into_inner());
43+
lm.acquire(txn_id, keys.clone())
44+
};
4145

4246
match outcome {
4347
AcquireOutcome::Ready => {
@@ -116,29 +120,46 @@ impl Scheduler {
116120
) {
117121
let epoch = txn_id.epoch;
118122

119-
let newly_unblocked = self.lock_manager.release(txn_id);
120-
121-
for waiter_id in newly_unblocked {
122-
if let Some(blocked) = self.blocked.get(&waiter_id)
123-
&& self.lock_manager.is_ready(waiter_id, &blocked.keys)
124-
{
125-
let keys = blocked.keys.clone();
126-
let outcome = self.lock_manager.acquire(waiter_id, keys.clone());
127-
debug_assert_eq!(
128-
outcome,
129-
AcquireOutcome::Ready,
130-
"is_ready returned true but acquire returned Blocked"
131-
);
132-
133-
if let Some(blocked_txn) = self.blocked.remove(&waiter_id) {
134-
let wait_ms = blocked_txn.blocked_at.elapsed().as_millis() as u64;
135-
self.metrics.record_lock_wait_ms(wait_ms);
136-
// no-determinism: lock_acquired_time for unblocked txn is scheduler observability, not Calvin WAL data
137-
self.dispatch_or_barrier(blocked_txn.txn, waiter_id, keys, Instant::now());
123+
// Release this txn's locks, promote any newly-unblocked waiters, and
124+
// collect the ones ready to dispatch — all under ONE guard so the
125+
// release and the promoted re-acquire are atomic against a concurrent
126+
// gate probe. Dispatch happens AFTER the guard drops: `dispatch_or_barrier`
127+
// does not touch the lock table, but holding the guard across it would
128+
// deadlock if it ever did, so the collect-then-dispatch split keeps the
129+
// critical section minimal and re-entrancy-safe.
130+
let lm = Arc::clone(&self.lock_manager);
131+
let mut to_dispatch: Vec<(SequencedTxn, TxnId, std::collections::BTreeSet<LockKey>)> =
132+
Vec::new();
133+
{
134+
let mut guard = lm.lock().unwrap_or_else(|p| p.into_inner());
135+
let newly_unblocked = guard.release(txn_id);
136+
137+
for waiter_id in newly_unblocked {
138+
if let Some(blocked) = self.blocked.get(&waiter_id)
139+
&& guard.is_ready(waiter_id, &blocked.keys)
140+
{
141+
let keys = blocked.keys.clone();
142+
let outcome = guard.acquire(waiter_id, keys.clone());
143+
debug_assert_eq!(
144+
outcome,
145+
AcquireOutcome::Ready,
146+
"is_ready returned true but acquire returned Blocked"
147+
);
148+
149+
if let Some(blocked_txn) = self.blocked.remove(&waiter_id) {
150+
let wait_ms = blocked_txn.blocked_at.elapsed().as_millis() as u64;
151+
self.metrics.record_lock_wait_ms(wait_ms);
152+
to_dispatch.push((blocked_txn.txn, waiter_id, keys));
153+
}
138154
}
139155
}
140156
}
141157

158+
for (txn, waiter_id, keys) in to_dispatch {
159+
// no-determinism: lock_acquired_time for unblocked txn is scheduler observability, not Calvin WAL data
160+
self.dispatch_or_barrier(txn, waiter_id, keys, Instant::now());
161+
}
162+
142163
if self.last_applied_epoch
143164
== crate::control::cluster::calvin::scheduler::NOT_YET_APPLIED_EPOCH
144165
|| epoch > self.last_applied_epoch

0 commit comments

Comments
 (0)