Skip to content

Commit 1a8747d

Browse files
committed
feat(control): thread real read watermarks through cross-node gather
ExecuteResponse now carries watermark_lsn so a remote-homed read reports the executing core's actual committed LSN instead of the former hardcoded Lsn::ZERO. The gateway exposes execute_with_watermarks to surface per-shard (vshard, lsn) pairs alongside the collected payloads, and dispatch_route returns a DispatchOutcome so local and remote dispatch report watermarks uniformly; remote dispatch is split out into its own module to keep the outcome-returning path readable. Graph traversal and cross-shard scatter/gather call sites are updated to consume the new outcome shape.
1 parent 4a17903 commit 1a8747d

13 files changed

Lines changed: 524 additions & 327 deletions

File tree

nodedb-cluster-tests/tests/cluster_execute_request.rs

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use nodedb_cluster::rpc_codec::{
2121
DescriptorVersionEntry, ExecuteRequest, RaftRpc, TypedClusterError,
2222
};
2323
use nodedb_physical::physical_plan::wire as plan_wire;
24-
use nodedb_physical::physical_plan::{KvOp, PhysicalPlan};
24+
use nodedb_physical::physical_plan::{DocumentOp, KvOp, PhysicalPlan};
2525

2626
/// Build an `ExecuteRequest` wrapping a trivial `KvOp::Put`.
2727
fn make_kv_put_request(
@@ -102,6 +102,81 @@ async fn execute_request_deadline_exceeded_immediate() {
102102
node1.shutdown().await;
103103
}
104104

105+
#[tokio::test(flavor = "multi_thread", worker_threads = 6)]
106+
async fn execute_request_read_carries_watermark_lsn() {
107+
// Single-node: create a document_schemaless collection, commit a write, then
108+
// ship a Document `Scan` via `ExecuteRequest` and assert the response body
109+
// carries a non-zero read watermark LSN. Before the wire gained
110+
// `ExecuteResponse.watermark_lsn`, the non-streaming read path implicitly
111+
// reported 0 (the cross-node gather hardcoded `Lsn::ZERO`); this proves the
112+
// producer captures the executing core's real committed LSN and it
113+
// roundtrips over the wire.
114+
let node1 = common::cluster_harness::TestClusterNode::spawn(1, vec![])
115+
.await
116+
.expect("spawn node 1");
117+
tokio::time::sleep(Duration::from_millis(300)).await;
118+
119+
node1
120+
.exec("CREATE COLLECTION watermark_scan_test WITH (engine='document_schemaless')")
121+
.await
122+
.expect("create collection");
123+
124+
// Commit a write so the leader's WAL LSN advances past zero.
125+
node1
126+
.exec("INSERT INTO watermark_scan_test (id, body) VALUES ('d1', 'hello')")
127+
.await
128+
.expect("insert document");
129+
130+
// Let the metadata + write commit and the Data Plane register the collection.
131+
tokio::time::sleep(Duration::from_millis(300)).await;
132+
133+
let transport = node1
134+
.shared
135+
.cluster_transport
136+
.as_ref()
137+
.expect("cluster_transport");
138+
139+
// Empty descriptor_versions → no descriptor-version validation on the
140+
// receiver, so the scan reaches the local executor unconditionally.
141+
let scan = PhysicalPlan::Document(DocumentOp::Scan {
142+
collection: "watermark_scan_test".into(),
143+
limit: 100,
144+
offset: 0,
145+
sort_keys: vec![],
146+
filters: vec![],
147+
distinct: false,
148+
projection: vec![],
149+
computed_columns: vec![],
150+
window_functions: vec![],
151+
system_time: nodedb_types::SystemTimeScope::Current,
152+
valid_at_ms: None,
153+
prefilter: None,
154+
});
155+
let req = ExecuteRequest {
156+
plan_bytes: plan_wire::encode(&scan).expect("encode scan plan"),
157+
tenant_id: 0,
158+
database_id: 0,
159+
deadline_remaining_ms: 5000,
160+
trace_id: [0u8; 16],
161+
descriptor_versions: vec![],
162+
};
163+
164+
let resp = send_execute_request(transport, node1.listen_addr, req).await;
165+
166+
assert!(
167+
resp.success,
168+
"scan should succeed, got error: {:?}",
169+
resp.error
170+
);
171+
assert!(
172+
resp.watermark_lsn > 0,
173+
"read response must carry the executing core's real committed LSN, got {}",
174+
resp.watermark_lsn
175+
);
176+
177+
node1.shutdown().await;
178+
}
179+
105180
#[tokio::test(flavor = "multi_thread", worker_threads = 6)]
106181
async fn execute_request_descriptor_mismatch() {
107182
// Single-node: create a collection, then send an ExecuteRequest with

nodedb-cluster/src/rpc_codec/execute.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ pub struct ExecuteResponse {
4747
/// Raw Data Plane response payloads, one per result set.
4848
pub payloads: Vec<Vec<u8>>,
4949
pub error: Option<TypedClusterError>,
50+
/// Max read watermark LSN observed by the executing node's cores; 0 for
51+
/// writes/errors. Mirrors [`ExecuteStreamChunk::watermark_lsn`]: raw `u64`
52+
/// on the wire, converted to `Lsn` at the coordinator via `Lsn::new`.
53+
pub watermark_lsn: u64,
5054
}
5155

5256
/// Typed error returned by the remote executor.
@@ -97,18 +101,20 @@ pub struct ExecuteStreamEnd {
97101
}
98102

99103
impl ExecuteResponse {
100-
pub fn ok(payloads: Vec<Vec<u8>>) -> Self {
104+
pub fn ok(payloads: Vec<Vec<u8>>, watermark_lsn: u64) -> Self {
101105
Self {
102106
success: true,
103107
payloads,
104108
error: None,
109+
watermark_lsn,
105110
}
106111
}
107112
pub fn err(error: TypedClusterError) -> Self {
108113
Self {
109114
success: false,
110115
payloads: vec![],
111116
error: Some(error),
117+
watermark_lsn: 0,
112118
}
113119
}
114120
}
@@ -268,12 +274,16 @@ mod tests {
268274

269275
#[test]
270276
fn roundtrip_execute_response_success() {
271-
let resp = ExecuteResponse::ok(vec![b"row1".to_vec(), b"row2".to_vec()]);
277+
let resp = ExecuteResponse::ok(vec![b"row1".to_vec(), b"row2".to_vec()], 0xCAFE_1234);
272278
let decoded = roundtrip_resp(resp);
273279
assert!(decoded.success);
274280
assert_eq!(decoded.payloads.len(), 2);
275281
assert_eq!(decoded.payloads[0], b"row1");
276282
assert!(decoded.error.is_none());
283+
assert_eq!(
284+
decoded.watermark_lsn, 0xCAFE_1234,
285+
"read watermark roundtrips on the response body"
286+
);
277287
}
278288

279289
#[test]
@@ -286,6 +296,10 @@ mod tests {
286296
});
287297
let decoded = roundtrip_resp(resp);
288298
assert!(!decoded.success);
299+
assert_eq!(
300+
decoded.watermark_lsn, 0,
301+
"error responses carry no watermark"
302+
);
289303
match decoded.error {
290304
Some(TypedClusterError::NotLeader {
291305
group_id,

nodedb/src/control/exec_receiver/executor.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@ impl LocalPlanExecutor {
271271
)
272272
.await
273273
{
274-
Ok(payload) => ExecuteResponse::ok(vec![payload]),
274+
// Replicated writes carry no read watermark → 0.
275+
Ok(payload) => ExecuteResponse::ok(vec![payload], 0),
275276
Err(e) => ExecuteResponse::err(TypedClusterError::Internal {
276277
code: PLAN_DECODE_FAILED,
277278
message: e.to_string(),
@@ -285,7 +286,9 @@ impl LocalPlanExecutor {
285286
)
286287
.await
287288
{
288-
Ok(Ok(result)) => ExecuteResponse::ok(vec![result.payload]),
289+
Ok(Ok(result)) => {
290+
ExecuteResponse::ok(vec![result.payload], result.watermark_lsn.as_u64())
291+
}
289292
Ok(Err(e)) => ExecuteResponse::err(TypedClusterError::Internal {
290293
code: PLAN_DECODE_FAILED,
291294
message: e.to_string(),

nodedb/src/control/gateway/core.rs

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use tracing::{Instrument, debug, info_span};
2727
use crate::Error;
2828
use crate::control::state::SharedState;
2929
use crate::control::trace_export::EmitSpanParams;
30-
use crate::types::{DatabaseId, TenantId, TraceId};
30+
use crate::types::{DatabaseId, Lsn, TenantId, TraceId, VShardId};
3131
use nodedb_physical::physical_plan::PhysicalPlan;
3232

3333
use super::dispatcher::{default_deadline_ms, dispatch_route};
@@ -106,11 +106,33 @@ impl Gateway {
106106
///
107107
/// Returns one `Vec<u8>` payload per vShard result. For point operations
108108
/// the returned Vec has exactly one element.
109+
///
110+
/// Thin wrapper over [`Gateway::execute_with_watermarks`] that discards the
111+
/// per-shard read watermarks — for the ~5 existing callers that only need
112+
/// payloads.
109113
pub async fn execute(
110114
&self,
111115
ctx: &QueryContext,
112116
plan: PhysicalPlan,
113117
) -> Result<Vec<Vec<u8>>, Error> {
118+
self.execute_with_watermarks(ctx, plan)
119+
.await
120+
.map(|(payloads, _watermarks)| payloads)
121+
}
122+
123+
/// Execute a pre-planned `PhysicalPlan`, returning both the raw payloads and
124+
/// the per-shard read watermarks observed across every dispatched route.
125+
///
126+
/// Each `(vshard, watermark_lsn)` entry is one participating shard's real
127+
/// committed LSN (local SPSC response watermark or the remote's
128+
/// `ExecuteResponse.watermark_lsn`). The cross-node gather consumer folds
129+
/// these into the transaction read-set so a remote-homed read records the
130+
/// remote's actual LSN instead of the former hardcoded `Lsn::ZERO`.
131+
pub async fn execute_with_watermarks(
132+
&self,
133+
ctx: &QueryContext,
134+
plan: PhysicalPlan,
135+
) -> Result<(Vec<Vec<u8>>, Vec<(VShardId, Lsn)>), Error> {
114136
let shared = self.shared()?;
115137
let span = info_span!(
116138
"gateway.execute",
@@ -201,7 +223,8 @@ impl Gateway {
201223
debug!(sql = %sql, "gateway: plan cache hit (two-phase)");
202224
return self
203225
.execute_with_version_set(ctx, (*cached_plan).clone(), stored_vs)
204-
.await;
226+
.await
227+
.map(|(payloads, _watermarks)| payloads);
205228
}
206229
}
207230
// Stored version set is stale or plan was evicted — fall through
@@ -225,16 +248,22 @@ impl Gateway {
225248
.insert_version_set(sql_key, actual_vs.clone());
226249
self.plan_cache.insert(actual_key, Arc::new(plan.clone()));
227250

228-
self.execute_with_version_set(ctx, plan, actual_vs).await
251+
self.execute_with_version_set(ctx, plan, actual_vs)
252+
.await
253+
.map(|(payloads, _watermarks)| payloads)
229254
}
230255

231256
/// Core execution path: route → dispatch with retry → fuse.
257+
///
258+
/// Returns the fused/collected payloads alongside every route's per-shard
259+
/// read watermarks (one `(vshard, watermark_lsn)` per participating shard,
260+
/// accumulated across routes — never collapsed).
232261
async fn execute_with_version_set(
233262
&self,
234263
ctx: &QueryContext,
235264
plan: PhysicalPlan,
236265
version_set: GatewayVersionSet,
237-
) -> Result<Vec<Vec<u8>>, Error> {
266+
) -> Result<(Vec<Vec<u8>>, Vec<(VShardId, Lsn)>), Error> {
238267
let shared = self.shared()?;
239268
let routes = self.compute_routes(plan, ctx)?;
240269

@@ -245,6 +274,7 @@ impl Gateway {
245274
// N × cap across routes.
246275
let max_total_bytes = shared.tuning.network.max_query_result_bytes as usize;
247276
let mut all_payloads: Vec<Vec<u8>> = Vec::new();
277+
let mut all_shard_watermarks: Vec<(VShardId, Lsn)> = Vec::new();
248278
let mut accumulated_bytes: usize = 0;
249279

250280
for route in routes {
@@ -258,7 +288,7 @@ impl Gateway {
258288
let retry_counter = Arc::clone(&self.not_leader_retry_count);
259289
let version_set_for_route = version_set.clone();
260290
let shared_for_route = Arc::clone(&shared);
261-
let payloads = retry_not_leader(routing_ref, move |attempt| {
291+
let outcome = retry_not_leader(routing_ref, move |attempt| {
262292
if attempt > 0 {
263293
retry_counter.fetch_add(1, Ordering::Relaxed);
264294
}
@@ -324,7 +354,12 @@ impl Gateway {
324354
e
325355
})?;
326356

327-
for p in payloads {
357+
// Accumulate this route's per-shard watermarks — one entry per
358+
// participating shard, never collapsed to a scalar, so a multi-route
359+
// read produces one read-set entry per shard.
360+
all_shard_watermarks.extend(outcome.shard_watermarks);
361+
362+
for p in outcome.payloads {
328363
accumulated_bytes = accumulated_bytes.saturating_add(p.len());
329364
if accumulated_bytes > max_total_bytes {
330365
return Err(Error::ExecutionLimitExceeded {
@@ -338,12 +373,14 @@ impl Gateway {
338373
}
339374
}
340375

341-
// For broadcast scans, fuse all shard payloads into one.
376+
// For broadcast scans, fuse all shard payloads into one. The per-shard
377+
// watermarks are NOT fused — each participating shard keeps its own
378+
// read-set entry.
342379
if all_payloads.len() > 1 {
343380
let fused = fuse_payloads(all_payloads)?;
344-
Ok(vec![fused.payload])
381+
Ok((vec![fused.payload], all_shard_watermarks))
345382
} else {
346-
Ok(all_payloads)
383+
Ok((all_payloads, all_shard_watermarks))
347384
}
348385
}
349386

0 commit comments

Comments
 (0)