Skip to content

Commit d6d2b69

Browse files
committed
test(control): pin foreign-collection read widens Calvin dispatch to multi-shard
Guards the read_vshards_of + classify_dispatch seam that interactive COMMIT relies on: a write buffered on one collection's vShard plus a read-set entry on a different collection must classify MultiShard, not SingleShard, so the transaction is validated through the Calvin barrier instead of only the local-WAL conflict check.
1 parent 1a8747d commit d6d2b69

1 file changed

Lines changed: 79 additions & 1 deletion

File tree

nodedb/src/control/planner/calvin/dispatch_tests.rs

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ use crate::Error;
99
use crate::control::planner::calvin::cross_shard_mode::CrossShardTxnMode;
1010
use crate::control::planner::calvin::types::{DispatchClass, DispatchOutcome};
1111
use crate::control::server::shared::session::TransactionState;
12-
use crate::types::{TenantId, VShardId};
12+
use crate::control::server::shared::session::read_set::{EngineTag, ReadKey, ReadSetEntry};
13+
use crate::types::{DatabaseId, Lsn, TenantId, VShardId};
1314
use nodedb_physical::physical_plan::{ColumnarOp, CrdtOp, DocumentOp, PhysicalPlan};
1415
use nodedb_physical::physical_task::{PhysicalTask, PostSetOp};
1516

@@ -242,6 +243,83 @@ fn classify_dispatch_read_widened_multi_shard() {
242243
}
243244
}
244245

246+
/// Find two collection names whose `DatabaseId::DEFAULT`-scoped vShard ids
247+
/// differ, so a write homed to one and a read homed to the other genuinely span
248+
/// two vShards. Mirrors the same-named helper the cross-node cluster tests use.
249+
fn two_distinct_vshard_collections() -> (String, String) {
250+
let mut first: Option<(String, u32)> = None;
251+
for i in 0u32..512 {
252+
let name = format!("dispatch_home_{i}");
253+
let vshard = VShardId::from_collection_in_database(DatabaseId::DEFAULT, &name).as_u32();
254+
match first {
255+
Some((ref fname, fv)) if fv != vshard => return (fname.clone(), name),
256+
None => first = Some((name, vshard)),
257+
_ => {}
258+
}
259+
}
260+
panic!("could not find two distinct-vshard collections in 512 tries");
261+
}
262+
263+
#[test]
264+
fn read_entry_on_foreign_collection_widens_class_to_multishard() {
265+
// Regression pin for the cross-node "silent-pass" serializability hole.
266+
//
267+
// A transaction that BUFFERS a write on collection A's vShard and READS a
268+
// DIFFERENT collection B must classify `MultiShard`, because the read-set
269+
// entry for B homes (via `read_vshards_of`) to B's own vShard and widens the
270+
// participant set. This exercises the real routing seam: `read_vshards_of`
271+
// homing + `classify_dispatch` union, exactly as interactive COMMIT invokes
272+
// them (`session::commit::run_commit`).
273+
//
274+
// WHY this must stay `MultiShard`: only the `MultiShard` branch of COMMIT
275+
// flushes through the Calvin barrier (`run_commit_calvin`), which validates
276+
// B's read slice on B's OWNING node using the real per-shard `read_lsn`. If a
277+
// foreign read failed to widen the class, COMMIT would take the `SingleShard`
278+
// branch and run only the local-WAL `si_conflict_abort`, which never sees a
279+
// stale read on the remote owner — silently committing a non-serializable
280+
// cross-node transaction. This test guarantees a future refactor of
281+
// `read_vshards_of` / `classify_dispatch` cannot reopen that hole.
282+
let (write_coll, read_coll) = two_distinct_vshard_collections();
283+
let write_vshard =
284+
VShardId::from_collection_in_database(DatabaseId::DEFAULT, &write_coll).as_u32();
285+
let read_vshard =
286+
VShardId::from_collection_in_database(DatabaseId::DEFAULT, &read_coll).as_u32();
287+
288+
let tasks = vec![doc_insert_task(write_vshard)];
289+
290+
let read_entry = ReadSetEntry {
291+
engine: EngineTag::Document,
292+
database_id: DatabaseId::DEFAULT,
293+
tenant_id: TenantId::new(1),
294+
collection: read_coll.clone(),
295+
key: ReadKey::Predicate,
296+
read_lsn: Lsn::new(1),
297+
};
298+
299+
// The homing step under test: a foreign-collection read must home to a
300+
// vShard distinct from the write's, contributing a new participant.
301+
let read_vshards = read_vshards_of(std::slice::from_ref(&read_entry));
302+
assert!(
303+
read_vshards.contains(&read_vshard) && !read_vshards.contains(&write_vshard),
304+
"read entry for `{read_coll}` must home to vShard {read_vshard}, not the write's {write_vshard}"
305+
);
306+
307+
match classify_dispatch(&tasks, &read_vshards) {
308+
DispatchClass::MultiShard { vshards } => {
309+
assert!(
310+
vshards.contains(&write_vshard) && vshards.contains(&read_vshard),
311+
"cross-collection read must widen the class to include both the write \
312+
vShard {write_vshard} and the read vShard {read_vshard}, got {vshards:?}"
313+
);
314+
}
315+
other => panic!(
316+
"expected MultiShard for write-on-{write_vshard} + foreign-read-on-{read_vshard}, \
317+
got {other:?} (a SingleShard here would route COMMIT to local-WAL \
318+
si_conflict_abort and reopen the cross-node serializability hole)"
319+
),
320+
}
321+
}
322+
245323
#[test]
246324
fn classify_dispatch_read_on_write_shard_stays_single() {
247325
// Reading the same shard the writes target does not widen the class.

0 commit comments

Comments
 (0)