Skip to content

Commit 0cc58a2

Browse files
committed
feat(control): carry applied RETURNING rows through Calvin completion
The Calvin completion path previously signalled success with a bare `()`/ack — the durable, replicated commit — so any RETURNING clause on a write routed through the sequencer (predicate deletes/updates, and dependent writes reconciled via OLLP) had its applied rows silently dropped, leaving the client with only a command tag. The per-vShard scheduler now deposits the applied Data-Plane response into a new in-process sidecar (`SharedState::calvin_apply_results`, keyed by `TxnId`) before proposing the replicated completion ack, gated to the one participant whose slice actually carries a RETURNING clause. The coordinator's completion paths (`submit_and_await_calvin`, `dispatch_dependent_edge_recon`) drain the sidecar once the ack fires and hand the response back through the native and pgwire dispatch paths so RETURNING rows are shaped into DATA-ROWs instead of a bare tag. Cross-node, the payload rides the existing non-Raft routed-submit RPC response rather than the sequencer Raft log, since it's a query result and not replicated state. A second RETURNING-bearing participant for one transaction is recorded as a conflict and drained as a loud error rather than returning a partial cross-shard union.
1 parent c9826d9 commit 0cc58a2

28 files changed

Lines changed: 658 additions & 68 deletions

File tree

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! A `DELETE ... RETURNING` routed through Calvin surfaces its deleted rows,
4+
//! not a bare command tag.
5+
//!
6+
//! An edge-bearing schemaless collection routes a `DELETE FROM coll WHERE id =
7+
//! 'x'` through the OLLP/Calvin dependent path (the implicit-edge routing gate
8+
//! rewrites it as a `BulkDelete` so mirrored edges are cleaned atomically). The
9+
//! Calvin completion path signals done via a Raft-replicated ack that carries no
10+
//! payload, so before this fix the applied Data-Plane response — including the
11+
//! `RETURNING` rows — was dropped and the client saw only `DELETE 1`.
12+
//!
13+
//! This test proves the fix: with the single-node Calvin stack on, deleting one
14+
//! implicit-edge document with `RETURNING *` returns that document's row through
15+
//! the completion path.
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+
27+
/// Number of implicit-edge documents seeded before the RETURNING delete.
28+
const SOURCES: usize = 6;
29+
30+
/// Count of transactions the single-node sequencer has admitted to an epoch, or
31+
/// `0` if the sequencer metrics handle is not installed yet.
32+
fn sequencer_admitted(node: &TestClusterNode) -> u64 {
33+
node.shared
34+
.sequencer_metrics
35+
.get()
36+
.map(|m| m.admitted_total.load(Ordering::Relaxed))
37+
.unwrap_or(0)
38+
}
39+
40+
/// Whether `coll` is flagged edge-bearing in this node's local catalog. The
41+
/// implicit-edge mark is committed via the replicated metadata path, so the
42+
/// DELETE must wait for it before planning — otherwise the PK delete lowers to a
43+
/// static `PointDelete` (fast path) instead of the Calvin/OLLP `BulkDelete`.
44+
fn collection_edge_bearing(node: &TestClusterNode, coll: &str) -> bool {
45+
node.shared
46+
.credentials
47+
.catalog()
48+
.load_collections_for_tenant(nodedb_types::DatabaseId::DEFAULT, 1)
49+
.map(|v| v.iter().any(|c| c.name == coll && c.has_implicit_edges))
50+
.unwrap_or(false)
51+
}
52+
53+
/// Flag ON: a `DELETE ... RETURNING` on an edge-bearing collection routes
54+
/// through the single-node Calvin OLLP path and returns the deleted row through
55+
/// the completion path (previously dropped as a bare tag).
56+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
57+
async fn calvin_returning_delete_surfaces_deleted_row() {
58+
// 4 Data-Plane cores so the edge endpoints land on distinct vShards — the
59+
// doc-delete participant carries the RETURNING rows while the edge-delete
60+
// participants carry none, exercising the deposit gate.
61+
let node = TestClusterNode::spawn_single_node_calvin(4)
62+
.await
63+
.expect("spawn standalone single-node-calvin server");
64+
65+
// The lone sequencer voter self-elects; wait for it before submitting.
66+
wait_for(
67+
"single-node sequencer leader elected",
68+
Duration::from_secs(10),
69+
Duration::from_millis(50),
70+
|| node.sequencer_leader() == node.node_id,
71+
)
72+
.await;
73+
74+
// The collection name deliberately embeds "returning": it doubles as
75+
// regression coverage that a `*_returning` identifier is not mistaken for the
76+
// RETURNING keyword by the clause stripper.
77+
let coll = "sncalvin_returning";
78+
node.client
79+
.simple_query(&format!(
80+
"CREATE COLLECTION {coll} WITH (engine='document_schemaless')"
81+
))
82+
.await
83+
.expect("CREATE COLLECTION");
84+
wait_for(
85+
"collection visible on the node",
86+
Duration::from_secs(10),
87+
Duration::from_millis(50),
88+
|| node.cached_collection_count() >= 1,
89+
)
90+
.await;
91+
92+
// src_0..src_(SOURCES-1) -> hub as IMPLICIT edges (plain docs carrying
93+
// _from/_to/_type). Inserting an edge document marks the collection
94+
// edge-bearing, which is what routes the later DELETE through OLLP/Calvin.
95+
for i in 0..SOURCES {
96+
node.client
97+
.simple_query(&format!(
98+
"INSERT INTO {coll} {{ id: 'edge_{i}', _from: 'src_{i}', _to: 'hub', _type: 'l', mark: 'keep' }}"
99+
))
100+
.await
101+
.unwrap_or_else(|e| panic!("insert implicit edge src_{i} -> hub: {e}"));
102+
}
103+
104+
// Wait for the implicit-edge mark to land so the PK delete plans as a
105+
// `BulkDelete` and routes through Calvin (not the fast `PointDelete` path).
106+
wait_for(
107+
"collection marked edge-bearing",
108+
Duration::from_secs(10),
109+
Duration::from_millis(50),
110+
|| collection_edge_bearing(&node, coll),
111+
)
112+
.await;
113+
114+
let admitted_before = sequencer_admitted(&node);
115+
116+
// A PK-equality DELETE on the edge-bearing collection is rewritten to a
117+
// `BulkDelete` (id filter) and routed through the OLLP/Calvin dependent path.
118+
// With RETURNING it must come back carrying the deleted document's row.
119+
let msgs = node
120+
.client
121+
.simple_query(&format!(
122+
"DELETE FROM {coll} WHERE id = 'edge_3' RETURNING *"
123+
))
124+
.await
125+
.expect("RETURNING delete routed through Calvin must complete");
126+
127+
// Proof it traversed the sequencer→scheduler path (not the fast PointDelete
128+
// path that never touches Calvin): the delete was admitted to a Calvin epoch.
129+
let admitted_after = sequencer_admitted(&node);
130+
assert!(
131+
admitted_after > admitted_before,
132+
"the RETURNING delete must be admitted to a Calvin epoch \
133+
(before={admitted_before}, after={admitted_after})"
134+
);
135+
136+
let rows: Vec<&tokio_postgres::SimpleQueryRow> = msgs
137+
.iter()
138+
.filter_map(|m| match m {
139+
tokio_postgres::SimpleQueryMessage::Row(r) => Some(r),
140+
_ => None,
141+
})
142+
.collect();
143+
144+
// The core assertion: the response CONTAINS the deleted row, not just a bare
145+
// `DELETE 1` command tag. Before the fix this vec is empty.
146+
assert_eq!(
147+
rows.len(),
148+
1,
149+
"DELETE ... RETURNING routed through Calvin must surface exactly the one \
150+
deleted row, not a bare command tag; got {} row(s)",
151+
rows.len()
152+
);
153+
let id = rows[0].get("id").expect("returned row has an id column");
154+
assert_eq!(
155+
id, "edge_3",
156+
"the returned row must be the deleted document (id = 'edge_3'); got id = {id:?}"
157+
);
158+
159+
// Sanity: the deleted document is actually gone and the others remain.
160+
let remaining = node
161+
.client
162+
.simple_query(&format!("SELECT * FROM {coll}"))
163+
.await
164+
.expect("SELECT all rows");
165+
let remaining_count = remaining
166+
.iter()
167+
.filter(|m| matches!(m, tokio_postgres::SimpleQueryMessage::Row(_)))
168+
.count();
169+
assert_eq!(
170+
remaining_count,
171+
SOURCES - 1,
172+
"exactly the RETURNING-deleted document must be removed"
173+
);
174+
175+
node.shutdown().await;
176+
}

nodedb-cluster-tests/tests/transport_security.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ impl RaftRpcHandler for EchoHandler {
151151
&self,
152152
_req: nodedb_cluster::rpc_codec::SubmitCalvinTxnRequest,
153153
) -> nodedb_cluster::rpc_codec::SubmitCalvinTxnResponse {
154-
nodedb_cluster::rpc_codec::SubmitCalvinTxnResponse { error: None }
154+
nodedb_cluster::rpc_codec::SubmitCalvinTxnResponse {
155+
error: None,
156+
payload_bytes: None,
157+
}
155158
}
156159

157160
async fn on_submit_calvin_inbox(

nodedb-cluster-tests/tests/wire_version_handshake.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,10 @@ impl RaftRpcHandler for EchoHandler {
117117
&self,
118118
_req: nodedb_cluster::rpc_codec::SubmitCalvinTxnRequest,
119119
) -> nodedb_cluster::rpc_codec::SubmitCalvinTxnResponse {
120-
nodedb_cluster::rpc_codec::SubmitCalvinTxnResponse { error: None }
120+
nodedb_cluster::rpc_codec::SubmitCalvinTxnResponse {
121+
error: None,
122+
payload_bytes: None,
123+
}
121124
}
122125

123126
async fn on_submit_calvin_inbox(
@@ -251,6 +254,7 @@ impl RaftRpcHandler for SentinelHandler {
251254
code: 0,
252255
message: "sentinel: unexpected submit-calvin-txn dispatch".into(),
253256
}),
257+
payload_bytes: None,
254258
}
255259
}
256260

nodedb-cluster/src/bootstrap/join.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,10 @@ mod tests {
621621
&self,
622622
_req: crate::rpc_codec::SubmitCalvinTxnRequest,
623623
) -> crate::rpc_codec::SubmitCalvinTxnResponse {
624-
crate::rpc_codec::SubmitCalvinTxnResponse { error: None }
624+
crate::rpc_codec::SubmitCalvinTxnResponse {
625+
error: None,
626+
payload_bytes: None,
627+
}
625628
}
626629

627630
async fn on_submit_calvin_inbox(

nodedb-cluster/src/raft_loop/handle_rpc/shuffle_calvin.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ impl<A: CommitApplier, P: PlanExecutor> RaftLoop<A, P> {
157157
code: 0,
158158
message: "calvin-submit not configured (no CalvinSubmit installed)".into(),
159159
}),
160+
payload_bytes: None,
160161
},
161162
}
162163
}

nodedb-cluster/src/rpc_codec/calvin_submit.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,16 @@ pub struct SubmitCalvinTxnRequest {
5252
/// leader-side submit-and-await failed (sequencer rejected the txn, assignment /
5353
/// completion timed out, a channel closed, the `TxClass` failed to decode, or no
5454
/// Calvin-submit hook is configured).
55+
///
56+
/// `payload_bytes` carries the applied transaction's RETURNING rows (the
57+
/// Data-Plane response payload) back to a remote coordinator when the submitted
58+
/// write had a RETURNING clause; it is `None` for plain writes with no rows to
59+
/// surface. These are a QUERY RESULT, not replicated state — they ride this
60+
/// non-Raft RPC response, never the sequencer Raft log.
5561
#[derive(Debug, Clone, rkyv::Archive, rkyv::Serialize, rkyv::Deserialize)]
5662
pub struct SubmitCalvinTxnResponse {
5763
pub error: Option<TypedClusterError>,
64+
pub payload_bytes: Option<Vec<u8>>,
5865
}
5966

6067
/// Coordinator → sequencer-leader routed Calvin-INBOX request (Cv1).
@@ -223,8 +230,12 @@ mod tests {
223230

224231
#[test]
225232
fn roundtrip_submit_calvin_txn_response_ok() {
226-
let decoded = roundtrip_resp(SubmitCalvinTxnResponse { error: None });
233+
let decoded = roundtrip_resp(SubmitCalvinTxnResponse {
234+
error: None,
235+
payload_bytes: Some(vec![0xAA, 0xBB]),
236+
});
227237
assert!(decoded.error.is_none());
238+
assert_eq!(decoded.payload_bytes, Some(vec![0xAA, 0xBB]));
228239
}
229240

230241
#[test]
@@ -234,6 +245,7 @@ mod tests {
234245
code: 0,
235246
message: "calvin-submit not configured".into(),
236247
}),
248+
payload_bytes: None,
237249
});
238250
match decoded.error {
239251
Some(TypedClusterError::Internal { code, message }) => {

nodedb-cluster/src/transport/client/tests.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ impl RaftRpcHandler for EchoHandler {
129129
&self,
130130
_req: crate::rpc_codec::SubmitCalvinTxnRequest,
131131
) -> crate::rpc_codec::SubmitCalvinTxnResponse {
132-
crate::rpc_codec::SubmitCalvinTxnResponse { error: None }
132+
crate::rpc_codec::SubmitCalvinTxnResponse {
133+
error: None,
134+
payload_bytes: None,
135+
}
133136
}
134137

135138
async fn on_submit_calvin_inbox(

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

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

86+
/// Whether any plan in `plans` carries a RETURNING clause.
87+
///
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 {
94+
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+
)
116+
})
117+
}
118+
86119
impl Scheduler {
87120
/// Whether THIS node is currently the leader of the data-group owning this
88121
/// scheduler's vshard.
@@ -182,6 +215,7 @@ impl Scheduler {
182215
return;
183216
}
184217
};
218+
let has_returning = plans_have_returning(&plans);
185219
let plan = PhysicalPlan::Meta(MetaOp::CalvinExecuteStatic {
186220
epoch,
187221
position,
@@ -260,6 +294,7 @@ impl Scheduler {
260294
// no-determinism: dispatch_time is scheduler observability, not Calvin WAL data
261295
dispatch_time: dispatch_instant,
262296
lock_acquired_time,
297+
has_returning,
263298
},
264299
);
265300
}
@@ -310,6 +345,7 @@ impl Scheduler {
310345
return;
311346
}
312347
};
348+
let has_returning = plans_have_returning(&plans);
313349
let plan = PhysicalPlan::Meta(MetaOp::CalvinExecuteActive {
314350
epoch,
315351
position,
@@ -389,6 +425,7 @@ impl Scheduler {
389425
// no-determinism: dispatch_time is scheduler observability, not Calvin WAL data
390426
dispatch_time: dispatch_instant,
391427
lock_acquired_time,
428+
has_returning,
392429
},
393430
);
394431
}

0 commit comments

Comments
 (0)