Skip to content

Commit c1ef0d8

Browse files
committed
fix(control): route single-vShard-homed gather to its owning core
A no-GROUP BY scalar aggregate on a single-vShard-homed collection (document/kv/columnar/timeseries/spatial/vector/text) plans as Exchange{Gather} over an Aggregate with no input. In single-node mode this was broadcast to every core, so every empty non-owning core seeded its own scalar-aggregate identity row and the merge returned N rows instead of one. gather_all_vshards now delegates to gather_single_node, which mirrors the cluster branch's routing: cluster-partitioned leaves still broadcast, but a single-vShard-homed plan dispatches to its one owning core alone.
1 parent 57fc6b7 commit c1ef0d8

4 files changed

Lines changed: 208 additions & 4 deletions

File tree

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,8 @@ pub fn gather_all_cores_stream(
323323
///
324324
/// # Single-node mode
325325
///
326-
/// If `state.gateway` is `None` this degenerates to [`gather_all_cores`] with
327-
/// unchanged behaviour.
326+
/// If `state.gateway` is `None`, routing is delegated to
327+
/// [`super::owning_core::gather_single_node`] (same shape-based routing).
328328
///
329329
/// # Cluster mode — single-vShard-homed sources (document, kv, columnar,
330330
/// timeseries, spatial, vector, text)
@@ -359,8 +359,18 @@ pub async fn gather_all_vshards(
359359
txn_id: Option<TxnId>,
360360
) -> crate::Result<GatherOutcome> {
361361
let Some(gateway) = state.gateway.get() else {
362-
// Single-node: delegate to the local fan-out path unchanged.
363-
return gather_all_cores(state, tenant_id, database_id, plan, trace_id, txn_id).await;
362+
// Single-node: route by plan shape (cluster-partitioned leaf → broadcast;
363+
// single-vShard-homed collection → its one owning core; else broadcast
364+
// fallback), mirroring the cluster branch below.
365+
return super::owning_core::gather_single_node(
366+
state,
367+
tenant_id,
368+
database_id,
369+
plan,
370+
trace_id,
371+
txn_id,
372+
)
373+
.await;
364374
};
365375

366376
if nodedb_physical::physical_plan::plan_contains_cluster_partitioned_leaf(&plan) {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
pub mod all_cores;
1212
pub mod full_scan;
1313
pub mod gather;
14+
pub mod owning_core;
1415
pub mod resolve;
1516
pub mod streamable;
1617

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! Route a single-vShard-homed plan to its ONE owning Data-Plane core.
4+
//!
5+
//! `gather_single_owning_core` is the single-node sibling of
6+
//! [`super::gather::gather_all_cores`] for plans whose whole collection lives on
7+
//! exactly one vShard (document / kv / columnar / timeseries / spatial / vector
8+
//! / text). Instead of broadcasting the plan to every core — which seeds an
9+
//! identity scalar-aggregate row on each empty non-owning core and returns one
10+
//! row per core for a no-`GROUP BY` aggregate — it resolves the collection's
11+
//! owning vShard and dispatches the bare plan to it alone, exactly mirroring
12+
//! what the cluster branch of [`super::gather::gather_all_vshards`] does via the
13+
//! gateway. The owning core already holds every row of a single-vShard-homed
14+
//! collection, so this returns the identical row set a broadcast would have,
15+
//! minus the empty cores' spurious contributions.
16+
17+
use crate::bridge::envelope::{ErrorCode, PhysicalPlan, Status};
18+
use crate::control::server::dispatch_utils::dispatch_to_data_plane_with_txn;
19+
use crate::control::server::payload_merge::{encode_msgpack_array, extract_msgpack_elements};
20+
use crate::control::state::SharedState;
21+
use crate::types::{DatabaseId, TenantId, TraceId, TxnId, VShardId};
22+
23+
use super::gather::{GatherOutcome, gather_all_cores};
24+
25+
/// Single-node routing decision for a resolved `Exchange{Gather}` child plan.
26+
///
27+
/// Mirrors the cluster branch of [`super::gather::gather_all_vshards`]:
28+
///
29+
/// - A cluster-partitioned leaf (graph traversal / array) spreads its rows
30+
/// across cores by node-id / tile-id, so it fans to every local core via
31+
/// [`gather_all_cores`].
32+
/// - A single-vShard-homed collection (document / kv / columnar / timeseries /
33+
/// spatial / vector / text) lives wholly on ONE core; the bare plan routes to
34+
/// that owning core via [`gather_single_owning_core`]. Broadcasting would seed
35+
/// a scalar-aggregate identity row on every empty non-owning core — so a
36+
/// no-`GROUP BY` aggregate returns one row PER core instead of one merged row
37+
/// — and would duplicate a plain scan's rows across cores.
38+
/// - A plan with no resolvable collection (e.g. `ProviderScan` carrying embedded
39+
/// rows) keeps the broadcast fallback unchanged.
40+
pub async fn gather_single_node(
41+
state: &SharedState,
42+
tenant_id: TenantId,
43+
database_id: DatabaseId,
44+
plan: PhysicalPlan,
45+
trace_id: TraceId,
46+
txn_id: Option<TxnId>,
47+
) -> crate::Result<GatherOutcome> {
48+
if nodedb_physical::physical_plan::plan_contains_cluster_partitioned_leaf(&plan) {
49+
return gather_all_cores(state, tenant_id, database_id, plan, trace_id, txn_id).await;
50+
}
51+
if let Some(collection) = plan.collection() {
52+
let vshard_id = VShardId::from_collection_in_database(database_id, collection);
53+
return gather_single_owning_core(
54+
state,
55+
tenant_id,
56+
database_id,
57+
plan,
58+
vshard_id,
59+
trace_id,
60+
txn_id,
61+
)
62+
.await;
63+
}
64+
gather_all_cores(state, tenant_id, database_id, plan, trace_id, txn_id).await
65+
}
66+
67+
/// Dispatch `plan` to the single Data-Plane core that owns `vshard_id` and
68+
/// gather the one bounded response into a [`GatherOutcome`].
69+
///
70+
/// `vshard_id` is the collection's owning vShard
71+
/// (`VShardId::from_collection_in_database(database_id, collection)`); the
72+
/// dispatcher's `VShardRouter` resolves it to the one core holding the
73+
/// collection's rows.
74+
///
75+
/// The returned outcome carries that core's own `watermark_lsn` /
76+
/// `read_version_lsn` and exactly one `shard_watermarks` entry keyed to the
77+
/// collection's vShard — matching the cluster `dispatch_local` path so an
78+
/// in-transaction read records the same OCC read-set entry the write-set uses
79+
/// (writes home to the same `from_collection_in_database` vShard). Aggregate
80+
/// finalization (`finalize_aggregate`) is a passthrough over the merged array,
81+
/// so one complete aggregate row in yields one row out.
82+
pub async fn gather_single_owning_core(
83+
state: &SharedState,
84+
tenant_id: TenantId,
85+
database_id: DatabaseId,
86+
plan: PhysicalPlan,
87+
vshard_id: VShardId,
88+
trace_id: TraceId,
89+
txn_id: Option<TxnId>,
90+
) -> crate::Result<GatherOutcome> {
91+
// `Box::pin` breaks an async-fn recursion cycle: `dispatch_to_data_plane_*`
92+
// re-enters `resolve_exchange_in_plan`. The plan handed here is the bare,
93+
// Exchange-free child of the resolved Gather, so the re-entrant resolve is a
94+
// no-op — but the future must be heap-indirected so its size stays finite.
95+
let resp = Box::pin(dispatch_to_data_plane_with_txn(
96+
state,
97+
tenant_id,
98+
database_id,
99+
vshard_id,
100+
plan,
101+
trace_id,
102+
txn_id,
103+
))
104+
.await?;
105+
106+
// Propagate a Data-Plane error status the same way `gather_all_cores` does:
107+
// a `NotFound` from the owning core means "no such slice here" and reads
108+
// back as an empty (still validatable) observation; any OTHER error status
109+
// must surface as a dispatch error rather than being silently swallowed as
110+
// an empty success (e.g. an FTS query-validation rejection, a constraint
111+
// error, or a deadline). Without this the owning-core path would drop every
112+
// Data-Plane error on a single-vShard read.
113+
if resp.status == Status::Error
114+
&& let Some(ec) = resp.error_code.as_deref()
115+
&& !matches!(ec, ErrorCode::NotFound)
116+
{
117+
return Err(crate::Error::Dispatch {
118+
detail: format!("{ec:?}"),
119+
});
120+
}
121+
122+
let payload_bytes: &[u8] = resp.payload.as_ref();
123+
let all_elements = extract_msgpack_elements(payload_bytes);
124+
let merged_array = encode_msgpack_array(&all_elements);
125+
126+
Ok(GatherOutcome {
127+
raw: payload_bytes.to_vec(),
128+
merged_array,
129+
watermark_lsn: resp.watermark_lsn,
130+
read_version_lsn: resp.read_version_lsn,
131+
shard_watermarks: vec![(vshard_id, resp.watermark_lsn)],
132+
})
133+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! Regression: a no-`GROUP BY` scalar aggregate on a single-vShard-homed
4+
//! collection must merge to ONE row on a multi-core server.
5+
//!
6+
//! A scalar aggregate with no `GROUP BY` plans as
7+
//! `QueryOp::Aggregate { input: None }` wrapped in `Exchange{Gather}`. In
8+
//! single-node mode the gather formerly BROADCAST the plan to every core; the
9+
//! collection's rows live on ONE owning core, so every other (empty) core
10+
//! seeded its own scalar-aggregate identity row and the coordinator merge is a
11+
//! passthrough — yielding N rows (N-1 identity rows + 1 real row) instead of
12+
//! one. The fix routes single-vShard-homed plans to their one owning core.
13+
//! A single-core harness masks the bug, so these tests drive an 8-core server.
14+
15+
mod common;
16+
17+
use common::pgwire_harness::TestServer;
18+
19+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
20+
async fn scalar_count_star_merges_to_one_row_multicore() {
21+
let srv = TestServer::start_multicores(8).await;
22+
srv.exec(
23+
"CREATE COLLECTION t \
24+
COLUMNS (id TEXT PRIMARY KEY, v INTEGER) \
25+
WITH (engine='document_strict')",
26+
)
27+
.await
28+
.unwrap();
29+
srv.exec("INSERT INTO t (id, v) VALUES ('a',1),('b',2),('c',3)")
30+
.await
31+
.unwrap();
32+
33+
let rows = srv.query_rows("SELECT count(*) FROM t").await.unwrap();
34+
assert_eq!(
35+
rows.len(),
36+
1,
37+
"scalar count(*) must merge to ONE row across cores, got {rows:?}"
38+
);
39+
assert_eq!(rows[0][0], "3");
40+
41+
let rows = srv
42+
.query_rows("SELECT count(*) AS c, sum(v) AS s FROM t")
43+
.await
44+
.unwrap();
45+
assert_eq!(
46+
rows.len(),
47+
1,
48+
"aliased scalar aggregate must be one row, got {rows:?}"
49+
);
50+
assert_eq!(rows[0][0], "3");
51+
// `sum` over an INTEGER column renders as a float ("6.0"); the merge
52+
// correctness this test guards is the single row + right value, so compare
53+
// the value numerically rather than pinning its textual formatting.
54+
assert_eq!(
55+
rows[0][1].parse::<f64>().expect("numeric sum"),
56+
6.0,
57+
"aliased sum must carry the merged value, got {:?}",
58+
rows[0][1]
59+
);
60+
}

0 commit comments

Comments
 (0)