Skip to content

Commit e642ba9

Browse files
committed
Merge remote-tracking branch 'origin/main' into fix/issues-188-193
# Conflicts: # nodedb/src/control/server/native/dispatch/transaction.rs
2 parents d79eccf + d75bfd4 commit e642ba9

6 files changed

Lines changed: 327 additions & 60 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ NodeDB 0.4.0 is a substantial distributed-correctness and durability release. It
5252
- **WAL replay correctness** — timeseries samples are no longer rejected during replay and a mid-record flush no longer duplicates rows on recovery; per-row surrogates are persisted and restored through columnar WAL replay; complete KV WAL replay (`incr`/`expire`/`persist`/`cas`/`field_set`/`register_index`/`drop_index`) with resolved expiry instants; graph node-label and columnar predicate UPDATE/DELETE records persisted and replayed.
5353
- **Query and SQL correctness** — scan and join execution no longer silently truncates rows at implicit caps; streamed scans drain every chunk; bitemporal range scans, indexed residual predicates, computed and distributed `GROUP BY`, and scalar aggregates over empty input return complete results; strict `AS OF SYSTEM TIME NULL` queries resolve the correct historical schema.
5454
- **Restore / backup** — WAL tombstones replicated via Raft on restore; plain-columnar rows re-issued durably rather than snapshot-installed; columnar/flushed-timeseries data and catalog propagated cluster-wide; replica multiplication and CRDT loss under RF>1 prevented.
55+
- **Native protocol transactions** — a row committed inside an explicit `Begin`/`Commit` over the native protocol is now visible to PK point lookups and filtered aggregates, not just full scans; the commit batch was routed to vShard 0 instead of the collection's owning vShard, and the gateway router now rejects unroutable commit meta-ops instead of silently misrouting them (#193).
5556
- **Session plan cache** — a repeated literal PK point lookup no longer replays a stale empty result after the same session inserted that key. Document point reads and PK-targeted document mutations are excluded from the schema-only plan cache, so a byte-identical `SELECT` reflects the session's own committed writes and the simple and extended protocols agree.
5657
- **Object ownership**`DROP USER` reassigns every object the user owned to a validated administrative principal — the tenant's recorded admin, else an active `tenant_admin`, else an active superuser in that tenant — and is refused when no such principal exists, instead of repointing objects at a synthetic name that was never created and leaving the data directory unbootable. Collections inside their drop-retention window are included. Ownership records are keyed by `(object_type, database_id, tenant_id, object_name)`, so ownership of a collection no longer extends to a same-named collection in another database. The startup catalog check now repairs dangling owner references and revokes grants to removed users instead of refusing to start, so a data directory already affected by this recovers on its next boot.
5758
- **Tenant management** — tenant IDs allocated via a durable high-water-mark; ghost rows in `SHOW TENANTS` after `DROP TENANT` eliminated; an existence gate enforced for unknown numeric tenant IDs; `DROP`/`ALTER`/`PURGE TENANT` accept a tenant name; duplicate `CREATE TENANT` names rejected with `42710` rather than allocating a second tenant id under the same name.

nodedb/src/control/cluster/metadata_applier/audit.rs

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -109,22 +109,50 @@ pub(super) fn emit_ddl_audit(
109109
// `descriptor_kind` + `descriptor_name`.
110110
let _ = std::any::type_name::<StoredCollection>();
111111

112-
let mut log = match shared.audit.lock() {
113-
Ok(g) => g,
114-
Err(p) => p.into_inner(),
112+
// Hand the record off rather than taking the audit mutex here.
113+
//
114+
// This runs on the Raft metadata apply loop — the single thread that
115+
// applies committed entries for group 0. Blocking it on a process-wide
116+
// mutex lets audit-log contention stall EVERY metadata apply, and with it
117+
// collection materialization, DDL, and any proposer waiting on the applied
118+
// index. That is a liveness bug rather than a slow path: one contended
119+
// acquisition here was measured parking the loop for a full 5s propose
120+
// timeout, so peer `CollectionSchema` announces never materialized and the
121+
// engine writes that followed them were rejected as unknown collections.
122+
//
123+
// The record's payload is fully built above and needs nothing further from
124+
// the apply loop, so deferring the emit keeps the compliance row (no silent
125+
// drop) while letting the loop advance. Hash-chain integrity is unaffected:
126+
// `record_with_auth` allocates the sequence number under the same mutex, so
127+
// chain order follows lock-acquisition order exactly as it does for every
128+
// other audit writer.
129+
let audit = std::sync::Arc::clone(&shared.audit);
130+
let emit = move || {
131+
let mut log = match audit.lock() {
132+
Ok(g) => g,
133+
Err(p) => p.into_inner(),
134+
};
135+
log.record_with_auth(
136+
AuditEvent::DdlChange,
137+
None,
138+
None,
139+
"metadata_group",
140+
&detail_json,
141+
&AuditAuth {
142+
user_id,
143+
user_name,
144+
session_id: String::new(),
145+
},
146+
);
115147
};
116-
log.record_with_auth(
117-
AuditEvent::DdlChange,
118-
None,
119-
None,
120-
"metadata_group",
121-
&detail_json,
122-
&AuditAuth {
123-
user_id,
124-
user_name,
125-
session_id: String::new(),
126-
},
127-
);
148+
match tokio::runtime::Handle::try_current() {
149+
Ok(handle) => {
150+
handle.spawn_blocking(emit);
151+
}
152+
// No reactor (unit tests, non-Tokio callers): emit inline. There is no
153+
// apply loop to protect in that context.
154+
Err(_) => emit(),
155+
}
128156
}
129157

130158
/// Return `(descriptor_name, version_after, hlc_string)` for a

nodedb/src/control/gateway/router.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,26 @@ pub fn route_plan(
6363
strategy_fn: impl Fn(&str) -> PartitionStrategy,
6464
extractor: &dyn KeyExtractor,
6565
) -> Result<Vec<TaskRoute>> {
66+
// Commit-time meta-ops (ResolveTxn / TransactionBatch) carry no collection
67+
// name, so their vShard cannot be derived here — the primary_vshard
68+
// fallback would silently send them to vShard 0 and durably apply the
69+
// commit batch on the wrong core (#193). They are dispatched with the
70+
// task's pre-classified `vshard_id` (see `dispatch_single_shard`), never
71+
// through the gateway.
72+
{
73+
use nodedb_physical::physical_plan::MetaOp;
74+
if matches!(
75+
&plan,
76+
PhysicalPlan::Meta(MetaOp::ResolveTxn { .. } | MetaOp::TransactionBatch { .. })
77+
) {
78+
return Err(crate::Error::Internal {
79+
detail: "commit meta-op cannot be routed by the gateway; \
80+
dispatch it with the task's explicit vshard_id"
81+
.to_owned(),
82+
});
83+
}
84+
}
85+
6686
// In single-node mode every plan runs locally.
6787
let Some(routing) = routing else {
6888
let vshard_id = primary_vshard(&plan, database_id);
@@ -440,4 +460,39 @@ mod tests {
440460
}
441461
unreachable!()
442462
}
463+
464+
/// Commit-time meta-ops carry no collection name, so the router cannot
465+
/// derive their vShard — silently falling back to vShard 0 durably applies
466+
/// the commit batch on the wrong core (#193). They must be rejected here;
467+
/// callers dispatch them with the task's pre-classified `vshard_id`.
468+
#[test]
469+
fn commit_meta_ops_are_rejected() {
470+
use nodedb_physical::physical_plan::MetaOp;
471+
472+
for plan in [
473+
PhysicalPlan::Meta(MetaOp::TransactionBatch {
474+
plans: vec![],
475+
txn_id: None,
476+
}),
477+
PhysicalPlan::Meta(MetaOp::ResolveTxn {
478+
txn_id: nodedb_types::id::TxnId::new(7),
479+
plans: vec![],
480+
}),
481+
] {
482+
for table in [None, Some(single_node_table())] {
483+
let result = route_plan(
484+
plan.clone(),
485+
1,
486+
table.as_ref(),
487+
DatabaseId::DEFAULT,
488+
|_| PartitionStrategy::CollectionHomed,
489+
&crate::control::gateway::UnwiredKeyExtractor,
490+
);
491+
assert!(
492+
result.is_err(),
493+
"commit meta-op must not be routable via the gateway: {plan:?}"
494+
);
495+
}
496+
}
497+
}
443498
}

nodedb/src/control/server/native/dispatch/transaction.rs

Lines changed: 21 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@ use nodedb_types::TraceId;
1616
use nodedb_types::protocol::NativeResponse;
1717

1818
use crate::bridge::envelope::{ErrorCode, Response};
19-
use crate::control::gateway::RouteDecision;
20-
use crate::control::server::dispatch_utils::{
21-
ChangeFeedOwner, SubmitWrite, WalDurability, WriteOrdering, submit_write,
22-
};
2319
use crate::control::server::shared::ddl::sqlstate::error_code_to_sqlstate;
2420
use crate::control::server::shared::session::{
2521
AbortReason, CommitOutcome, TxnDataPlane, commit, lifecycle,
@@ -28,15 +24,17 @@ use crate::control::state::SharedState;
2824
use crate::types::Lsn;
2925
use nodedb_physical::physical_task::PhysicalTask;
3026

27+
use super::super::super::dispatch_utils;
3128
use super::DispatchCtx;
3229

3330
/// Native Data-Plane dispatch seam for the neutral transaction orchestrator.
3431
///
35-
/// Routes a task through the cluster gateway when one is configured, otherwise
36-
/// through the direct SPSC dispatch path — the exact branch native COMMIT used
37-
/// before extraction. The gateway path synthesizes an `Ok` [`Response`] on
38-
/// success (carrying the first vShard payload so overlay-marker meta-ops still
39-
/// decode), and surfaces gateway errors as a Rust `Err`.
32+
/// Always dispatches through the direct SPSC write path using the task's
33+
/// pre-classified `vshard_id`, mirroring pgwire's `dispatch_task_no_wal`.
34+
/// The gateway must NOT be used here: commit-time tasks carry `MetaOp` plans
35+
/// (`ResolveTxn`, `TransactionBatch`) with no named collection, so the
36+
/// gateway's router cannot derive a route for them and falls back to
37+
/// vShard 0 — durably applying the commit batch on the wrong core (#193).
4038
pub(crate) struct NativeTxnDp<'a> {
4139
pub(crate) state: &'a SharedState,
4240
}
@@ -49,42 +47,21 @@ impl TxnDataPlane for NativeTxnDp<'_> {
4947
) -> Pin<Box<dyn Future<Output = crate::Result<Response>> + Send + 'a>> {
5048
let state = self.state;
5149
Box::pin(async move {
52-
let decision = crate::control::server::shared::session::resolve_leader(&task, state);
53-
if matches!(decision, RouteDecision::Local) {
54-
return submit_write(
55-
state,
56-
SubmitWrite {
57-
tenant_id: task.tenant_id,
58-
database_id: task.database_id,
59-
vshard_id: task.vshard_id,
60-
plan: task.plan,
61-
trace_id: TraceId::generate(),
62-
event_source: crate::event::EventSource::User,
63-
txn_id: task.txn_id,
64-
user_id: None,
65-
durability: WalDurability::CallerSupplied {
66-
wal_lsn,
67-
resolved_now_ms: None,
68-
},
69-
ordering: WriteOrdering::Gate,
70-
change_feed: ChangeFeedOwner::Funnel,
71-
},
72-
)
73-
.await
74-
.map(|outcome| outcome.response);
75-
}
76-
77-
if wal_lsn.is_some() {
78-
return Err(crate::Error::Internal {
79-
detail: "non-local transaction commit must route through Calvin".into(),
80-
});
81-
}
82-
let version_plan = task.plan.clone();
83-
crate::control::server::shared::session::forward_to_leader(
50+
dispatch_utils::dispatch_write_to_data_plane(
8451
state,
85-
decision,
86-
task,
87-
&version_plan,
52+
dispatch_utils::WriteDispatch {
53+
tenant_id: task.tenant_id,
54+
database_id: task.database_id,
55+
vshard_id: task.vshard_id,
56+
plan: task.plan,
57+
trace_id: TraceId::ZERO,
58+
event_source: crate::event::EventSource::User,
59+
txn_id: None,
60+
wal_lsn,
61+
// Batch COMMIT record, not per-task WAL append — see
62+
// `dispatch_task_no_wal`'s equivalent limitation.
63+
resolved_now_ms: None,
64+
},
8865
)
8966
.await
9067
})

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ pub mod plan_cache;
3535
pub mod prepared_cache;
3636

3737
pub use self::cross_shard_mode::{CrossShardTxnMode, parse_value as parse_cross_shard_value};
38-
pub(crate) use self::leader_forward::{forward_to_leader, resolve_leader};
3938
pub use self::outcome::{AbortReason, CommitOutcome, TxnDataPlane};
4039
pub use self::params::{
4140
is_known_pg_runtime_parameter, is_known_settable_runtime_parameter, parse_set_command,

0 commit comments

Comments
 (0)