Skip to content

Commit bceb877

Browse files
committed
fix(control): track every vShard a transaction stages writes to
Sessions previously assumed a transaction stages writes to a single "home" vShard, recording only the first one. Multi-core transactions (e.g. two INSERTs to collections homed on different vShards) leaked the staging overlay on every vShard after the first at ROLLBACK, and savepoint mark/rewind only ever touched that one core. Replace the single `tx_vshard: Option<VShardId>` with a `tx_vshards: BTreeSet<VShardId>` recorded on every staged write. Overlay teardown and per-vShard savepoint markers now fan out over the full set: `MetaOp::DropTxnOverlay` targets every staged vShard, and each savepoint stores a per-vShard `(value_marker, graph_marker)` map so ROLLBACK TO rewinds all of them, defaulting to `(0, 0)` for a vShard first staged after the savepoint was taken. Also derive `PartialOrd`/`Ord` on `VShardId` so it can key a `BTreeMap`/`BTreeSet`.
1 parent c5c3fd0 commit bceb877

7 files changed

Lines changed: 310 additions & 88 deletions

File tree

nodedb-types/src/id/vshard.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ use serde::{Deserialize, Serialize};
1313
Copy,
1414
PartialEq,
1515
Eq,
16+
PartialOrd,
17+
Ord,
1618
Hash,
1719
Serialize,
1820
Deserialize,

nodedb/src/control/server/shared/session/lifecycle.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ pub async fn run_rollback(
5858
) {
5959
ddl_buffer::discard();
6060
// Snapshot the overlay identity BEFORE `rollback()` clears session state,
61-
// so the staging overlay can be released on its home vShard.
62-
let (overlay_txn_id, overlay_vshard) = sessions.txn_identity(addr);
61+
// so the staging overlay can be released on EVERY vShard the transaction
62+
// staged writes to (a transaction may span multiple cores).
63+
let (overlay_txn_id, overlay_vshards) = sessions.txn_identity(addr);
6364
let reservations = sessions.rollback(addr).unwrap_or_default();
6465
for handle in &reservations {
6566
let key = &handle.sequence_key;
@@ -87,8 +88,11 @@ pub async fn run_rollback(
8788
// Discard NOTIFY messages buffered during this transaction.
8889
sessions.discard_pending_notifies(addr);
8990

90-
// Release any staging overlay populated by statement-time point writes.
91-
if let (Some(txn_id), Some(vshard_id)) = (overlay_txn_id, overlay_vshard) {
92-
drop_txn_overlay(dp, identity.tenant_id, vshard_id, txn_id).await;
91+
// Release any staging overlay populated by statement-time point writes on
92+
// every vShard the transaction staged to, so no core's overlay leaks.
93+
if let Some(txn_id) = overlay_txn_id {
94+
for vshard_id in overlay_vshards {
95+
drop_txn_overlay(dp, identity.tenant_id, vshard_id, txn_id).await;
96+
}
9397
}
9498
}

nodedb/src/control/server/shared/session/savepoint_ops.rs

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@
88
//! neutral `SessionStore` savepoint stack. Transports translate the returned
99
//! [`SavepointError`] into their own SQLSTATE (`25P01` / `3B001`).
1010
11+
use std::collections::BTreeMap;
1112
use std::net::SocketAddr;
1213

1314
use crate::bridge::envelope::PhysicalPlan;
14-
use crate::types::TenantId;
15+
use crate::types::{TenantId, VShardId};
1516
use nodedb_physical::physical_plan::MetaOp;
1617
use nodedb_physical::physical_task::{PhysicalTask, PostSetOp};
1718

@@ -20,6 +21,7 @@ use super::state::TransactionState;
2021
use super::store::SessionStore;
2122

2223
/// Typed savepoint failure. Adapters map each variant to its SQLSTATE.
24+
#[derive(Debug)]
2325
pub enum SavepointError {
2426
/// A savepoint command was issued outside a transaction block. → `25P01`.
2527
NoActiveTransaction,
@@ -36,19 +38,14 @@ fn require_active_txn(sessions: &SessionStore, addr: &SocketAddr) -> Result<(),
3638
Ok(())
3739
}
3840

39-
/// Dispatch a savepoint overlay meta-op to the transaction's home vShard and
40-
/// return the raw response payload bytes, or `None` when no staged write has
41-
/// homed a vShard yet (the overlay — and its journal — is empty, so there is
42-
/// nothing on the Data Plane to mark or rewind).
41+
/// Dispatch a savepoint overlay meta-op to a specific vShard's core and return
42+
/// the raw response payload bytes, or `None` on dispatch failure.
4343
async fn dispatch_overlay_savepoint(
44-
sessions: &SessionStore,
45-
addr: &SocketAddr,
4644
tenant_id: TenantId,
45+
vshard_id: VShardId,
4746
dp: &impl TxnDataPlane,
4847
op: MetaOp,
4948
) -> Option<Vec<u8>> {
50-
let (txn_id, vshard) = sessions.txn_identity(addr);
51-
let (_txn_id, vshard_id) = (txn_id?, vshard?);
5249
let task = PhysicalTask {
5350
tenant_id,
5451
vshard_id,
@@ -88,10 +85,11 @@ fn decode_markers(payload: Option<Vec<u8>>) -> (usize, usize) {
8885

8986
/// Handle SAVEPOINT `<name>`.
9087
///
91-
/// Captures the composite overlay undo-journal marker on the txn's home vShard
92-
/// so a later ROLLBACK TO reverts staged value/TTL AND graph state to exactly
93-
/// here. A missing/short payload (or no vShard yet) means empty journals →
94-
/// `(0, 0)`.
88+
/// Captures the composite overlay undo-journal marker on EVERY vShard the
89+
/// transaction has staged writes to, so a later ROLLBACK TO reverts staged
90+
/// value/TTL AND graph state on all of them to exactly here. A missing/short
91+
/// payload means empty journals → `(0, 0)`. With no vShard staged yet the
92+
/// marker map is empty.
9593
pub async fn run_savepoint(
9694
sessions: &SessionStore,
9795
addr: &SocketAddr,
@@ -100,21 +98,21 @@ pub async fn run_savepoint(
10098
name: &str,
10199
) -> Result<(), SavepointError> {
102100
require_active_txn(sessions, addr)?;
103-
let (value_marker, graph_marker) = match sessions.tx_id(addr) {
104-
Some(txn_id) => {
101+
let (txn_id, vshards) = sessions.txn_identity(addr);
102+
let mut markers: BTreeMap<VShardId, (usize, usize)> = BTreeMap::new();
103+
if let Some(txn_id) = txn_id {
104+
for vshard_id in vshards {
105105
let payload = dispatch_overlay_savepoint(
106-
sessions,
107-
addr,
108106
tenant_id,
107+
vshard_id,
109108
dp,
110109
MetaOp::MarkSavepoint { txn_id },
111110
)
112111
.await;
113-
decode_markers(payload)
112+
markers.insert(vshard_id, decode_markers(payload));
114113
}
115-
None => (0, 0),
116-
};
117-
sessions.create_savepoint(addr, name.to_string(), value_marker, graph_marker);
114+
}
115+
sessions.create_savepoint(addr, name.to_string(), markers);
118116
Ok(())
119117
}
120118

@@ -139,7 +137,12 @@ pub fn run_release_savepoint(
139137
/// Handle ROLLBACK TO SAVEPOINT `<name>`.
140138
///
141139
/// Truncates the write buffer to the saved position and rewinds BOTH the
142-
/// value/TTL overlay and the graph overlay to the marked journal points.
140+
/// value/TTL overlay and the graph overlay on every vShard the transaction has
141+
/// staged to. Iterates the CURRENT staged set (a superset of the savepoint's,
142+
/// since writes may have staged to NEW vShards after the savepoint): a vShard
143+
/// with a saved marker rewinds to it; a vShard first staged AFTER the savepoint
144+
/// has no saved marker and rewinds to `(0, 0)`, dropping ALL of its staged
145+
/// writes.
143146
pub async fn run_rollback_to_savepoint(
144147
sessions: &SessionStore,
145148
addr: &SocketAddr,
@@ -148,25 +151,28 @@ pub async fn run_rollback_to_savepoint(
148151
name: &str,
149152
) -> Result<(), SavepointError> {
150153
require_active_txn(sessions, addr)?;
151-
let (value_marker, graph_marker) =
154+
let markers =
152155
sessions
153156
.rollback_to_savepoint(addr, name)
154157
.map_err(|e| SavepointError::NotFound {
155158
message: e.to_string(),
156159
})?;
157-
if let Some(txn_id) = sessions.tx_id(addr) {
158-
dispatch_overlay_savepoint(
159-
sessions,
160-
addr,
161-
tenant_id,
162-
dp,
163-
MetaOp::RollbackToSavepoint {
164-
txn_id,
165-
value_marker: value_marker as u64,
166-
graph_marker: graph_marker as u64,
167-
},
168-
)
169-
.await;
160+
let (txn_id, vshards) = sessions.txn_identity(addr);
161+
if let Some(txn_id) = txn_id {
162+
for vshard_id in vshards {
163+
let (value_marker, graph_marker) = markers.get(&vshard_id).copied().unwrap_or((0, 0));
164+
dispatch_overlay_savepoint(
165+
tenant_id,
166+
vshard_id,
167+
dp,
168+
MetaOp::RollbackToSavepoint {
169+
txn_id,
170+
value_marker: value_marker as u64,
171+
graph_marker: graph_marker as u64,
172+
},
173+
)
174+
.await;
175+
}
170176
}
171177
Ok(())
172178
}

nodedb/src/control/server/shared/session/state.rs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,28 @@
22

33
//! Per-connection session state types.
44
5-
use std::collections::HashMap;
5+
use std::collections::{BTreeMap, BTreeSet, HashMap};
66

77
use crate::types::{DatabaseId, Lsn, TenantId, TxnId, VShardId};
88
use nodedb_physical::physical_task::PhysicalTask;
99

10+
/// One entry on the transaction's savepoint stack.
11+
///
12+
/// A savepoint captures the write buffer length AND, for each vShard that had
13+
/// staged writes when the savepoint was established, that vShard's value/TTL and
14+
/// graph overlay undo-journal markers. On ROLLBACK TO, the buffer is truncated
15+
/// to `buffer_len` and every currently-staged vShard's overlays are rewound —
16+
/// to its saved marker if present, else to `(0, 0)` (a vShard first staged
17+
/// AFTER the savepoint must have ALL of its staged writes rewound).
18+
pub struct SavepointEntry {
19+
/// User-visible savepoint name.
20+
pub name: String,
21+
/// `tx_buffer` length captured when the savepoint was established.
22+
pub buffer_len: usize,
23+
/// Per-vShard `(value_marker, graph_marker)` overlay journal markers.
24+
pub markers: BTreeMap<VShardId, (usize, usize)>,
25+
}
26+
1027
/// PostgreSQL transaction state for ReadyForQuery status byte.
1128
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
1229
pub enum TransactionState {
@@ -73,23 +90,23 @@ pub struct ConnSession {
7390
/// and cleared on `COMMIT`/`ROLLBACK`. Keys the per-transaction staging
7491
/// overlay. `None` outside a transaction block.
7592
pub tx_id: Option<TxnId>,
76-
/// vShard this transaction's staged writes were homed on, captured on the
77-
/// first staged/buffered write. Under the single-vshard-per-transaction
78-
/// assumption, this is where the staging overlay lives — used to target
79-
/// `MetaOp::DropTxnOverlay` at COMMIT/ROLLBACK. `None` until the first write.
80-
pub tx_vshard: Option<VShardId>,
93+
/// Set of vShards this transaction has staged writes to, recorded on every
94+
/// staged/buffered write. A transaction can stage to multiple vShards/cores
95+
/// (e.g. two INSERTs to collections homed on different cores), so overlay
96+
/// teardown (`MetaOp::DropTxnOverlay` at ROLLBACK) and per-vShard savepoint
97+
/// mark/rewind must fan over ALL of them. Ordered (BTree) for deterministic
98+
/// teardown. Empty until the first staged write.
99+
pub tx_vshards: BTreeSet<VShardId>,
81100
/// Read-set: LSN-versioned, predicate-aware entries for write conflict
82101
/// detection, captured on the shared read seam by every transport. At
83102
/// COMMIT, each entry is checked — if the entry's collection has a current
84103
/// write-LSN past `read_lsn`, a concurrent write occurred and the
85104
/// transaction is rejected with SERIALIZATION_FAILURE.
86105
pub tx_read_set: Vec<super::read_set::ReadSetEntry>,
87-
/// Savepoint stack: each entry is `(name, tx_buffer_len_at_savepoint,
88-
/// value_overlay_marker, graph_overlay_marker)`.
89-
/// On ROLLBACK TO, truncate tx_buffer to the saved length AND rewind both
90-
/// Data-Plane staging overlays (value/TTL and GRAPH) to their saved
91-
/// journal markers.
92-
pub savepoints: Vec<(String, usize, usize, usize)>,
106+
/// Savepoint stack. On ROLLBACK TO, truncate tx_buffer to the saved length
107+
/// AND rewind each staged vShard's two Data-Plane staging overlays (value/TTL
108+
/// and GRAPH) to their saved journal markers. See [`SavepointEntry`].
109+
pub savepoints: Vec<SavepointEntry>,
93110
/// Pending consumer offset commits deferred until COMMIT.
94111
/// Each entry: (tenant_id, stream_name, group_name, partition_id, lsn).
95112
/// Flushed atomically on COMMIT, discarded on ROLLBACK.
@@ -159,7 +176,7 @@ impl ConnSession {
159176
tx_snapshot_lsn: None,
160177
tx_snapshot_epoch: None,
161178
tx_id: None,
162-
tx_vshard: None,
179+
tx_vshards: BTreeSet::new(),
163180
tx_read_set: Vec::new(),
164181
savepoints: Vec::new(),
165182
pending_offset_commits: Vec::new(),

nodedb/src/control/server/shared/session/store.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ impl SessionStore {
164164
session.tx_snapshot_lsn = None;
165165
session.tx_snapshot_epoch = None;
166166
session.tx_id = None;
167-
session.tx_vshard = None;
167+
session.tx_vshards.clear();
168168
session.tx_read_set.clear();
169169
session.savepoints.clear();
170170
session.pending_offset_commits.clear();

0 commit comments

Comments
 (0)