fix(txn,search): route predicate writes outside a block, refuse two writes of one sum balance, refuse the doc_id score alias

docs/architecture.md

@@ -160,6 +160,8 @@ SET cross_shard_txn = 'strict';
 SET cross_shard_txn = 'best_effort_non_atomic';
 ```
 
+In this mode a predicate write and the shipped balance of a materialized sum commit as separate groups. A failure of one group leaves the balance unequal to the source sum.
+
 (Bare `best_effort` is deliberately rejected; invalid values return SQLSTATE `22023`.)
 
 **Single-node deployments always run Calvin.** A node with no `[cluster]` section synthesizes a one-node cluster with its own sequencer group, so transactions spanning multiple cores (vShards) commit atomically. Uncontended single-shard point writes bypass the sequencer entirely and go directly through the relevant data-group Raft; contended or predicate/bulk writes route through the deterministic scheduler.

nodedb-cluster-tests/tests/common_suite/cases/materialized_sum_cross_shard.rs

@@ -23,18 +23,18 @@ use nodedb::types::DatabaseId;
 
 /// Source and target, chosen for readability rather than for their hashes — the
 /// homing assertion below is what makes the choice meaningful.
-const SOURCE: &str = "xs_entries";
-const TARGET: &str = "xs_accounts";
+pub(super) const SOURCE: &str = "xs_entries";
+pub(super) const TARGET: &str = "xs_accounts";
 
-fn pg_detail(e: &tokio_postgres::Error) -> String {
+pub(super) fn pg_detail(e: &tokio_postgres::Error) -> String {
     match e.as_db_error() {
         Some(db) => format!("{}: {}", db.code().code(), db.message()),
         None => format!("{e}"),
     }
 }
 
 /// The balance column of one `TARGET` row, as read through `client`.
-async fn balance_of(client: &tokio_postgres::Client, account: &str) -> String {
+pub(super) async fn balance_of(client: &tokio_postgres::Client, account: &str) -> String {
     let rows = client
         .simple_query(&format!(
             "SELECT balance FROM {TARGET} WHERE id = '{account}'"
@@ -55,7 +55,7 @@ async fn balance(client: &tokio_postgres::Client) -> String {
 }
 
 /// Create the two collections and declare the binding.
-async fn declare_binding(cluster: &TestCluster) {
+pub(super) async fn declare_binding(cluster: &TestCluster) {
     cluster
         .exec_ddl_on_any_leader(&format!(
             "CREATE COLLECTION {TARGET} (id TEXT PRIMARY KEY, owner TEXT) \
@@ -196,7 +196,12 @@ async fn repeated_cross_shard_inserts_accumulate() {
 /// The seeding entries go in through the INSERT path, which already credits the
 /// target across shards; every test below then exercises a NON-insert shape
 /// against that settled starting total.
-async fn seed(cluster: &TestCluster, account: &str, start: &str, entries: &[(&str, &str)]) {
+pub(super) async fn seed(
+    cluster: &TestCluster,
+    account: &str,
+    start: &str,
+    entries: &[(&str, &str)],
+) {
     cluster.nodes[0]
         .client
         .simple_query(&format!(

nodedb-cluster-tests/tests/common_suite/cases/materialized_sum_gateway_occ.rs

@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: BUSL-1.1
+
+//! A pgwire UPDATE received on a follower keeps its OCC read versions when
+//! the leader of every group it touches is one remote node.
+//!
+//! Node `L` leads both the source group (`xs_entries`) and the target group
+//! (`xs_accounts`, holding the materialized sum). Follower `F` receives
+//! `UPDATE xs_entries SET amount = '40' WHERE id = 'e1'`. Every group the
+//! statement touches has the same remote leader, so placement picks the
+//! gateway forward. The forward must carry the read versions the sum
+//! settlement stamped on the source row. Without them, a change to the
+//! source row on `L` after the settlement read leaves a stale balance delta
+//! to apply.
+//!
+//! The fail gate `stmt::before_route` parks the statement on `F` after
+//! planning and before routing. While it is parked, `L` rewrites the same
+//! source row. After the release and convergence the balance must equal
+//! `100 + sum(source amounts)`. The statement on `F` can commit after a
+//! retry or fail with an OCC error. Both outcomes keep the sum exact.
+//!
+//! Requires `--features failpoints`.
+
+#![cfg(feature = "failpoints")]
+
+use std::path::PathBuf;
+use std::time::Duration;
+
+use nodedb_test_support::fail_point::{FailAction, FailGuard};
+
+use super::materialized_sum_cross_shard::{
+    SOURCE, TARGET, balance_of, declare_binding, pg_detail, seed,
+};
+use crate::common::cluster_harness::shared_steps::group_of;
+use crate::common::cluster_harness::{GroupLeader, TestCluster, wait_for};
+
+/// Opening balance of the account, before any source row counts.
+const START: i64 = 100;
+
+/// How long each wait in the case gets.
+const STEP_DEADLINE: Duration = Duration::from_secs(30);
+
+/// Pause between two polls.
+const STEP_POLL: Duration = Duration::from_millis(20);
+
+/// Bring up the cluster with `nodes[0]` leading the source and target
+/// groups, then seed `acc-1` at `START` with one entry `e1` of 25.
+async fn cluster_led_by_first_node() -> TestCluster {
+    let cluster = TestCluster::spawn_three()
+        .await
+        .expect("spawn 3-node cluster");
+    declare_binding(&cluster).await;
+    cluster.wait_for_full_apply_convergence(STEP_DEADLINE).await;
+
+    let leader = cluster.nodes[0].node_id;
+    for collection in [SOURCE, TARGET] {
+        let group = group_of(&cluster.nodes[0], collection);
+        cluster.transfer_leadership(group, leader).await;
+        let found = cluster
+            .wait_for_group_leader(group, GroupLeader::Node(leader), STEP_DEADLINE)
+            .await;
+        assert_eq!(
+            found, leader,
+            "node {leader} must lead the group of {collection}"
+        );
+        assert_eq!(
+            cluster.group_leader(group),
+            Some(leader),
+            "group {group} of {collection} must have node {leader} as its one leader"
+        );
+    }
+
+    seed(&cluster, "acc-1", &START.to_string(), &[("e1", "25")]).await;
+    assert_eq!(
+        balance_of(&cluster.nodes[0].client, "acc-1").await,
+        "125",
+        "the seeded balance must be 100 + 25"
+    );
+    cluster
+}
+
+/// The sum of every `SOURCE` amount, read through node `idx`.
+async fn source_sum(cluster: &TestCluster, idx: usize) -> f64 {
+    let rows = cluster.nodes[idx]
+        .client
+        .simple_query(&format!("SELECT amount FROM {SOURCE}"))
+        .await
+        .unwrap_or_else(|e| panic!("read source amounts: {}", pg_detail(&e)));
+    rows.into_iter()
+        .filter_map(|m| match m {
+            tokio_postgres::SimpleQueryMessage::Row(r) => r.get(0).map(str::to_string),
+            _ => None,
+        })
+        .map(|amount| {
+            amount
+                .parse::<f64>()
+                .unwrap_or_else(|_| panic!("amount '{amount}' must parse as a number"))
+        })
+        .sum()
+}
+
+/// Assert, on every node, that the balance of `acc-1` equals
+/// `START + sum(source amounts)`.
+async fn assert_balance_equals_source_sum(cluster: &TestCluster) {
+    cluster.wait_for_full_apply_convergence(STEP_DEADLINE).await;
+    for idx in 0..cluster.nodes.len() {
+        let sum = source_sum(cluster, idx).await;
+        let text = balance_of(&cluster.nodes[idx].client, "acc-1").await;
+        let balance: f64 = text
+            .parse()
+            .unwrap_or_else(|_| panic!("balance '{text}' must parse as a number"));
+        assert!(
+            (balance - (START as f64 + sum)).abs() < 1e-9,
+            "node {}: the balance must equal the source sum: balance {balance}, \
+             expected {START} + {sum}",
+            cluster.nodes[idx].node_id
+        );
+    }
+}
+
+/// Send `statement` from a follower that parks it before routing, rewrite
+/// the source row `e1` on the leader meanwhile, release it, and check that
+/// the balance equals the source sum on every node.
+async fn forwarded_statement_then_move_source(statement: &'static str) {
+    let cluster = cluster_led_by_first_node().await;
+    let follower = &cluster.nodes[1];
+    assert_ne!(
+        follower.node_id, cluster.nodes[0].node_id,
+        "the statement must arrive on a node other than the leader"
+    );
+
+    let mut conn = follower.raw_pgwire().await;
+
+    let gate_dir = tempfile::tempdir().expect("gate directory");
+    let release: PathBuf = gate_dir.path().join("release");
+    let parked = gate_dir.path().join("release.parked");
+    let _gate = FailGuard::for_node(
+        follower.node_id,
+        "stmt::before_route",
+        FailAction::WaitForFile(release.clone()),
+    );
+
+    // The raw session panics on an ErrorResponse, so a join error is the
+    // statement failing with an OCC error. That outcome is allowed.
+    let forwarded = tokio::spawn(async move { conn.simple_query(statement).await });
+
+    wait_for(
+        "the follower's statement parks before routing",
+        STEP_DEADLINE,
+        STEP_POLL,
+        || parked.exists(),
+    )
+    .await;
+
+    cluster.nodes[0]
+        .client
+        .simple_query(&format!(
+            "UPDATE {SOURCE} SET amount = '100' WHERE id = 'e1'"
+        ))
+        .await
+        .unwrap_or_else(|e| panic!("update on the leader: {}", pg_detail(&e)));
+
+    std::fs::write(&release, b"go").expect("release the gate");
+    match forwarded.await {
+        Ok(_) => {}
+        Err(join) => eprintln!("the forwarded statement ended with an error: {join}"),
+    }
+
+    assert_balance_equals_source_sum(&cluster).await;
+}
+
+/// A point UPDATE forwarded from a follower to the one leader of all its
+/// groups stays exact when the source row changes on the leader mid-statement.
+#[tokio::test(flavor = "multi_thread", worker_threads = 6)]
+async fn a_forwarded_update_keeps_the_balance_equal_to_the_source_sum() {
+    forwarded_statement_then_move_source("UPDATE xs_entries SET amount = '40' WHERE id = 'e1'")
+        .await;
+}
+
+/// The same for a predicate UPDATE, whose rows the coordinator reads first.
+#[tokio::test(flavor = "multi_thread", worker_threads = 6)]
+async fn a_forwarded_predicate_update_keeps_the_balance_equal_to_the_source_sum() {
+    forwarded_statement_then_move_source("UPDATE xs_entries SET amount = '40' WHERE amount = '25'")
+        .await;
+}

nodedb-cluster-tests/tests/common_suite/cases/mod.rs

@@ -116,6 +116,7 @@ mod listeners_gateway_smoke;
 mod listeners_typed_not_leader;
 mod materialized_sum_cross_core;
 mod materialized_sum_cross_shard;
+mod materialized_sum_gateway_occ;
 mod materialized_sum_replication;
 mod metadata_floor_restart;
 mod metadata_replay;

nodedb/src/control/fail_gate.rs

@@ -93,6 +93,22 @@ pub(crate) async fn after_calvin_stamp(
     }
 }
 
+/// The recon submit gate of node `node_id`, `recon::before_submit`: a
+/// dependent predicate write, a TRUNCATE or a CRDT document delete has
+/// settled its materialized sums and parks before it submits its Calvin
+/// transaction. A predicate write has also reconnoitred its rows. A test
+/// changes a row the settlement read, then releases the gate.
+pub(crate) async fn before_recon_submit(node_id: u64) {
+    wait(FailScope::Node(node_id), "recon::before_submit").await;
+}
+
+/// The statement routing gate of node `node_id`, `stmt::before_route`: a
+/// statement has planned and parks before the gateway forward and the Calvin
+/// classification.
+pub(crate) async fn before_route(node_id: u64) {
+    wait(FailScope::Node(node_id), "stmt::before_route").await;
+}
+
 /// The Calvin scheduler's redo gate on node `node_id`: after a committed
 /// slice's redo is resolved and before the data-group leader proposes it.
 /// Named per collection, `calvin::before_redo_propose::<collection>`.

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

@@ -31,6 +31,7 @@ use crate::control::planner::implicit_edges::{
     EdgeUpdateCtx, append_implicit_edge_delete_tasks, append_implicit_edge_update_tasks,
 };
 use crate::control::server::shared::session::StatementDeadline;
+use crate::control::server::shared::session::read_set::ReadSetEntry;
 use crate::control::state::SharedState;
 use crate::types::{DatabaseId, TenantId, TraceId};
 use nodedb_physical::physical_plan::OllpPredictedEdge;
@@ -151,12 +152,19 @@ pub fn is_edge_recon_plan(plan: &nodedb_physical::physical_plan::PhysicalPlan) -
 /// guard and edges all home on one vShard is a legitimate one-vShard
 /// transaction, and so is a contended single-collection predicate write
 /// routed here by the write-admission gate.
+///
+/// `reads` are the images materialized-sum settlement folded the tasks'
+/// cross-shard balance deltas from. Every attempt carries them unchanged in
+/// its `TxClass`, so a source row that moved after settlement aborts the
+/// transaction. A fresh reconnaissance cannot refresh them: the shipped
+/// deltas stay the ones computed from the settled images.
 pub async fn dispatch_authorized_dependent_edge_recon(
     state: &SharedState,
     authorized: crate::control::server::shared::authorization::AuthorizedTaskSet,
     identity: &crate::control::security::identity::AuthenticatedIdentity,
     tenant_id: TenantId,
     database_id: DatabaseId,
+    reads: &[ReadSetEntry],
 ) -> crate::Result<DependentReconOutcome> {
     let tasks = authorized
         .into_tasks()
@@ -171,6 +179,7 @@ pub async fn dispatch_authorized_dependent_edge_recon(
         tenant_id,
         database_id,
         user_roles,
+        reads,
     )
     .await
 }
@@ -185,7 +194,8 @@ pub(crate) async fn dispatch_dependent_edge_recon(
     database_id: DatabaseId,
     user_roles: Vec<String>,
 ) -> crate::Result<DependentReconOutcome> {
-    dispatch_dependent_edge_recon_inner(state, tasks, None, tenant_id, database_id, user_roles)
+    // The caller settled no materialized sum, so it has no images to carry.
+    dispatch_dependent_edge_recon_inner(state, tasks, None, tenant_id, database_id, user_roles, &[])
         .await
 }
 
@@ -196,11 +206,12 @@ async fn dispatch_dependent_edge_recon_inner(
     tenant_id: TenantId,
     database_id: DatabaseId,
     user_roles: Vec<String>,
+    reads: &[ReadSetEntry],
 ) -> crate::Result<DependentReconOutcome> {
     // A TRUNCATE empties the collection and tombstones its edges on every
     // vShard, in one transaction.
     if let Some(truncated) =
-        super::edge_truncate::dispatch_truncate(state, &tasks, identity, tenant_id).await?
+        super::edge_truncate::dispatch_truncate(state, &tasks, identity, tenant_id, reads).await?
     {
         return Ok(DependentReconOutcome {
             tasks_dispatched: tasks.len() as u64,
@@ -215,6 +226,7 @@ async fn dispatch_dependent_edge_recon_inner(
             identity,
             tenant_id,
             database_id,
+            reads,
         )
         .await?
     {
@@ -239,6 +251,20 @@ async fn dispatch_dependent_edge_recon_inner(
                 .to_owned(),
         })?;
 
+    // A request of several predicate writes runs as an implicit transaction,
+    // so each reads the rows the one before it wrote. Recon scans one write.
+    if tasks
+        .iter()
+        .filter(|t| is_dependent_predicate(&t.plan))
+        .count()
+        > 1
+    {
+        return Err(Error::Internal {
+            detail: "dependent-edge recon dispatch invoked with several predicate writes"
+                .to_owned(),
+        });
+    }
+
     let orc = orchestrator.ok_or(Error::SequencerUnavailable)?;
     // Hoisted across the retry loop so both `submit` and `rescan` can borrow them.
     let (dep_collection, dep_filter_bytes) = extract_bulk_predicate_info(&dep_task.plan);
@@ -376,6 +402,8 @@ async fn dispatch_dependent_edge_recon_inner(
                     .collect();
             }
 
+            #[cfg(feature = "failpoints")]
+            crate::control::fail_gate::before_recon_submit(state.node_id).await;
             orc.submit_with_retry_via(
                 pred_class,
                 tenant_id,
@@ -400,7 +428,7 @@ async fn dispatch_dependent_edge_recon_inner(
                         tenant_id,
                         dep_collection,
                         &surrogates,
-                        &[],
+                        reads,
                     )
                     .map_err(|e| OllpError::Terminal(Box::new(e)))?;
                     if let Some(class) = tx_class.as_mut() {