Skip to content

feat(control): native SQL in-txn read-your-own-writes (U3) + cross-cluster txn_id groundwork#176

Open
pizofreude wants to merge 3 commits into
NodeDB-Lab:mainfrom
pizofreude:verify/native-sql-ryow
Open

feat(control): native SQL in-txn read-your-own-writes (U3) + cross-cluster txn_id groundwork#176
pizofreude wants to merge 3 commits into
NodeDB-Lab:mainfrom
pizofreude:verify/native-sql-ryow

Conversation

@pizofreude

Copy link
Copy Markdown
Member

What & why

Verifies and closes design unit U3 (in-transaction read-your-own-writes) on the SQL paths, and lands the cluster-wire groundwork needed for the cross-node continuation (U4/U5).

Three focused commits:

  1. fix(control): ensure native session on first-frame BEGIN — a BEGIN arriving on the very first native frame no longer races session creation, so the transaction has a session to attach to.
  2. fix(control): propagate txn_id through the gateway for native in-txn RYOW — threads the session txn_id through the native gateway dispatch so an in-block read resolves this transaction's staging overlay. Single-node native in-txn RYOW now works.
  3. feat(control): thread txn_id across cluster wire; defer cross-node RYOW to U4/U5 — adds ExecuteRequest.txn_id (WIRE_VERSION 2→3, coordinated upgrade), threads the id through dispatcher/stream/receiver/all-cores, and mints globally-unique ids via TxnId::from_origin(node_id, seq).

Scope decision: U3 closed as single-node; cross-node deferred to U4/U5

Per the LOCKED design §8, U3's validation surface is single-node only ("single-node-verifiable: U1, U2, U3 (RYOW), U7"; cross-node is "cluster-bound (stage-2): U4, U5, U6, U8"). A 3-node probe (cross_node_ryow_probe.rs) is included but #[ignore]d as the executable spec for U4/U5.

Confirmed root cause of the cross-node gap (live trace): the per-transaction machinery — staging gate, COMMIT buffer, COMMIT/ROLLBACK overlay flush — is local-only. On a non-owner node the pgwire dispatcher takes the remote-leader forward short-circuit (routing/execute.rs should_forward_via_gatewaydispatch_tasks_via_gateway) and returns before dispatch_task_loop, so route_in_tx_write never runs. The in-txn write ships to the owner as a plain replicable write (Raft-proposed = committed at statement time, never staged), and gateway_dispatch.rs hardcodes QueryContext.txn_id = None, so the read arrives with no txn id. The wire txn_id field here is a necessary-but-not-sufficient prerequisite for the U4/U5 fix.

How to test

# Single-node native in-txn RYOW — passes
cargo nextest run -p nodedb-cluster-tests -E 'test(native_sql_ryow)'

# Cross-node probe — ignored by default; run explicitly to reproduce the U4/U5 gap
cargo nextest run -p nodedb-cluster-tests -E 'test(cross_node_in_txn_select_reads_own_write)' --run-ignored all

Also verified locally: cargo fmt --all clean; cargo check -p nodedb clean (zero warnings); cargo clippy introduces no new warnings in any file this branch touches.

Tradeoffs / alternatives considered

  • Coordinated (not rolling) wire upgrade for WIRE_VERSION 2→3 — the field is additive but mixed-version clusters are out of scope for this branch.
  • Defer cross-node RYOW rather than build the cross-shard-transaction machinery (gateway-routed staging / COMMIT / ROLLBACK / overlay-drop) ahead of the U4/U5 schedule and out of order.
  • TxnId::from_origin(node_id, seq) kept as groundwork so ids are globally unique before cross-node txn coordination exists (all non-txn call sites pass None/identity).

Notes for review

  • Touches wire dispatch on pgwire/native/RESP paths and bumps WIRE_VERSION — flagging per the "wire protocol changes" note in CONTRIBUTING.md.
  • CI is opt-in via the run-ci label. Heads-up: the workspace currently has pre-existing clippy lints under current stable (iter_kv_map, question_mark, unneeded_wildcard_pattern) in files unrelated to this branch; with -D warnings they would trip the lint job independently of this change.

A native (MessagePack) client that sends BEGIN as its first post-auth frame previously no-oped the transaction: handle_begin -> run_begin -> SessionStore::begin silently returned Ok without setting InBlock when no session yet existed for the peer address (ensure_session was only called lazily from the SQL statement path). The first in-block write then autocommitted instead of buffering into the staging overlay. Call sessions.ensure_session(peer_addr) at the top of handle_begin so BEGIN transitions the session to InBlock regardless of frame ordering, matching pgwire which creates the session at connection startup. Guarded by native_first_frame_begin_does_not_buffer_probe.
…RYOW

Native SQL reads inside an explicit transaction route through sql_gateway.rs::dispatch_task_via_gateway, which built a GatewayQueryContext without the connection's txn_id before calling gateway.execute(). That dropped txn_id ahead of the Data Plane, so the per-transaction staging overlay was never resolved and a transaction could not read its own writes. The pgwire path was unaffected because its local in-block dispatch stamps task.txn_id straight onto the Request envelope.

Add QueryContext.txn_id and thread it end to end: gateway::execute -> dispatch_route -> dispatch_local -> dispatch_to_data_plane_with_txn. The dispatch_route arguments are bundled into a new DispatchRouteParams struct (mirroring DispatchRouteStreamParams) to stay under the too_many_arguments threshold. The real txn_id is carried at the in-txn read seams (sql_gateway, direct_ops, gather, native streaming); None is used at non-interactive entry points (HTTP/RESP/ILP/CDC/topic/ws_rpc/promql/calvin), the four remote graph-dispatch callers, and the two pgwire gateway sites (remote-forward is cross-node, streaming is autocommit-only).

Cross-node in-transaction RYOW remains a tracked gap: the RPC envelope does not yet carry txn_id. Guarded by native_in_txn_sql_select_reads_own_write_probe.
…OW to U4/U5

Add ExecuteRequest.txn_id (WIRE_VERSION 2->3, coordinated upgrade) and
thread the session txn id through dispatcher/stream/receiver/all-cores.
Mint globally-unique ids via TxnId::from_origin(node_id, seq).

Add cross_node_ryow_probe.rs, #[ignore]d: cross-node in-txn RYOW is
confirmed to require the cross-shard-transaction machinery (U4/U5); U3 is
single-node only per LOCKED design section 8. Root cause: the pgwire
remote-leader forward path returns before the staging gate and hardcodes
QueryContext.txn_id = None, so a non-owner's in-txn write ships as a plain
committed write and its read carries no txn id.

Remove RYOW debug instrumentation.
@farhan-syah

farhan-syah commented Jul 10, 2026

Copy link
Copy Markdown
Member

Single-node native RYOW looks right. Two asks before merge:

  1. Drop the internal roadmap labels from source. The new test files/comments carry U1/U4/U5, "design §8", "GAP B", etc. — describe the behavior instead (e.g. cross_node_ryow_probe.rs → name/ignore-reason about "non-owner forwards write without threading the session overlay"). The technical notes are useful; just the unit labels need to go.

  2. The WIRE_VERSION 2→3 bump has no consumer yet. txn_id is stamped None everywhere and cross-node RYOW is deferred, so this forces a coordinated cluster upgrade for an inert field. Suggest folding the wire break into the change that actually reads it — one upgrade at feature time instead of one now for groundwork.

Commits 1–2 are fine to land independently if you want to split the wire groundwork out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants