Skip to content

Commit 07b271f

Browse files
committed
feat(control): add write-admission gate and enforce it at the SPSC chokepoint
Introduce a neutral write-admission gate as the single seam every write-class PhysicalPlan passes through before being enqueued to a Data-Plane core, and a consolidated plan_is_write predicate derived from the exhaustive required_permission mapping so a new PhysicalPlan variant cannot silently miss the write/read split. Add a required Admission field to Request (Admitted, or Exempt with a reason of Read or AlreadyOrdered) so every construction site makes an explicit choice, and wire a chokepoint guard into both SPSC dispatch paths that flags any write-class plan reaching a core marked exempt-as-read. Lock acquisition against the per-vShard deterministic lock manager is a no-op for now; this change only funnels writes through one place and makes the invariant enforceable ahead of that follow-up.
1 parent 5e3063d commit 07b271f

60 files changed

Lines changed: 630 additions & 0 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

nodedb-test-support/src/cluster_harness/node/inspect/crdt.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ impl TestClusterNode {
5656
statement_digest: None,
5757
txn_id: None,
5858
wal_lsn: None,
59+
admission: nodedb::bridge::envelope::Admission::Exempt(
60+
nodedb::bridge::envelope::ExemptReason::Read,
61+
),
5962
};
6063

6164
// Register for response routing before dispatching, then submit through
@@ -126,6 +129,7 @@ impl TestClusterNode {
126129
statement_digest: None,
127130
txn_id: None,
128131
wal_lsn: None,
132+
admission: nodedb::bridge::envelope::Admission::Admitted,
129133
};
130134

131135
let mut rx = self.shared.tracker.register(request_id);

nodedb-test-support/src/cluster_harness/node/inspect/snapshot.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ impl TestClusterNode {
5656
statement_digest: None,
5757
txn_id: None,
5858
wal_lsn: None,
59+
admission: nodedb::bridge::envelope::Admission::Exempt(
60+
nodedb::bridge::envelope::ExemptReason::Read,
61+
),
5962
};
6063

6164
let mut rx = self.shared.tracker.register(request_id);
@@ -117,6 +120,9 @@ impl TestClusterNode {
117120
statement_digest: None,
118121
txn_id: None,
119122
wal_lsn: None,
123+
admission: nodedb::bridge::envelope::Admission::Exempt(
124+
nodedb::bridge::envelope::ExemptReason::Read,
125+
),
120126
};
121127

122128
let mut rx = self.shared.tracker.register(request_id);

nodedb-test-support/src/tx_batch_helpers.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ pub fn make_request(plan: PhysicalPlan) -> Request {
5757
statement_digest: None,
5858
txn_id: None,
5959
wal_lsn: None,
60+
admission: nodedb::bridge::envelope::Admission::Admitted,
6061
}
6162
}
6263

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! Write-admission chokepoint guard for the SPSC enqueue points.
4+
//!
5+
//! Both [`crate::bridge::dispatch::Dispatcher::dispatch`] and
6+
//! `dispatch_to_core` funnel every request into a Data-Plane core here: no
7+
//! write-class plan may reach a core marked exempt-as-read — that marker means
8+
//! the plan claimed to be a non-write and skipped the write-admission gate.
9+
//! Debug builds trip loudly; release builds increment a counter so a future
10+
//! write path that bypasses the write-admission gate is caught even when the
11+
//! assertion is compiled out — turning "a missed write path is a silent
12+
//! serializability hole" into a loud, observable regression.
13+
14+
use std::sync::atomic::{AtomicU64, Ordering};
15+
16+
use crate::bridge::envelope::Request;
17+
18+
/// Count of write-class Requests that reached an SPSC enqueue point marked
19+
/// [`Admission::Exempt`]`(`[`ExemptReason::Read`]`)` — i.e. hand-labeled a
20+
/// non-write and thus bypassed the write-admission gate.
21+
///
22+
/// [`Admission::Exempt`]: crate::bridge::envelope::Admission::Exempt
23+
/// [`ExemptReason::Read`]: crate::bridge::envelope::ExemptReason::Read
24+
///
25+
/// Stays at zero by construction: a base-state write is either
26+
/// [`Admission::Admitted`] by the gate or [`Admission::Exempt`] with
27+
/// [`ExemptReason::AlreadyOrdered`]. A non-zero value signals a regressed write
28+
/// path that was marked exempt-as-read.
29+
///
30+
/// [`Admission::Admitted`]: crate::bridge::envelope::Admission::Admitted
31+
/// [`ExemptReason::AlreadyOrdered`]: crate::bridge::envelope::ExemptReason::AlreadyOrdered
32+
static WRITES_BYPASSED_ADMISSION_GATE: AtomicU64 = AtomicU64::new(0);
33+
34+
/// Read the count of writes that reached an SPSC enqueue point marked
35+
/// exempt-as-read (bypassing the write-admission gate). Exposed for the metrics
36+
/// exporter and tests.
37+
pub fn writes_bypassed_admission_gate() -> u64 {
38+
WRITES_BYPASSED_ADMISSION_GATE.load(Ordering::Relaxed)
39+
}
40+
41+
/// Chokepoint guard shared by both SPSC enqueue points: catch a write-class
42+
/// plan that reached a Data-Plane core marked exempt-as-read — i.e. bypassed
43+
/// the write-admission gate while claiming to be a non-write.
44+
#[inline]
45+
pub(crate) fn assert_write_admitted(request: &Request) {
46+
let bypassed = crate::control::server::shared::write_admission::plan_is_write(&request.plan)
47+
&& request.admission.is_exempt_as_read();
48+
if bypassed {
49+
WRITES_BYPASSED_ADMISSION_GATE.fetch_add(1, Ordering::Relaxed);
50+
}
51+
debug_assert!(
52+
!bypassed,
53+
"a write-class plan reached the SPSC enqueue marked Exempt(Read) — it bypassed the write-admission gate"
54+
);
55+
}
56+
57+
#[cfg(test)]
58+
mod tests {
59+
use super::*;
60+
use crate::bridge::envelope::{Admission, ExemptReason, PhysicalPlan, Priority};
61+
use crate::control::server::shared::write_admission::plan_is_write;
62+
use crate::types::{DatabaseId, ReadConsistency, RequestId, TenantId, TraceId, VShardId};
63+
use nodedb_physical::physical_plan::{DocumentOp, KvOp};
64+
use std::time::{Duration, Instant};
65+
66+
/// A write-class plan: `plan_is_write` returns true for `KvOp::Put`.
67+
fn write_plan() -> PhysicalPlan {
68+
PhysicalPlan::Kv(KvOp::Put {
69+
collection: "c".into(),
70+
key: b"k".to_vec(),
71+
value: b"v".to_vec(),
72+
ttl_ms: 0,
73+
surrogate: nodedb_types::Surrogate::ZERO,
74+
})
75+
}
76+
77+
/// A read-class plan: `plan_is_write` returns false for `DocumentOp::PointGet`.
78+
fn read_plan() -> PhysicalPlan {
79+
PhysicalPlan::Document(DocumentOp::PointGet {
80+
collection: "c".into(),
81+
document_id: "d".into(),
82+
surrogate: nodedb_types::Surrogate::ZERO,
83+
pk_bytes: Vec::new(),
84+
rls_filters: Vec::new(),
85+
system_time: nodedb_types::SystemTimeScope::Current,
86+
valid_at_ms: None,
87+
})
88+
}
89+
90+
fn request_with(plan: PhysicalPlan, admission: Admission) -> Request {
91+
Request {
92+
request_id: RequestId::new(1),
93+
tenant_id: TenantId::new(1),
94+
database_id: DatabaseId::DEFAULT,
95+
vshard_id: VShardId::new(0),
96+
plan,
97+
deadline: Instant::now() + Duration::from_secs(5),
98+
priority: Priority::Normal,
99+
trace_id: TraceId::generate(),
100+
consistency: ReadConsistency::Strong,
101+
idempotency_key: None,
102+
event_source: crate::event::EventSource::User,
103+
user_roles: Vec::new(),
104+
user_id: None,
105+
statement_digest: None,
106+
txn_id: None,
107+
wal_lsn: None,
108+
admission,
109+
}
110+
}
111+
112+
/// The guard BITES: a write-class plan marked `Exempt(Read)` bypassed the
113+
/// write-admission gate, so the debug assertion must fire.
114+
#[test]
115+
#[should_panic(expected = "bypassed the write-admission gate")]
116+
fn write_marked_exempt_as_read_trips_guard() {
117+
let req = request_with(write_plan(), Admission::Exempt(ExemptReason::Read));
118+
assert_write_admitted(&req);
119+
}
120+
121+
/// A write that passed the gate (`Admitted`) does not trip the guard.
122+
#[test]
123+
fn admitted_write_does_not_trip() {
124+
let req = request_with(write_plan(), Admission::Admitted);
125+
assert_write_admitted(&req);
126+
}
127+
128+
/// A legitimate read marked `Exempt(Read)` does not trip the guard.
129+
#[test]
130+
fn read_marked_exempt_as_read_does_not_trip() {
131+
let req = request_with(read_plan(), Admission::Exempt(ExemptReason::Read));
132+
assert_write_admitted(&req);
133+
}
134+
135+
/// An already-ordered write (Calvin/Raft-follower/replay) is legitimately
136+
/// exempt and does not trip the guard.
137+
#[test]
138+
fn write_marked_already_ordered_does_not_trip() {
139+
let req = request_with(
140+
write_plan(),
141+
Admission::Exempt(ExemptReason::AlreadyOrdered),
142+
);
143+
assert_write_admitted(&req);
144+
}
145+
146+
/// Sanity: the write plan is classified as a write and the read plan is not.
147+
/// The bypass predicate the counter keys on is
148+
/// `plan_is_write(&plan) && admission.is_exempt_as_read()`; asserting the
149+
/// predicate directly (rather than calling `assert_write_admitted`, which
150+
/// panics in debug) verifies the counter-increment condition without the
151+
/// panic.
152+
#[test]
153+
fn plan_is_write_classification_and_bypass_predicate() {
154+
let w = write_plan();
155+
let r = read_plan();
156+
assert!(plan_is_write(&w));
157+
assert!(!plan_is_write(&r));
158+
159+
let exempt_read = Admission::Exempt(ExemptReason::Read);
160+
// The exact condition under which the counter increments and the guard trips.
161+
assert!(plan_is_write(&w) && exempt_read.is_exempt_as_read());
162+
// A read marked exempt-as-read is NOT a bypass.
163+
assert!(!(plan_is_write(&r) && exempt_read.is_exempt_as_read()));
164+
// A write marked Admitted is NOT a bypass.
165+
assert!(!(plan_is_write(&w) && Admission::Admitted.is_exempt_as_read()));
166+
// A write marked AlreadyOrdered is NOT a bypass.
167+
assert!(
168+
!(plan_is_write(&w)
169+
&& Admission::Exempt(ExemptReason::AlreadyOrdered).is_exempt_as_read())
170+
);
171+
}
172+
}

nodedb/src/bridge/dispatch.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ use crate::bridge::envelope;
1414
use crate::control::router::vshard::VShardRouter;
1515
use crate::data::eventfd::EventFdNotifier;
1616

17+
use crate::bridge::admission_chokepoint::assert_write_admitted;
18+
1719
/// Serialized form of a request that goes through the SPSC ring buffer.
1820
///
1921
/// The bridge crate is generic over `T` — we serialize our typed `Request`
@@ -273,6 +275,7 @@ impl Dispatcher {
273275
/// then flushes WFQ → physical ring. Returns `Err` when the WFQ itself is
274276
/// full (total capacity reached across all active databases on that core).
275277
pub fn dispatch(&mut self, request: envelope::Request) -> crate::Result<()> {
278+
assert_write_admitted(&request);
276279
let tenant_id = request.tenant_id.as_u64();
277280
let req_id = request.request_id.as_u64();
278281
let database_id = request.database_id.as_u64();
@@ -373,6 +376,7 @@ impl Dispatcher {
373376
core_id: usize,
374377
request: envelope::Request,
375378
) -> crate::Result<()> {
379+
assert_write_admitted(&request);
376380
if core_id >= self.cores.len() {
377381
return Err(crate::Error::Dispatch {
378382
detail: format!("core {core_id} out of range (have {})", self.cores.len()),
@@ -510,6 +514,7 @@ mod tests {
510514
statement_digest: None,
511515
txn_id: None,
512516
wal_lsn: None,
517+
admission: Admission::Exempt(ExemptReason::Read),
513518
}
514519
}
515520

@@ -539,6 +544,7 @@ mod tests {
539544
statement_digest: None,
540545
txn_id: None,
541546
wal_lsn: None,
547+
admission: Admission::Exempt(ExemptReason::Read),
542548
}
543549
}
544550

nodedb/src/bridge/envelope.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,73 @@ pub struct Request {
151151
///
152152
/// [`ExecutionTask`]: crate::data::executor::task::ExecutionTask
153153
pub wal_lsn: Option<Lsn>,
154+
155+
/// Write-admission decision for this request.
156+
///
157+
/// Every write-class [`PhysicalPlan`] MUST pass the neutral write-admission
158+
/// gate (`crate::control::server::shared::write_admission`) before it is
159+
/// enqueued to a Data-Plane core; the gate stamps [`Admission::Admitted`].
160+
/// Requests that do not re-enter the gate carry [`Admission::Exempt`] with
161+
/// an [`ExemptReason`] — [`ExemptReason::Read`] for reads / savepoint /
162+
/// overlay meta ops, [`ExemptReason::AlreadyOrdered`] for writes already
163+
/// serialized elsewhere (Calvin-scheduled applies, Raft-follower / replay /
164+
/// clone / checkpoint).
165+
///
166+
/// The field is REQUIRED (no `Default`, no `#[serde(default)]`) so every
167+
/// `Request` construction site makes an explicit choice — that is the
168+
/// write-ingress completeness enforcement. The SPSC enqueue chokepoint
169+
/// (`crate::bridge::dispatch`) asserts no write-class plan reaches a core
170+
/// with the decision unmade.
171+
pub admission: Admission,
172+
}
173+
174+
/// Write-admission marker carried by every [`Request`].
175+
///
176+
/// A write-class plan becomes [`Admission::Admitted`] only by passing the
177+
/// neutral write-admission gate. Everything that does not re-enter the gate's
178+
/// OCC fence carries [`Admission::Exempt`] with an explicit [`ExemptReason`].
179+
/// There is intentionally no "unresolved" variant: the required field makes an
180+
/// unmade decision unrepresentable, so a missed write path is a compile error
181+
/// at the construction site rather than a silent serializability hole at
182+
/// runtime.
183+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
184+
pub enum Admission {
185+
/// Passed the write-admission gate.
186+
Admitted,
187+
/// Does not re-enter the write-admission gate — either a non-write, or a
188+
/// write whose ordering was already decided elsewhere. The [`ExemptReason`]
189+
/// records which, so the SPSC chokepoint can tell a legitimately exempt
190+
/// write apart from a base-state write that bypassed the gate.
191+
Exempt(ExemptReason),
192+
}
193+
194+
/// Why a [`Request`] is exempt from the write-admission gate.
195+
///
196+
/// The distinction is load-bearing at the SPSC chokepoint: a write-class plan
197+
/// marked [`ExemptReason::Read`] is a bug (a write that bypassed the gate),
198+
/// whereas [`ExemptReason::AlreadyOrdered`] is a legitimately exempt write.
199+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
200+
pub enum ExemptReason {
201+
/// The plan is not a base-state write — a read / query, or an overlay /
202+
/// savepoint meta-op. It never needs the write fence.
203+
Read,
204+
/// A write-class plan whose ordering was ALREADY decided elsewhere and does
205+
/// NOT re-enter the OCC fence: Calvin-scheduler applies (the scheduler
206+
/// already holds the locks), replicated / Raft-follower applies, recovery /
207+
/// replay, clone / copy-up materialization, and checkpoint. These are
208+
/// legitimately exempt writes.
209+
AlreadyOrdered,
210+
}
211+
212+
impl Admission {
213+
/// Whether this marker is [`Admission::Exempt`] with reason
214+
/// [`ExemptReason::Read`] — i.e. claims the plan is not a base-state write.
215+
///
216+
/// The SPSC chokepoint uses this to catch a write-class plan wrongly marked
217+
/// exempt-as-read: such a plan bypassed the write-admission gate.
218+
pub fn is_exempt_as_read(&self) -> bool {
219+
matches!(self, Admission::Exempt(ExemptReason::Read))
220+
}
154221
}
155222

156223
/// Response envelope: Data Plane -> Control Plane.
@@ -391,6 +458,7 @@ mod tests {
391458
statement_digest: None,
392459
txn_id: None,
393460
wal_lsn: None,
461+
admission: Admission::Exempt(ExemptReason::Read),
394462
}
395463
}
396464

@@ -460,6 +528,7 @@ mod tests {
460528
statement_digest: None,
461529
txn_id: None,
462530
wal_lsn: None,
531+
admission: Admission::Exempt(ExemptReason::Read),
463532
};
464533
match req.plan {
465534
PhysicalPlan::Meta(MetaOp::Cancel { target_request_id }) => {

nodedb/src/bridge/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3+
pub mod admission_chokepoint;
34
pub mod dispatch;
45
pub mod envelope;
56
pub mod quiesce;
@@ -19,5 +20,6 @@ pub mod window_func {
1920
pub use nodedb_query::window::*;
2021
}
2122

23+
pub use admission_chokepoint::writes_bypassed_admission_gate;
2224
pub use dispatch::Dispatcher;
2325
pub use envelope::{Request, Response, Status};

nodedb/src/control/array_sync/raft_apply.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,9 @@ fn build_array_request(
425425
statement_digest: None,
426426
txn_id: None,
427427
wal_lsn: None,
428+
admission: crate::bridge::envelope::Admission::Exempt(
429+
crate::bridge::envelope::ExemptReason::AlreadyOrdered,
430+
),
428431
}
429432
}
430433

nodedb/src/control/catalog_entry/post_apply/async_dispatch/continuous_aggregate.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ async fn dispatch_meta(
103103
statement_digest: None,
104104
txn_id: None,
105105
wal_lsn: None,
106+
admission: crate::bridge::envelope::Admission::Exempt(
107+
crate::bridge::envelope::ExemptReason::AlreadyOrdered,
108+
),
106109
};
107110
let rx = shared.tracker.register(request_id);
108111
if d.dispatch_to_core(core_id, request).is_err() {

nodedb/src/control/catalog_entry/post_apply/async_dispatch/materialized_view.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ pub async fn delete_async(tenant_id: u64, name: String, shared: Arc<SharedState>
5151
statement_digest: None,
5252
txn_id: None,
5353
wal_lsn: None,
54+
admission: crate::bridge::envelope::Admission::Exempt(
55+
crate::bridge::envelope::ExemptReason::AlreadyOrdered,
56+
),
5457
};
5558
let rx = shared.tracker.register(request_id);
5659
if d.dispatch_to_core(core_id, request).is_err() {

0 commit comments

Comments
 (0)