Skip to content

Commit 4828165

Browse files
committed
fix(cluster): stop blocking metadata apply loop on audit log mutex
emit_ddl_audit ran on the Raft metadata apply loop and took the process-wide audit mutex inline, so audit-log contention could stall every metadata apply and, with it, collection materialization, DDL, and any proposer waiting on the applied index. Defer the emit onto a blocking task when a Tokio runtime is available, falling back to an inline emit for non-Tokio callers (unit tests). The sequence number is still allocated under the same mutex inside record_with_auth, so the hash chain's ordering guarantee is unaffected.
1 parent 637ea2a commit 4828165

2 files changed

Lines changed: 47 additions & 15 deletions

File tree

Cargo.lock

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

0 commit comments

Comments
 (0)