From b9cf720d14b176479900a161b5bcb531d9ab1527 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 10:12:13 +0700 Subject: [PATCH 1/7] feat(mq): versioned WAL v3 effect-record types + codec for MQ durability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three new WalRecordType discriminants (MqPush 0x72, MqPop 0x73, MqTrigger 0x74) and rewrites src/mq/wal.rs as a versioned envelope codec for all five MQ mutation kinds (MqCreate, MqPush, MqPop, MqAck, MqTrigger). MqCreate (0x70) and MqAck (0x71) keep their existing discriminants but move to a new payload layout: every payload now starts with a version byte and carries the affected db index explicitly, fixing a pre-existing db-0 hardcode bug. Precedent for changing an on-disk payload layout without bumping the discriminant: WalRecordType::XactCommit's own doc comment notes the 0x51->0x53 format freeze — WAL v3 segments are short-lived and rotated with no cross-version compatibility contract. Every decoder returns None on a malformed payload OR an unsupported (future) version byte, never panics — callers are expected to skip-and-warn rather than abort a replay scan on one bad record. MqPop's payload carries the full claim set (id + delivery_count per claimed message) plus any DLQ routing decisions (source id -> assigned DLQ id), so a later replay can reconstruct the consumer group's PEL and last_delivered_id exactly instead of approximating via a count-based heuristic. ~35 unit tests cover roundtrips, empty/malformed/truncated payloads, and unknown-version rejection for all five record kinds. This is durability-plane scaffolding only; no emission or replay call sites are wired up yet (next commits). author: Tin Dang --- src/mq/wal.rs | 665 +++++++++++++++++++++++++------ src/persistence/wal_v3/record.rs | 30 +- src/persistence/wal_v3/replay.rs | 5 +- 3 files changed, 583 insertions(+), 117 deletions(-) diff --git a/src/mq/wal.rs b/src/mq/wal.rs index 33043fdf..871660fa 100644 --- a/src/mq/wal.rs +++ b/src/mq/wal.rs @@ -1,17 +1,55 @@ -//! WAL encode/decode for MQ lifecycle operations. +//! WAL encode/decode for MQ lifecycle + effect operations. //! -//! Provides serialization for `MqCreate` and `MqAck` WAL records. -//! Layout follows the same defensive decode pattern as -//! `encode_workspace_create` / `decode_workspace_create` in -//! `workspace/wal.rs`. +//! Wave B stage 2a (task #34) gives MQ its own durability sub-plane: every +//! owner-shard mutation (`MQ.CREATE`/`PUSH`/`POP`/`ACK`/`TRIGGER`) emits a +//! versioned effect record through the typed `wal_append` channel. Replay +//! (`crate::shard::shared_databases::replay_mq_wal`) applies these records +//! IN ORDER after the AOF-authority wipe (`main.rs`'s `db.clear()`), which is +//! what lets a durable queue's content, consumer-group PEL, DLQ routing, and +//! trigger registrations all survive a kill-9 even when `--appendonly yes` +//! would otherwise discard the whole keyspace. //! -//! All decode functions return `Option` -- NEVER panic or unwrap. +//! All payloads share a leading `version: u8` byte. Current version is `1` +//! for every record kind below. Decoders return `None` for ANY structurally +//! invalid payload (including an unrecognized/future version byte) -- NEVER +//! panic or unwrap -- so a single corrupt or newer-than-supported record +//! degrades to "skip and warn" at the replay call site instead of aborting +//! the whole WAL scan. +//! +//! `MqCreate` (0x70) and `MqAck` (0x71) keep their pre-existing WAL +//! discriminants but their payload layout is bumped to this versioned, +//! db-index-carrying form (fixing the "replay always assumes db 0" bug). +//! This is a deliberate breaking change to those two payloads' bytes, +//! precedented by the 0x51->0x53 XactCommit format freeze documented in +//! `WalRecordType`: WAL v3 segments are short-lived/rotated and carry no +//! cross-version compatibility contract. A pre-K2 unversioned payload simply +//! fails to decode under the new layout and is skipped like any other +//! malformed record. + +use bytes::Bytes; + +/// Only supported payload version for every MQ WAL record kind. +pub const MQ_WAL_VERSION: u8 = 1; + +/// Peek the leading version byte of an MQ WAL payload without fully +/// decoding it. Used by replay call sites to distinguish "malformed" from +/// "well-formed but a version newer than this build understands" for +/// logging purposes (`decode_*` returns `None` for both cases; skip-and-warn +/// applies either way, but the log message differs). +#[inline] +pub fn peek_version(payload: &[u8]) -> Option { + payload.first().copied() +} + +// ── MqCreate (0x70) ─────────────────────────────────────────────────────── /// Encode an MqCreate WAL payload. /// -/// Layout: `[key_len: u32 LE][key: N bytes][max_delivery_count: u32 LE]` -pub fn encode_mq_create(queue_key: &[u8], max_delivery_count: u32) -> Vec { - let mut payload = Vec::with_capacity(4 + queue_key.len() + 4); +/// Layout: `[version:u8=1][db_index:u32 LE][key_len:u32 LE][key:N][max_delivery_count:u32 LE]` +pub fn encode_mq_create(db_index: u32, queue_key: &[u8], max_delivery_count: u32) -> Vec { + let mut payload = Vec::with_capacity(1 + 4 + 4 + queue_key.len() + 4); + payload.push(MQ_WAL_VERSION); + payload.extend_from_slice(&db_index.to_le_bytes()); payload.extend_from_slice(&(queue_key.len() as u32).to_le_bytes()); payload.extend_from_slice(queue_key); payload.extend_from_slice(&max_delivery_count.to_le_bytes()); @@ -20,30 +58,36 @@ pub fn encode_mq_create(queue_key: &[u8], max_delivery_count: u32) -> Vec { /// Decode an MqCreate WAL payload. /// -/// Returns `(queue_key, max_delivery_count)` or `None` if malformed. -/// Returns owned `Vec` for queue_key to avoid lifetime issues in WAL replay. -pub fn decode_mq_create(payload: &[u8]) -> Option<(Vec, u32)> { - // Minimum: 4 (key_len) = 4 bytes to read the length - if payload.len() < 4 { +/// Returns `(db_index, queue_key, max_delivery_count)` or `None` if +/// malformed or an unsupported version. +pub fn decode_mq_create(payload: &[u8]) -> Option<(u32, Vec, u32)> { + if payload.is_empty() || payload[0] != MQ_WAL_VERSION { + return None; + } + let p = &payload[1..]; + if p.len() < 8 { return None; } - let key_len = u32::from_le_bytes(payload[..4].try_into().ok()?) as usize; - // Need: 4 (key_len) + key_len + 4 (max_delivery_count) - if payload.len() < 4 + key_len + 4 { + let db_index = u32::from_le_bytes(p[0..4].try_into().ok()?); + let key_len = u32::from_le_bytes(p[4..8].try_into().ok()?) as usize; + if p.len() < 8 + key_len + 4 { return None; } - let key = payload[4..4 + key_len].to_vec(); - let mdc_offset = 4 + key_len; - let max_delivery_count = - u32::from_le_bytes(payload[mdc_offset..mdc_offset + 4].try_into().ok()?); - Some((key, max_delivery_count)) + let key = p[8..8 + key_len].to_vec(); + let mdc_offset = 8 + key_len; + let max_delivery_count = u32::from_le_bytes(p[mdc_offset..mdc_offset + 4].try_into().ok()?); + Some((db_index, key, max_delivery_count)) } +// ── MqAck (0x71) ────────────────────────────────────────────────────────── + /// Encode an MqAck WAL payload. /// -/// Layout: `[key_len: u32 LE][key: N bytes][msg_id_ms: u64 LE][msg_id_seq: u64 LE]` -pub fn encode_mq_ack(queue_key: &[u8], msg_id_ms: u64, msg_id_seq: u64) -> Vec { - let mut payload = Vec::with_capacity(4 + queue_key.len() + 16); +/// Layout: `[version:u8=1][db_index:u32 LE][key_len:u32 LE][key:N][msg_id_ms:u64 LE][msg_id_seq:u64 LE]` +pub fn encode_mq_ack(db_index: u32, queue_key: &[u8], msg_id_ms: u64, msg_id_seq: u64) -> Vec { + let mut payload = Vec::with_capacity(1 + 4 + 4 + queue_key.len() + 16); + payload.push(MQ_WAL_VERSION); + payload.extend_from_slice(&db_index.to_le_bytes()); payload.extend_from_slice(&(queue_key.len() as u32).to_le_bytes()); payload.extend_from_slice(queue_key); payload.extend_from_slice(&msg_id_ms.to_le_bytes()); @@ -53,24 +97,307 @@ pub fn encode_mq_ack(queue_key: &[u8], msg_id_ms: u64, msg_id_seq: u64) -> Vec` for queue_key to avoid lifetime issues in WAL replay. -pub fn decode_mq_ack(payload: &[u8]) -> Option<(Vec, u64, u64)> { - // Minimum: 4 (key_len) = 4 bytes to read the length - if payload.len() < 4 { +/// Returns `(db_index, queue_key, ms, seq)` or `None` if malformed or an +/// unsupported version. +pub fn decode_mq_ack(payload: &[u8]) -> Option<(u32, Vec, u64, u64)> { + if payload.is_empty() || payload[0] != MQ_WAL_VERSION { return None; } - let key_len = u32::from_le_bytes(payload[..4].try_into().ok()?) as usize; - // Need: 4 (key_len) + key_len + 8 (ms) + 8 (seq) = 4 + key_len + 16 - if payload.len() < 4 + key_len + 16 { + let p = &payload[1..]; + if p.len() < 8 { return None; } - let key = payload[4..4 + key_len].to_vec(); - let ms_offset = 4 + key_len; - let ms = u64::from_le_bytes(payload[ms_offset..ms_offset + 8].try_into().ok()?); + let db_index = u32::from_le_bytes(p[0..4].try_into().ok()?); + let key_len = u32::from_le_bytes(p[4..8].try_into().ok()?) as usize; + if p.len() < 8 + key_len + 16 { + return None; + } + let key = p[8..8 + key_len].to_vec(); + let ms_offset = 8 + key_len; + let ms = u64::from_le_bytes(p[ms_offset..ms_offset + 8].try_into().ok()?); let seq_offset = ms_offset + 8; - let seq = u64::from_le_bytes(payload[seq_offset..seq_offset + 8].try_into().ok()?); - Some((key, ms, seq)) + let seq = u64::from_le_bytes(p[seq_offset..seq_offset + 8].try_into().ok()?); + Some((db_index, key, ms, seq)) +} + +// ── MqPush (0x72) ───────────────────────────────────────────────────────── + +/// Encode an MqPush WAL payload. Captures the ASSIGNED message id (not the +/// request), so replay is outcome-deterministic regardless of wall-clock +/// skew between the original write and the replay pass. +/// +/// Layout: +/// `[version:u8=1][db_index:u32][key_len:u32][key:N][id_ms:u64][id_seq:u64]` +/// `[field_count:u32]` then `field_count` times `[flen:u32][f:N][vlen:u32][v:N]` +pub fn encode_mq_push( + db_index: u32, + queue_key: &[u8], + id_ms: u64, + id_seq: u64, + fields: &[(Bytes, Bytes)], +) -> Vec { + let fields_size: usize = fields.iter().map(|(f, v)| 8 + f.len() + v.len()).sum(); + let mut payload = Vec::with_capacity(1 + 4 + 4 + queue_key.len() + 16 + 4 + fields_size); + payload.push(MQ_WAL_VERSION); + payload.extend_from_slice(&db_index.to_le_bytes()); + payload.extend_from_slice(&(queue_key.len() as u32).to_le_bytes()); + payload.extend_from_slice(queue_key); + payload.extend_from_slice(&id_ms.to_le_bytes()); + payload.extend_from_slice(&id_seq.to_le_bytes()); + payload.extend_from_slice(&(fields.len() as u32).to_le_bytes()); + for (f, v) in fields { + payload.extend_from_slice(&(f.len() as u32).to_le_bytes()); + payload.extend_from_slice(f); + payload.extend_from_slice(&(v.len() as u32).to_le_bytes()); + payload.extend_from_slice(v); + } + payload +} + +/// Decode an MqPush WAL payload. +/// +/// Returns `(db_index, queue_key, id_ms, id_seq, fields)` or `None` if +/// malformed or an unsupported version. +#[allow(clippy::type_complexity)] +pub fn decode_mq_push(payload: &[u8]) -> Option<(u32, Vec, u64, u64, Vec<(Bytes, Bytes)>)> { + if payload.is_empty() || payload[0] != MQ_WAL_VERSION { + return None; + } + let p = &payload[1..]; + if p.len() < 8 { + return None; + } + let db_index = u32::from_le_bytes(p[0..4].try_into().ok()?); + let key_len = u32::from_le_bytes(p[4..8].try_into().ok()?) as usize; + let mut off = 8usize; + if p.len() < off + key_len { + return None; + } + let key = p[off..off + key_len].to_vec(); + off += key_len; + if p.len() < off + 20 { + return None; + } + let id_ms = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + off += 8; + let id_seq = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + off += 8; + let field_count = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + + let mut fields = Vec::with_capacity(field_count.min(4096)); + for _ in 0..field_count { + if p.len() < off + 4 { + return None; + } + let flen = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + if p.len() < off + flen + 4 { + return None; + } + let f = Bytes::copy_from_slice(&p[off..off + flen]); + off += flen; + let vlen = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + if p.len() < off + vlen { + return None; + } + let v = Bytes::copy_from_slice(&p[off..off + vlen]); + off += vlen; + fields.push((f, v)); + } + + Some((db_index, key, id_ms, id_seq, fields)) +} + +// ── MqPop (0x73) ────────────────────────────────────────────────────────── + +/// One claimed entry: id + resulting delivery_count. +pub type ClaimedEntry = (u64, u64, u64); +/// One DLQ routing decision: (source_ms, source_seq, dlq_ms, dlq_seq). +pub type DlqRouting = (u64, u64, u64, u64); + +/// Encode an MqPop WAL payload. Captures the full outcome of a POP: which +/// ids were claimed (and at what delivery_count), the consumer group's +/// resulting `last_delivered_id`, and which claimed ids were immediately +/// routed to the DLQ (source id -> assigned DLQ-stream id). The fixed +/// group/consumer names (`__mq_consumers` / `__mq_default`) used by every +/// MQ.POP are NOT carried -- baked in at replay, matching the single-group- +/// per-queue design `src/shard/mq_exec.rs` implements today. +/// +/// Layout: +/// `[version:u8=1][db_index:u32][key_len:u32][key:N]` +/// `[last_delivered_ms:u64][last_delivered_seq:u64]` +/// `[claimed_count:u32]` then per entry `[ms:u64][seq:u64][delivery_count:u64]` +/// `[dlq_count:u32]` then per entry `[src_ms:u64][src_seq:u64][dlq_ms:u64][dlq_seq:u64]` +pub fn encode_mq_pop( + db_index: u32, + queue_key: &[u8], + last_delivered: (u64, u64), + claimed: &[ClaimedEntry], + dlq: &[DlqRouting], +) -> Vec { + let mut payload = Vec::with_capacity( + 1 + 4 + 4 + queue_key.len() + 16 + 4 + claimed.len() * 24 + 4 + dlq.len() * 32, + ); + payload.push(MQ_WAL_VERSION); + payload.extend_from_slice(&db_index.to_le_bytes()); + payload.extend_from_slice(&(queue_key.len() as u32).to_le_bytes()); + payload.extend_from_slice(queue_key); + payload.extend_from_slice(&last_delivered.0.to_le_bytes()); + payload.extend_from_slice(&last_delivered.1.to_le_bytes()); + payload.extend_from_slice(&(claimed.len() as u32).to_le_bytes()); + for (ms, seq, dc) in claimed { + payload.extend_from_slice(&ms.to_le_bytes()); + payload.extend_from_slice(&seq.to_le_bytes()); + payload.extend_from_slice(&dc.to_le_bytes()); + } + payload.extend_from_slice(&(dlq.len() as u32).to_le_bytes()); + for (src_ms, src_seq, dlq_ms, dlq_seq) in dlq { + payload.extend_from_slice(&src_ms.to_le_bytes()); + payload.extend_from_slice(&src_seq.to_le_bytes()); + payload.extend_from_slice(&dlq_ms.to_le_bytes()); + payload.extend_from_slice(&dlq_seq.to_le_bytes()); + } + payload +} + +/// Decode an MqPop WAL payload. +/// +/// Returns `(db_index, queue_key, last_delivered, claimed, dlq)` or `None` +/// if malformed or an unsupported version. +#[allow(clippy::type_complexity)] +pub fn decode_mq_pop( + payload: &[u8], +) -> Option<(u32, Vec, (u64, u64), Vec, Vec)> { + if payload.is_empty() || payload[0] != MQ_WAL_VERSION { + return None; + } + let p = &payload[1..]; + if p.len() < 8 { + return None; + } + let db_index = u32::from_le_bytes(p[0..4].try_into().ok()?); + let key_len = u32::from_le_bytes(p[4..8].try_into().ok()?) as usize; + let mut off = 8usize; + if p.len() < off + key_len { + return None; + } + let key = p[off..off + key_len].to_vec(); + off += key_len; + if p.len() < off + 16 + 4 { + return None; + } + let last_ms = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + off += 8; + let last_seq = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + off += 8; + + let claimed_count = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + let mut claimed = Vec::with_capacity(claimed_count.min(65536)); + for _ in 0..claimed_count { + if p.len() < off + 24 { + return None; + } + let ms = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + let seq = u64::from_le_bytes(p[off + 8..off + 16].try_into().ok()?); + let dc = u64::from_le_bytes(p[off + 16..off + 24].try_into().ok()?); + off += 24; + claimed.push((ms, seq, dc)); + } + + if p.len() < off + 4 { + return None; + } + let dlq_count = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + let mut dlq = Vec::with_capacity(dlq_count.min(65536)); + for _ in 0..dlq_count { + if p.len() < off + 32 { + return None; + } + let src_ms = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + let src_seq = u64::from_le_bytes(p[off + 8..off + 16].try_into().ok()?); + let dlq_ms = u64::from_le_bytes(p[off + 16..off + 24].try_into().ok()?); + let dlq_seq = u64::from_le_bytes(p[off + 24..off + 32].try_into().ok()?); + off += 32; + dlq.push((src_ms, src_seq, dlq_ms, dlq_seq)); + } + + Some((db_index, key, (last_ms, last_seq), claimed, dlq)) +} + +// ── MqTrigger (0x74) ────────────────────────────────────────────────────── + +/// Encode an MqTrigger WAL payload. The trigger registry is shard-level +/// (not per-db), so unlike the other MQ records this carries no db_index -- +/// mirrors `src/shard/mq_exec.rs::handle_trigger`, which explicitly ignores +/// `db_index` for the same reason. `last_fire_ms`/`pending_fire_ms` are +/// transient runtime state (not persisted); replay always restores a fresh +/// entry with both at 0, matching a never-yet-armed registration. +/// +/// Layout: +/// `[version:u8=1][trig_key_len:u32][trig_key:N][queue_key_len:u32][queue_key:N]` +/// `[callback_len:u32][callback:N][debounce_ms:u64]` +pub fn encode_mq_trigger( + trig_key: &[u8], + queue_key: &[u8], + callback_cmd: &[u8], + debounce_ms: u64, +) -> Vec { + let mut payload = Vec::with_capacity( + 1 + 4 + trig_key.len() + 4 + queue_key.len() + 4 + callback_cmd.len() + 8, + ); + payload.push(MQ_WAL_VERSION); + payload.extend_from_slice(&(trig_key.len() as u32).to_le_bytes()); + payload.extend_from_slice(trig_key); + payload.extend_from_slice(&(queue_key.len() as u32).to_le_bytes()); + payload.extend_from_slice(queue_key); + payload.extend_from_slice(&(callback_cmd.len() as u32).to_le_bytes()); + payload.extend_from_slice(callback_cmd); + payload.extend_from_slice(&debounce_ms.to_le_bytes()); + payload +} + +/// Decode an MqTrigger WAL payload. +/// +/// Returns `(trig_key, queue_key, callback_cmd, debounce_ms)` or `None` if +/// malformed or an unsupported version. +#[allow(clippy::type_complexity)] +pub fn decode_mq_trigger(payload: &[u8]) -> Option<(Vec, Vec, Vec, u64)> { + if payload.is_empty() || payload[0] != MQ_WAL_VERSION { + return None; + } + let p = &payload[1..]; + if p.len() < 4 { + return None; + } + let mut off = 0usize; + let trig_key_len = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + if p.len() < off + trig_key_len + 4 { + return None; + } + let trig_key = p[off..off + trig_key_len].to_vec(); + off += trig_key_len; + let queue_key_len = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + if p.len() < off + queue_key_len + 4 { + return None; + } + let queue_key = p[off..off + queue_key_len].to_vec(); + off += queue_key_len; + let callback_len = u32::from_le_bytes(p[off..off + 4].try_into().ok()?) as usize; + off += 4; + if p.len() < off + callback_len + 8 { + return None; + } + let callback_cmd = p[off..off + callback_len].to_vec(); + off += callback_len; + let debounce_ms = u64::from_le_bytes(p[off..off + 8].try_into().ok()?); + + Some((trig_key, queue_key, callback_cmd, debounce_ms)) } #[cfg(test)] @@ -81,34 +408,34 @@ mod tests { #[test] fn test_mq_create_roundtrip() { - let key = b"orders"; - let max_delivery = 5u32; - let payload = encode_mq_create(key, max_delivery); - - let (decoded_key, decoded_max) = decode_mq_create(&payload).unwrap(); - assert_eq!(decoded_key, key); - assert_eq!(decoded_max, max_delivery); + let payload = encode_mq_create(2, b"orders", 5); + let (db, key, mdc) = decode_mq_create(&payload).unwrap(); + assert_eq!(db, 2); + assert_eq!(key, b"orders"); + assert_eq!(mdc, 5); } #[test] fn test_mq_create_roundtrip_zero_delivery() { - let payload = encode_mq_create(b"q", 0); - let (key, mdc) = decode_mq_create(&payload).unwrap(); + let payload = encode_mq_create(0, b"q", 0); + let (db, key, mdc) = decode_mq_create(&payload).unwrap(); + assert_eq!(db, 0); assert_eq!(key, b"q"); assert_eq!(mdc, 0); } #[test] fn test_mq_create_roundtrip_max_delivery() { - let payload = encode_mq_create(b"q", u32::MAX); - let (_, mdc) = decode_mq_create(&payload).unwrap(); + let payload = encode_mq_create(15, b"q", u32::MAX); + let (db, _, mdc) = decode_mq_create(&payload).unwrap(); + assert_eq!(db, 15); assert_eq!(mdc, u32::MAX); } #[test] fn test_mq_create_roundtrip_empty_key() { - let payload = encode_mq_create(b"", 3); - let (key, mdc) = decode_mq_create(&payload).unwrap(); + let payload = encode_mq_create(1, b"", 3); + let (_, key, mdc) = decode_mq_create(&payload).unwrap(); assert!(key.is_empty()); assert_eq!(mdc, 3); } @@ -116,8 +443,9 @@ mod tests { #[test] fn test_mq_create_roundtrip_long_key() { let long_key = vec![b'x'; 1024]; - let payload = encode_mq_create(&long_key, 10); - let (key, mdc) = decode_mq_create(&payload).unwrap(); + let payload = encode_mq_create(3, &long_key, 10); + let (db, key, mdc) = decode_mq_create(&payload).unwrap(); + assert_eq!(db, 3); assert_eq!(key, long_key); assert_eq!(mdc, 10); } @@ -129,47 +457,53 @@ mod tests { #[test] fn test_mq_create_malformed_too_short() { - // Only 3 bytes -- can't even read key_len - assert!(decode_mq_create(&[0, 0, 0]).is_none()); + assert!(decode_mq_create(&[1, 0, 0]).is_none()); } #[test] fn test_mq_create_malformed_truncated_key() { - // key_len says 100 but payload is too short - let mut bad = Vec::new(); - bad.extend_from_slice(&100u32.to_le_bytes()); + let mut bad = vec![MQ_WAL_VERSION]; + bad.extend_from_slice(&0u32.to_le_bytes()); // db_index + bad.extend_from_slice(&100u32.to_le_bytes()); // key_len = 100 bad.extend_from_slice(&[0u8; 10]); // only 10 bytes of key assert!(decode_mq_create(&bad).is_none()); } #[test] - fn test_mq_create_malformed_missing_max_delivery() { - // key_len = 4, key = "test", but no max_delivery_count bytes - let mut bad = Vec::new(); - bad.extend_from_slice(&4u32.to_le_bytes()); - bad.extend_from_slice(b"test"); - assert!(decode_mq_create(&bad).is_none()); + fn test_mq_create_unknown_version_rejected() { + let mut payload = encode_mq_create(0, b"q", 5); + payload[0] = 99; // future version + assert!(decode_mq_create(&payload).is_none()); + assert_eq!(peek_version(&payload), Some(99)); + } + + #[test] + fn test_mq_create_extra_bytes_ignored() { + let mut payload = encode_mq_create(1, b"q", 5); + payload.extend_from_slice(b"trailing"); + let (db, key, mdc) = decode_mq_create(&payload).unwrap(); + assert_eq!(db, 1); + assert_eq!(key, b"q"); + assert_eq!(mdc, 5); } // --- MqAck roundtrip --- #[test] fn test_mq_ack_roundtrip() { - let key = b"orders"; - let ms = 1_713_394_800_000u64; - let seq = 42u64; - let payload = encode_mq_ack(key, ms, seq); - - let (decoded_key, decoded_ms, decoded_seq) = decode_mq_ack(&payload).unwrap(); - assert_eq!(decoded_key, key); - assert_eq!(decoded_ms, ms); - assert_eq!(decoded_seq, seq); + let payload = encode_mq_ack(4, b"orders", 1_713_394_800_000u64, 42); + let (db, key, ms, seq) = decode_mq_ack(&payload).unwrap(); + assert_eq!(db, 4); + assert_eq!(key, b"orders"); + assert_eq!(ms, 1_713_394_800_000u64); + assert_eq!(seq, 42); } #[test] fn test_mq_ack_roundtrip_zero_id() { - let payload = encode_mq_ack(b"q", 0, 0); - let (key, ms, seq) = decode_mq_ack(&payload).unwrap(); + let payload = encode_mq_ack(0, b"q", 0, 0); + let (db, key, ms, seq) = decode_mq_ack(&payload).unwrap(); + assert_eq!(db, 0); assert_eq!(key, b"q"); assert_eq!(ms, 0); assert_eq!(seq, 0); @@ -177,67 +511,170 @@ mod tests { #[test] fn test_mq_ack_roundtrip_max_id() { - let payload = encode_mq_ack(b"q", u64::MAX, u64::MAX); - let (_, ms, seq) = decode_mq_ack(&payload).unwrap(); + let payload = encode_mq_ack(0, b"q", u64::MAX, u64::MAX); + let (_, _, ms, seq) = decode_mq_ack(&payload).unwrap(); assert_eq!(ms, u64::MAX); assert_eq!(seq, u64::MAX); } #[test] - fn test_mq_ack_roundtrip_empty_key() { - let payload = encode_mq_ack(b"", 100, 200); - let (key, ms, seq) = decode_mq_ack(&payload).unwrap(); - assert!(key.is_empty()); - assert_eq!(ms, 100); - assert_eq!(seq, 200); + fn test_mq_ack_malformed_empty() { + assert!(decode_mq_ack(b"").is_none()); } #[test] - fn test_mq_ack_malformed_empty() { - assert!(decode_mq_ack(b"").is_none()); + fn test_mq_ack_malformed_missing_seq() { + let mut bad = vec![MQ_WAL_VERSION]; + bad.extend_from_slice(&0u32.to_le_bytes()); + bad.extend_from_slice(&2u32.to_le_bytes()); + bad.extend_from_slice(b"ok"); + bad.extend_from_slice(&1000u64.to_le_bytes()); + // Missing seq bytes + assert!(decode_mq_ack(&bad).is_none()); } #[test] - fn test_mq_ack_malformed_too_short() { - assert!(decode_mq_ack(&[0, 0, 0]).is_none()); + fn test_mq_ack_unknown_version_rejected() { + let mut payload = encode_mq_ack(0, b"q", 100, 200); + payload[0] = 2; + assert!(decode_mq_ack(&payload).is_none()); } + // --- MqPush roundtrip --- + #[test] - fn test_mq_ack_malformed_truncated_key() { - let mut bad = Vec::new(); - bad.extend_from_slice(&100u32.to_le_bytes()); - bad.extend_from_slice(&[0u8; 10]); - assert!(decode_mq_ack(&bad).is_none()); + fn test_mq_push_roundtrip() { + let fields = vec![ + (Bytes::from_static(b"seq"), Bytes::from_static(b"1")), + (Bytes::from_static(b"name"), Bytes::from_static(b"alice")), + ]; + let payload = encode_mq_push(3, b"orders", 1000, 5, &fields); + let (db, key, ms, seq, decoded_fields) = decode_mq_push(&payload).unwrap(); + assert_eq!(db, 3); + assert_eq!(key, b"orders"); + assert_eq!(ms, 1000); + assert_eq!(seq, 5); + assert_eq!(decoded_fields, fields); } #[test] - fn test_mq_ack_malformed_missing_seq() { - // key_len = 2, key = "ok", ms = 8 bytes, but no seq - let mut bad = Vec::new(); - bad.extend_from_slice(&2u32.to_le_bytes()); - bad.extend_from_slice(b"ok"); - bad.extend_from_slice(&1000u64.to_le_bytes()); - // Missing seq bytes - assert!(decode_mq_ack(&bad).is_none()); + fn test_mq_push_roundtrip_no_fields() { + let payload = encode_mq_push(0, b"q", 1, 0, &[]); + let (_, _, _, _, fields) = decode_mq_push(&payload).unwrap(); + assert!(fields.is_empty()); } #[test] - fn test_mq_ack_extra_bytes_ignored() { - // Extra bytes after valid payload should not cause failure - let mut payload = encode_mq_ack(b"q", 100, 200); - payload.extend_from_slice(b"extra_trailing_data"); - let (key, ms, seq) = decode_mq_ack(&payload).unwrap(); - assert_eq!(key, b"q"); - assert_eq!(ms, 100); - assert_eq!(seq, 200); + fn test_mq_push_roundtrip_empty_field_value() { + let fields = vec![(Bytes::from_static(b""), Bytes::from_static(b""))]; + let payload = encode_mq_push(0, b"q", 1, 0, &fields); + let (_, _, _, _, decoded) = decode_mq_push(&payload).unwrap(); + assert_eq!(decoded, fields); } #[test] - fn test_mq_create_extra_bytes_ignored() { - let mut payload = encode_mq_create(b"q", 5); - payload.extend_from_slice(b"trailing"); - let (key, mdc) = decode_mq_create(&payload).unwrap(); - assert_eq!(key, b"q"); - assert_eq!(mdc, 5); + fn test_mq_push_malformed_truncated_fields() { + let mut bad = vec![MQ_WAL_VERSION]; + bad.extend_from_slice(&0u32.to_le_bytes()); // db + bad.extend_from_slice(&1u32.to_le_bytes()); // key_len + bad.push(b'q'); + bad.extend_from_slice(&1u64.to_le_bytes()); // id_ms + bad.extend_from_slice(&0u64.to_le_bytes()); // id_seq + bad.extend_from_slice(&1u32.to_le_bytes()); // field_count = 1 + bad.extend_from_slice(&100u32.to_le_bytes()); // flen = 100 but nothing follows + assert!(decode_mq_push(&bad).is_none()); + } + + #[test] + fn test_mq_push_unknown_version_rejected() { + let mut payload = encode_mq_push(0, b"q", 1, 0, &[]); + payload[0] = 7; + assert!(decode_mq_push(&payload).is_none()); + } + + // --- MqPop roundtrip --- + + #[test] + fn test_mq_pop_roundtrip() { + let claimed: Vec = vec![(1, 0, 1), (1, 1, 1), (1, 2, 2)]; + let dlq: Vec = vec![(1, 2, 2, 0)]; + let payload = encode_mq_pop(1, b"orders", (1, 2), &claimed, &dlq); + let (db, key, last, decoded_claimed, decoded_dlq) = decode_mq_pop(&payload).unwrap(); + assert_eq!(db, 1); + assert_eq!(key, b"orders"); + assert_eq!(last, (1, 2)); + assert_eq!(decoded_claimed, claimed); + assert_eq!(decoded_dlq, dlq); + } + + #[test] + fn test_mq_pop_roundtrip_empty() { + let payload = encode_mq_pop(0, b"q", (0, 0), &[], &[]); + let (_, _, last, claimed, dlq) = decode_mq_pop(&payload).unwrap(); + assert_eq!(last, (0, 0)); + assert!(claimed.is_empty()); + assert!(dlq.is_empty()); + } + + #[test] + fn test_mq_pop_malformed_truncated_claimed() { + let mut bad = vec![MQ_WAL_VERSION]; + bad.extend_from_slice(&0u32.to_le_bytes()); + bad.extend_from_slice(&1u32.to_le_bytes()); + bad.push(b'q'); + bad.extend_from_slice(&0u64.to_le_bytes()); + bad.extend_from_slice(&0u64.to_le_bytes()); + bad.extend_from_slice(&5u32.to_le_bytes()); // claims 5 but none follow + assert!(decode_mq_pop(&bad).is_none()); + } + + #[test] + fn test_mq_pop_unknown_version_rejected() { + let mut payload = encode_mq_pop(0, b"q", (0, 0), &[], &[]); + payload[0] = 42; + assert!(decode_mq_pop(&payload).is_none()); + } + + // --- MqTrigger roundtrip --- + + #[test] + fn test_mq_trigger_roundtrip() { + let payload = encode_mq_trigger(b"wshex:orders", b"orders", b"PUBLISH ch msg", 1500); + let (trig_key, queue_key, callback, debounce) = decode_mq_trigger(&payload).unwrap(); + assert_eq!(trig_key, b"wshex:orders"); + assert_eq!(queue_key, b"orders"); + assert_eq!(callback, b"PUBLISH ch msg"); + assert_eq!(debounce, 1500); + } + + #[test] + fn test_mq_trigger_roundtrip_empty_callback() { + let payload = encode_mq_trigger(b"q", b"q", b"", 0); + let (_, _, callback, debounce) = decode_mq_trigger(&payload).unwrap(); + assert!(callback.is_empty()); + assert_eq!(debounce, 0); + } + + #[test] + fn test_mq_trigger_malformed_truncated_callback() { + let mut bad = vec![MQ_WAL_VERSION]; + bad.extend_from_slice(&1u32.to_le_bytes()); + bad.push(b'q'); + bad.extend_from_slice(&1u32.to_le_bytes()); + bad.push(b'q'); + bad.extend_from_slice(&50u32.to_le_bytes()); // callback_len = 50, nothing follows + assert!(decode_mq_trigger(&bad).is_none()); + } + + #[test] + fn test_mq_trigger_unknown_version_rejected() { + let mut payload = encode_mq_trigger(b"q", b"q", b"CMD", 100); + payload[0] = 9; + assert!(decode_mq_trigger(&payload).is_none()); + } + + #[test] + fn test_peek_version_empty() { + assert_eq!(peek_version(&[]), None); } } diff --git a/src/persistence/wal_v3/record.rs b/src/persistence/wal_v3/record.rs index b48c7330..bc02bf4d 100644 --- a/src/persistence/wal_v3/record.rs +++ b/src/persistence/wal_v3/record.rs @@ -70,10 +70,30 @@ pub enum WalRecordType { WorkspaceCreate = 0x60, /// Workspace deletion record. WorkspaceDrop = 0x61, - /// MQ queue creation record. + /// MQ queue creation record. Wave B stage 2a bumped the payload to a + /// versioned form (leading version byte + db index) — see + /// `crate::mq::wal::encode_mq_create`. The discriminant is unchanged + /// (precedent: the 0x51->0x53 XactCommit format freeze), pre-K2 payloads + /// are simply no longer decodable and are skipped-with-warn like any + /// other malformed record. MqCreate = 0x70, - /// MQ message acknowledge record (used for cursor-rollback). + /// MQ message acknowledge record, applied by id (not count) at replay. + /// Payload versioned + db-index-carrying as of Wave B stage 2a, same + /// discriminant-preserved bump as `MqCreate`. MqAck = 0x71, + /// MQ message push record (Wave B stage 2a). Captures the ASSIGNED + /// message id + field/value pairs so replay can rebuild stream content + /// deterministically without depending on wall-clock id generation. + MqPush = 0x72, + /// MQ message pop/claim record (Wave B stage 2a). Captures claimed ids + /// (with resulting delivery_count), the consumer group's resulting + /// `last_delivered_id`, and any DLQ routing decisions (source id -> + /// assigned DLQ id) taken by that pop. + MqPop = 0x73, + /// MQ trigger registration record (Wave B stage 2a). Registration is + /// durable/replayed as opaque data; the callback is never fired during + /// replay (only live `MQ.PUSH` debounce arming fires it). + MqTrigger = 0x74, } impl WalRecordType { @@ -101,6 +121,9 @@ impl WalRecordType { 0x61 => Some(Self::WorkspaceDrop), 0x70 => Some(Self::MqCreate), 0x71 => Some(Self::MqAck), + 0x72 => Some(Self::MqPush), + 0x73 => Some(Self::MqPop), + 0x74 => Some(Self::MqTrigger), _ => None, } } @@ -500,6 +523,9 @@ mod tests { assert_eq!(WalRecordType::WorkspaceDrop as u8, 0x61); assert_eq!(WalRecordType::MqCreate as u8, 0x70); assert_eq!(WalRecordType::MqAck as u8, 0x71); + assert_eq!(WalRecordType::MqPush as u8, 0x72); + assert_eq!(WalRecordType::MqPop as u8, 0x73); + assert_eq!(WalRecordType::MqTrigger as u8, 0x74); // from_u8 roundtrips for &v in &[ diff --git a/src/persistence/wal_v3/replay.rs b/src/persistence/wal_v3/replay.rs index 2fcb6783..2aa7e8e3 100644 --- a/src/persistence/wal_v3/replay.rs +++ b/src/persistence/wal_v3/replay.rs @@ -413,7 +413,10 @@ pub fn replay_wal_v3_file_until( | WalRecordType::WorkspaceCreate | WalRecordType::WorkspaceDrop | WalRecordType::MqCreate - | WalRecordType::MqAck => { + | WalRecordType::MqAck + | WalRecordType::MqPush + | WalRecordType::MqPop + | WalRecordType::MqTrigger => { on_command(&record); result.commands_replayed += 1; } From b87e516771d41c4e74a63cdc5db92c28c07f7af3 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 10:12:16 +0700 Subject: [PATCH 2/7] feat(mq): emit WAL effect records at MQ owner-shard execution sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the new versioned MQ WAL codec (src/mq/wal.rs) into every owner-shard MQ command handler in src/shard/mq_exec.rs: - handle_create: MqCreate now carries db_index (was hardcoded to 0). - handle_push: builds the MqPush payload inside the with_shard_db closure, before `fields` is moved into `stream.add`, then emits it after the closure returns. - handle_pop: threads a WAL payload out of every early-return path (restructured to return `(Frame, Option>)`); records the full claimed-id set with delivery counts, the resulting last_delivered_id, and any DLQ routing decisions (source id -> assigned DLQ id) so replay can reproduce POP's outcome exactly instead of re-deriving it. - handle_ack: emits one MqAck record per acked id (by id, not by count — the replay-side fix landing in the next commit depends on this). - handle_trigger: emits MqTrigger before the registry insert consumes the key/callback bytes. `wal_append_on_slice` moves from private to `pub(crate)` since a later commit calls it from the TXN materialization hop (handler_monoio/txn.rs, handler_sharded/txn.rs, spsc_handler.rs) as well. No replay-side changes yet — these records land on disk but are not yet applied on restart (next commit). author: Tin Dang --- src/shard/mq_exec.rs | 275 +++++++++++++++++++++++++++++-------------- 1 file changed, 184 insertions(+), 91 deletions(-) diff --git a/src/shard/mq_exec.rs b/src/shard/mq_exec.rs index bf838422..5d04e31b 100644 --- a/src/shard/mq_exec.rs +++ b/src/shard/mq_exec.rs @@ -191,8 +191,13 @@ fn derive_trig_key(key_prefix: &Bytes, raw_queue_key: &Bytes) -> Bytes { /// Mirrors `ShardDatabases::wal_append` semantics — `try_send` failures are /// ignored (the channel is bounded; under extreme backpressure the record is /// dropped, same as the existing lock-path). +/// +/// `pub(crate)`: also called from `src/server/conn/handler_monoio/txn.rs` and +/// `src/server/conn/handler_sharded/txn.rs` (MQ.PUBLISH self-fold) and +/// `src/shard/spsc_handler.rs` (`MqTxnMaterialize` foreign-leg fold) to emit +/// `MqPush` records at TXN materialization time (Wave B stage 2a). #[inline] -fn wal_append_on_slice( +pub(crate) fn wal_append_on_slice( record_type: crate::persistence::wal_v3::record::WalRecordType, payload: bytes::Bytes, ) { @@ -243,8 +248,11 @@ fn handle_create(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { // WAL: MqCreate record on the owner shard. K1a: send the unframed payload // tagged with its real type — the event-loop drain does the single framing. + // Wave B stage 2a: payload now carries `db_index` so replay recreates the + // stream in the SAME db it was created in (fixes the db-0 hardcode). { - let payload = crate::mq::wal::encode_mq_create(&eff_key, max_delivery_count); + let payload = + crate::mq::wal::encode_mq_create(db_index as u32, &eff_key, max_delivery_count); wal_append_on_slice( crate::persistence::wal_v3::record::WalRecordType::MqCreate, Bytes::from(payload), @@ -267,8 +275,12 @@ fn handle_push(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { let eff_key = effective_key(key_prefix, &raw_key); let trig_key = derive_trig_key(key_prefix, &raw_key); - // Push into the stream. - type PushResult = Result, Frame>; + // Push into the stream. The WAL payload is built INSIDE the closure + // (before `fields` is moved into `stream.add`) — it captures the + // ASSIGNED id, not the request, so replay is outcome-deterministic. + // Encoding here is a pure function call, not a nested `with_shard*` + // call, so it doesn't violate the non-reentrancy contract. + type PushResult = Result)>, Frame>; let push_result: PushResult = crate::shard::slice::with_shard_db(db_index, |db| match db.get_stream_mut(&eff_key) { Ok(Some(stream)) => { @@ -276,8 +288,15 @@ fn handle_push(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { Ok(None) } else { let msg_id = stream.next_auto_id(); + let payload = crate::mq::wal::encode_mq_push( + db_index as u32, + &eff_key, + msg_id.ms, + msg_id.seq, + &fields, + ); let msg_id = stream.add(msg_id, fields); - Ok(Some(msg_id)) + Ok(Some((msg_id, payload))) } } Ok(None) => Ok(None), @@ -285,7 +304,14 @@ fn handle_push(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { }); match push_result { - Ok(Some(msg_id)) => { + Ok(Some((msg_id, payload))) => { + // WAL: MqPush record on the owner shard (Wave B stage 2a) — + // durability plane only; no replication emission here (stage 2b). + wal_append_on_slice( + crate::persistence::wal_v3::record::WalRecordType::MqPush, + Bytes::from(payload), + ); + // Debounce trigger: set pending_fire_ms if not already armed. let now_ms = current_time_ms(); crate::shard::slice::with_shard(|s| { @@ -316,6 +342,11 @@ fn handle_push(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { /// MQ.POP — owner claims messages, routing max-delivery entries to the DLQ. /// /// Mirrors handler_sharded/write.rs MQ POP arm (lock-path `else` branch). +/// Emits an `MqPop` WAL record capturing the full outcome (claimed ids + +/// resulting delivery_count, the group's resulting `last_delivered_id`, and +/// any DLQ routing decisions) so replay can reproduce it deterministically +/// without recomputing "which entries would this claim" against +/// possibly-different post-crash state. fn handle_pop(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { let (raw_key, count) = match validate_mq_pop(args) { Ok(v) => v, @@ -325,99 +356,145 @@ fn handle_pop(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { let group_name = Bytes::from_static(b"__mq_consumers"); let consumer_name = Bytes::from_static(b"__mq_default"); - // All POP logic runs in a single with_shard_db closure to avoid re-entrancy. - crate::shard::slice::with_shard_db(db_index, |db| { - // Step 1: read max_delivery_count. - let mdc = match db.get_stream_mut(&eff_key) { - Ok(Some(stream)) => { - if !stream.durable { - return Frame::Error(Bytes::from_static(ERR_MQ_NOT_DURABLE)); + // All POP logic runs in a single with_shard_db closure to avoid + // re-entrancy. Returns `(reply, wal_payload)`; `wal_payload` is `None` + // on any error/early-exit path (nothing claimed, nothing to record). + let (reply, wal_payload): (Frame, Option>) = + crate::shard::slice::with_shard_db(db_index, |db| -> (Frame, Option>) { + // Step 1: read max_delivery_count. + let mdc = match db.get_stream_mut(&eff_key) { + Ok(Some(stream)) => { + if !stream.durable { + return (Frame::Error(Bytes::from_static(ERR_MQ_NOT_DURABLE)), None); + } + stream.max_delivery_count } - stream.max_delivery_count - } - Ok(None) => return Frame::Error(Bytes::from_static(ERR_MQ_NOT_DURABLE)), - Err(e) => return e, - }; - - let request_count = count + (mdc as usize); - - // Step 2: read_group_new to claim entries. - let stream = match db.get_stream_mut(&eff_key) { - Ok(Some(s)) => s, - _ => return Frame::Error(Bytes::from_static(ERR_MQ_NOT_DURABLE)), - }; - let claimed = - match stream.read_group_new(&group_name, &consumer_name, Some(request_count), false) { + Ok(None) => return (Frame::Error(Bytes::from_static(ERR_MQ_NOT_DURABLE)), None), + Err(e) => return (e, None), + }; + + let request_count = count + (mdc as usize); + + // Step 2: read_group_new to claim entries. + let stream = match db.get_stream_mut(&eff_key) { + Ok(Some(s)) => s, + _ => return (Frame::Error(Bytes::from_static(ERR_MQ_NOT_DURABLE)), None), + }; + let claimed = match stream.read_group_new( + &group_name, + &consumer_name, + Some(request_count), + false, + ) { Ok(entries) => entries, - Err(_) => return Frame::Array(vec![].into()), + Err(_) => return (Frame::Array(vec![].into()), None), }; + if claimed.is_empty() { + return (Frame::Array(vec![].into()), None); + } - // Step 3: partition claimed into good entries and DLQ entries. - let mut results: Vec<(StreamId, Vec<(Bytes, Bytes)>)> = - Vec::with_capacity(count.min(claimed.len())); - let mut dlq_entries: Vec<(StreamId, Vec<(Bytes, Bytes)>)> = Vec::new(); - let mut dlq_ack_ids: Vec = Vec::new(); + // Step 3: partition claimed into good entries and DLQ entries. + let mut results: Vec<(StreamId, Vec<(Bytes, Bytes)>)> = + Vec::with_capacity(count.min(claimed.len())); + let mut dlq_entries: Vec<(StreamId, Vec<(Bytes, Bytes)>)> = Vec::new(); + let mut dlq_ack_ids: Vec = Vec::new(); + // Every claimed id, with its delivery_count at claim time — + // recorded for the WAL regardless of DLQ routing (mirrors what + // read_group_new just inserted into the PEL). + let mut claimed_for_wal: Vec = + Vec::with_capacity(claimed.len()); + + for (id, fields) in &claimed { + let delivery_count = stream + .groups + .get(group_name.as_ref()) + .and_then(|g| g.pel.get(id)) + .map(|pe| pe.delivery_count) + .unwrap_or(1); + claimed_for_wal.push((id.ms, id.seq, delivery_count)); + if mdc > 0 && delivery_count >= mdc as u64 { + dlq_entries.push((*id, fields.clone())); + dlq_ack_ids.push(*id); + } else if results.len() < count { + results.push((*id, fields.clone())); + } + } + + // Step 4: ACK DLQ entries from the main stream PEL. + if !dlq_ack_ids.is_empty() { + let _ = stream.xack(&group_name, &dlq_ack_ids); + } - for (id, fields) in &claimed { - let delivery_count = stream + let last_delivered_id = stream .groups .get(group_name.as_ref()) - .and_then(|g| g.pel.get(id)) - .map(|pe| pe.delivery_count) - .unwrap_or(1); - if mdc > 0 && delivery_count >= mdc as u64 { - dlq_entries.push((*id, fields.clone())); - dlq_ack_ids.push(*id); - } else if results.len() < count { - results.push((*id, fields.clone())); + .map(|g| g.last_delivered_id) + .unwrap_or(StreamId::ZERO); + + // Step 5: append DLQ entries to the sibling DLQ stream, capturing + // the ASSIGNED dlq-stream id for each (outcome-deterministic — + // replay must reproduce the exact id, not regenerate one from + // its own wall clock). + let mut dlq_for_wal: Vec = + Vec::with_capacity(dlq_entries.len()); + if !dlq_entries.is_empty() { + let dlq_key = { + let mut buf = Vec::with_capacity(eff_key.len() + 8); + buf.extend_from_slice(&eff_key); + buf.extend_from_slice(b"::mq:dlq"); + Bytes::from(buf) + }; + if let Ok(dlq_stream) = db.get_or_create_stream(&dlq_key) { + for (src_id, fields) in dlq_entries { + let dlq_id = dlq_stream.next_auto_id(); + dlq_stream.add(dlq_id, fields); + dlq_for_wal.push((src_id.ms, src_id.seq, dlq_id.ms, dlq_id.seq)); + } + } } - } - // Step 4: ACK DLQ entries from the main stream PEL. - if !dlq_ack_ids.is_empty() { - let _ = stream.xack(&group_name, &dlq_ack_ids); - } + let wal_payload = crate::mq::wal::encode_mq_pop( + db_index as u32, + &eff_key, + (last_delivered_id.ms, last_delivered_id.seq), + &claimed_for_wal, + &dlq_for_wal, + ); - // Step 5: append DLQ entries to the sibling DLQ stream. - if !dlq_entries.is_empty() { - let dlq_key = { - let mut buf = Vec::with_capacity(eff_key.len() + 8); - buf.extend_from_slice(&eff_key); - buf.extend_from_slice(b"::mq:dlq"); - Bytes::from(buf) - }; - if let Ok(dlq_stream) = db.get_or_create_stream(&dlq_key) { - for (_id, fields) in dlq_entries { - let dlq_id = dlq_stream.next_auto_id(); - dlq_stream.add(dlq_id, fields); - } - } - } + // Step 6: build response frames. + let result_frames: Vec = results + .iter() + .map(|(id, fields)| { + let mut entry_frames = Vec::with_capacity(2); + let mut ms_buf = itoa::Buffer::new(); + let mut seq_buf = itoa::Buffer::new(); + let ms_str = ms_buf.format(id.ms); + let seq_str = seq_buf.format(id.seq); + let mut id_bytes = Vec::with_capacity(ms_str.len() + 1 + seq_str.len()); + id_bytes.extend_from_slice(ms_str.as_bytes()); + id_bytes.push(b'-'); + id_bytes.extend_from_slice(seq_str.as_bytes()); + entry_frames.push(Frame::BulkString(Bytes::from(id_bytes))); + let field_frames: Vec = fields + .iter() + .flat_map(|(f, v)| { + [Frame::BulkString(f.clone()), Frame::BulkString(v.clone())] + }) + .collect(); + entry_frames.push(Frame::Array(field_frames.into())); + Frame::Array(entry_frames.into()) + }) + .collect(); + (Frame::Array(result_frames.into()), Some(wal_payload)) + }); - // Step 6: build response frames. - let result_frames: Vec = results - .iter() - .map(|(id, fields)| { - let mut entry_frames = Vec::with_capacity(2); - let mut ms_buf = itoa::Buffer::new(); - let mut seq_buf = itoa::Buffer::new(); - let ms_str = ms_buf.format(id.ms); - let seq_str = seq_buf.format(id.seq); - let mut id_bytes = Vec::with_capacity(ms_str.len() + 1 + seq_str.len()); - id_bytes.extend_from_slice(ms_str.as_bytes()); - id_bytes.push(b'-'); - id_bytes.extend_from_slice(seq_str.as_bytes()); - entry_frames.push(Frame::BulkString(Bytes::from(id_bytes))); - let field_frames: Vec = fields - .iter() - .flat_map(|(f, v)| [Frame::BulkString(f.clone()), Frame::BulkString(v.clone())]) - .collect(); - entry_frames.push(Frame::Array(field_frames.into())); - Frame::Array(entry_frames.into()) - }) - .collect(); - Frame::Array(result_frames.into()) - }) + if let Some(payload) = wal_payload { + wal_append_on_slice( + crate::persistence::wal_v3::record::WalRecordType::MqPop, + Bytes::from(payload), + ); + } + reply } /// MQ.ACK — owner acknowledges one or more message IDs. @@ -450,8 +527,12 @@ fn handle_ack(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame { match ack_result { Some(acked_count) => { // Emit one WAL record per acked message id (unframed, real type). + // Replay applies MqAck BY ID via `Stream::xack` — idempotent even + // if a request id was never actually in the PEL (matches this + // aggregate-count live path, which can't distinguish per-id + // success without changing `Stream::xack`'s return type). for (ms, seq) in &msg_ids { - let payload = crate::mq::wal::encode_mq_ack(&eff_key, *ms, *seq); + let payload = crate::mq::wal::encode_mq_ack(db_index as u32, &eff_key, *ms, *seq); wal_append_on_slice( crate::persistence::wal_v3::record::WalRecordType::MqAck, Bytes::from(payload), @@ -498,6 +579,12 @@ fn handle_trigger(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame let eff_key = effective_key(key_prefix, &raw_key); let trig_key = derive_trig_key(key_prefix, &raw_key); + // WAL: MqTrigger record (Wave B stage 2a) — registration is durable/ + // replayed as opaque data, never fired during replay. Built before the + // registry insert consumes `eff_key`/`callback_cmd`/`trig_key`. + let payload = + crate::mq::wal::encode_mq_trigger(&trig_key, &eff_key, &callback_cmd, debounce_ms); + let entry = crate::mq::TriggerEntry { queue_key: eff_key, callback_cmd, @@ -513,8 +600,14 @@ fn handle_trigger(args: &[Frame], key_prefix: &Bytes, db_index: usize) -> Frame reg.register(trig_key, entry); }); - // `db_index` is unused for TRIGGER (registry only) but kept in the - // signature for API uniformity. Suppress the unused-variable warning. + wal_append_on_slice( + crate::persistence::wal_v3::record::WalRecordType::MqTrigger, + Bytes::from(payload), + ); + + // `db_index` is unused for TRIGGER (registry is shard-level, not + // per-db) but kept in the signature for API uniformity. Suppress the + // unused-variable warning. let _ = db_index; Frame::SimpleString(Bytes::from_static(b"OK")) From 61aecee929506064df6bc99e2c29e2043f8bbdaf Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 10:12:19 +0700 Subject: [PATCH 3/7] fix(mq): replay MQ WAL records strictly in order and ack by id, not count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites replay_mq_wal (src/shard/shared_databases.rs) to apply every MQ WAL record one at a time, in true on-disk LSN order, via a new apply_mq_wal_record dispatcher and five apply_mq_* handlers (create, push, pop, ack, trigger) — replacing the old behavior that collected replayed state into intermediate maps and rolled the consumer group's PEL back to a COUNT-based snapshot cursor. That heuristic was wrong by construction: a POP that padded its claim beyond the client-visible count (MAXDELIVERY > 0 over-claims for DLQ candidates), or a partial ACK of a multi-message claim, could not be reconstructed from a message count alone. apply_mq_ack now calls Stream::xack by id (idempotent — a replayed ack that was already applied, or an id that was never actually pending, is a harmless no-op). apply_mq_pop rebuilds the PEL entry for every claimed id with its recorded delivery_count, restores last_delivered_id verbatim, and replays DLQ routing by looking up each source id's field content from `stream.entries` (already rebuilt by every MqPush applied so far) before taking a mutable borrow of `stream.groups`, then re-pushing into the sibling DLQ stream at its original assigned id. apply_mq_create now creates the stream in the record's own db_index instead of hardcoding db 0. Every apply path is idempotent by construction (guarded by `contains_key`/`xack`'s no-op semantics), so replaying a partially-persisted segment or restarting twice is safe. Decode failures (malformed payload or an unsupported future version byte) are skip-and-warned via warn_skip_mq_record + tracing::warn!, tracked in a new MqReplayStats counter set logged once per shard — never abort the replay scan. Unit tests replace the old count-based-rollback test with: - test_replay_mq_wal_restores_full_lifecycle_by_id - test_replay_mq_wal_dlq_routing_restored - test_replay_mq_wal_trigger_registered - test_replay_mq_wal_survives_prior_db_clear (pins the ordering invariant: replay_mq_wal must run AFTER the AOF-authority db.clear() wipe, which was already true in main.rs's call order but had no regression test) - test_replay_mq_wal_unknown_version_skipped_not_fatal author: Tin Dang --- src/shard/shared_databases.rs | 850 +++++++++++++++++++++++++++------- 1 file changed, 691 insertions(+), 159 deletions(-) diff --git a/src/shard/shared_databases.rs b/src/shard/shared_databases.rs index 1e28e863..03da395f 100644 --- a/src/shard/shared_databases.rs +++ b/src/shard/shared_databases.rs @@ -411,28 +411,376 @@ pub fn replay_workspace_wal(shared: &Arc, persistence_dir: &std: } } -/// Replay MQ WAL records to restore DurableQueueRegistry and apply cursor-rollback. +/// Fixed consumer-group / consumer names used by every MQ.POP (mirrors +/// `src/shard/mq_exec.rs`'s hardcoded constants — MQ.* never exposes custom +/// group/consumer names, unlike the generic XREADGROUP API). +const MQ_GROUP_NAME: &[u8] = b"__mq_consumers"; +const MQ_CONSUMER_NAME: &[u8] = b"__mq_default"; + +/// Replay-time counters for `replay_mq_wal`, logged once per shard. +#[derive(Default)] +struct MqReplayStats { + create: u64, + push: u64, + pop: u64, + ack: u64, + trigger: u64, + skipped: u64, +} + +impl MqReplayStats { + fn total(&self) -> u64 { + self.create + self.push + self.pop + self.ack + self.trigger + } +} + +/// Log a skip-and-warn for a malformed or unsupported-version MQ WAL record. +/// Never aborts the replay scan — one bad record just doesn't get applied. +fn warn_skip_mq_record(shard_id: usize, kind: &str, payload: &[u8], stats: &mut MqReplayStats) { + stats.skipped += 1; + match crate::mq::wal::peek_version(payload) { + Some(v) if v > crate::mq::wal::MQ_WAL_VERSION => { + tracing::warn!( + "Shard {}: {} WAL record has unsupported version {} (this build \ + supports {}) — skipping, replay continues", + shard_id, + kind, + v, + crate::mq::wal::MQ_WAL_VERSION, + ); + } + _ => { + tracing::warn!( + "Shard {}: {} WAL record is malformed — skipping, replay continues", + shard_id, + kind, + ); + } + } +} + +/// Apply a single decoded MqCreate record: registers the shard-level durable +/// queue config AND (re)creates the stream in the record's OWN db (fixes the +/// pre-K2 db-0 hardcode — MqCreate now carries `db_index`). +fn apply_mq_create( + init: &mut crate::shard::slice::ShardSliceInit, + db_index: usize, + key: &[u8], + max_delivery_count: u32, +) { + let key_bytes = bytes::Bytes::copy_from_slice(key); + let reg = init + .durable_queue_registry + .get_or_insert_with(|| Box::new(crate::mq::DurableQueueRegistry::new())); + reg.insert( + key_bytes.clone(), + crate::mq::DurableStreamConfig::new(key_bytes, max_delivery_count), + ); + + if let Some(db) = init.databases.get_mut(db_index) { + if let Ok(stream) = db.get_or_create_stream(key) { + stream.durable = true; + stream.max_delivery_count = max_delivery_count; + let group_name = bytes::Bytes::from_static(MQ_GROUP_NAME); + // Idempotent: `create_group` errs BUSYGROUP if already created by + // an earlier replayed record (or a surviving RDB/rrdshard load); + // that's expected and fine, nothing else to do. + let _ = stream.create_group(group_name, crate::storage::stream::StreamId::ZERO); + } + } +} + +/// Apply a single decoded MqPush record: re-inserts the entry at its +/// ORIGINAL assigned id. Idempotent — replaying the same id twice (e.g. a +/// stream that already had this entry via a surviving RDB/rrdshard load) is +/// a harmless no-op rather than double-counting `length`. +fn apply_mq_push( + init: &mut crate::shard::slice::ShardSliceInit, + db_index: usize, + key: &[u8], + id: crate::storage::stream::StreamId, + fields: Vec<(bytes::Bytes, bytes::Bytes)>, +) { + if let Some(db) = init.databases.get_mut(db_index) { + if let Ok(stream) = db.get_or_create_stream(key) { + if !stream.entries.contains_key(&id) { + stream.add(id, fields); + } + } + } +} + +/// Apply a single decoded MqPop record: rebuilds the consumer group's PEL +/// entries for every claimed id, restores `last_delivered_id`, and replays +/// any DLQ routing (removing the source id from the PEL and re-pushing it +/// into the sibling DLQ stream at its ORIGINAL assigned id). Field content +/// for DLQ pushes is looked up from the main stream's `entries` map, which +/// by this point already reflects every MqPush record replayed so far +/// (records are applied strictly in WAL order). +fn apply_mq_pop( + init: &mut crate::shard::slice::ShardSliceInit, + db_index: usize, + key: &[u8], + last_delivered: crate::storage::stream::StreamId, + claimed: Vec<(crate::storage::stream::StreamId, u64)>, + dlq: Vec<( + crate::storage::stream::StreamId, + crate::storage::stream::StreamId, + )>, +) { + use crate::storage::stream::{Consumer, PendingEntry}; + + let Some(db) = init.databases.get_mut(db_index) else { + return; + }; + + let dlq_pushes: Vec<( + crate::storage::stream::StreamId, + Vec<(bytes::Bytes, bytes::Bytes)>, + )> = match db.get_stream_mut(key) { + Ok(Some(stream)) => { + // Look up DLQ field content FIRST (reads of `stream.entries`) + // before taking a mutable borrow of `stream.groups` below — + // keeps the borrow shape unambiguous rather than relying on + // interleaved disjoint-field access. + let dlq_fields: Vec> = dlq + .iter() + .map(|(src_id, _)| stream.entries.get(src_id).cloned().unwrap_or_default()) + .collect(); + + let group_name = bytes::Bytes::from_static(MQ_GROUP_NAME); + let consumer_name = bytes::Bytes::from_static(MQ_CONSUMER_NAME); + if !stream.groups.contains_key(group_name.as_ref()) { + // Should already exist from MqCreate replay; defensively + // create it so a truncated/out-of-order WAL can't panic. + let _ = + stream.create_group(group_name.clone(), crate::storage::stream::StreamId::ZERO); + } + + match stream.groups.get_mut(group_name.as_ref()) { + Some(group) => { + for (id, delivery_count) in &claimed { + group.pel.insert( + *id, + PendingEntry { + consumer: consumer_name.clone(), + delivery_time: 0, + delivery_count: *delivery_count, + }, + ); + let consumer = + group + .consumers + .entry(consumer_name.clone()) + .or_insert_with(|| Consumer { + name: consumer_name.clone(), + pending: std::collections::BTreeMap::new(), + seen_time: 0, + }); + consumer.pending.insert(*id, ()); + } + group.last_delivered_id = last_delivered; + + let mut collected = Vec::with_capacity(dlq.len()); + for ((src_id, dlq_id), fields) in dlq.iter().zip(dlq_fields) { + group.pel.remove(src_id); + if let Some(c) = group.consumers.get_mut(consumer_name.as_ref()) { + c.pending.remove(src_id); + } + collected.push((*dlq_id, fields)); + } + collected + } + None => Vec::new(), + } + } + _ => Vec::new(), + }; + + if !dlq_pushes.is_empty() { + let mut dlq_key = Vec::with_capacity(key.len() + 8); + dlq_key.extend_from_slice(key); + dlq_key.extend_from_slice(b"::mq:dlq"); + if let Ok(dlq_stream) = db.get_or_create_stream(&dlq_key) { + for (dlq_id, fields) in dlq_pushes { + if !dlq_stream.entries.contains_key(&dlq_id) { + dlq_stream.add(dlq_id, fields); + } + } + } + } +} + +/// Apply a single decoded MqAck record BY ID (not count) — the task #34 fix +/// for the old cursor-rollback-by-count heuristic. `Stream::xack` is +/// idempotent (no-ops if the id isn't in the PEL), so replaying the same ack +/// twice or acking an id that was never actually pending is harmless. +fn apply_mq_ack( + init: &mut crate::shard::slice::ShardSliceInit, + db_index: usize, + key: &[u8], + id: crate::storage::stream::StreamId, +) { + if let Some(db) = init.databases.get_mut(db_index) { + if let Ok(Some(stream)) = db.get_stream_mut(key) { + let group_name = bytes::Bytes::from_static(MQ_GROUP_NAME); + let _ = stream.xack(&group_name, &[id]); + } + } +} + +/// Apply a single decoded MqTrigger record: registers the trigger as opaque +/// data. Registration is durable/replayed but NEVER fired during replay — +/// firing only happens from live `MQ.PUSH` debounce arming +/// (`src/shard/timers.rs::fire_pending_mq_triggers`). +fn apply_mq_trigger( + init: &mut crate::shard::slice::ShardSliceInit, + trig_key: Vec, + queue_key: Vec, + callback_cmd: Vec, + debounce_ms: u64, +) { + let entry = crate::mq::TriggerEntry { + queue_key: bytes::Bytes::from(queue_key), + callback_cmd: bytes::Bytes::from(callback_cmd), + debounce_ms, + last_fire_ms: 0, + pending_fire_ms: 0, + }; + let reg = init + .trigger_registry + .get_or_insert_with(|| Box::new(crate::mq::TriggerRegistry::new())); + reg.register(bytes::Bytes::from(trig_key), entry); +} + +/// Decode + dispatch one MQ WAL record to its `apply_mq_*` handler, +/// skip-and-warn on any decode failure (malformed payload OR an +/// unsupported/future version byte). +fn apply_mq_wal_record( + init: &mut crate::shard::slice::ShardSliceInit, + shard_id: usize, + record_type: crate::persistence::wal_v3::record::WalRecordType, + payload: &[u8], + stats: &mut MqReplayStats, +) { + use crate::persistence::wal_v3::record::WalRecordType; + use crate::storage::stream::StreamId; + + match record_type { + WalRecordType::MqCreate => match crate::mq::wal::decode_mq_create(payload) { + Some((db_index, key, mdc)) => { + apply_mq_create(init, db_index as usize, &key, mdc); + stats.create += 1; + } + None => warn_skip_mq_record(shard_id, "MqCreate", payload, stats), + }, + WalRecordType::MqPush => match crate::mq::wal::decode_mq_push(payload) { + Some((db_index, key, ms, seq, fields)) => { + apply_mq_push(init, db_index as usize, &key, StreamId { ms, seq }, fields); + stats.push += 1; + } + None => warn_skip_mq_record(shard_id, "MqPush", payload, stats), + }, + WalRecordType::MqPop => match crate::mq::wal::decode_mq_pop(payload) { + Some((db_index, key, last_delivered, claimed, dlq)) => { + let claimed: Vec<(StreamId, u64)> = claimed + .into_iter() + .map(|(ms, seq, dc)| (StreamId { ms, seq }, dc)) + .collect(); + let dlq: Vec<(StreamId, StreamId)> = dlq + .into_iter() + .map(|(src_ms, src_seq, dlq_ms, dlq_seq)| { + ( + StreamId { + ms: src_ms, + seq: src_seq, + }, + StreamId { + ms: dlq_ms, + seq: dlq_seq, + }, + ) + }) + .collect(); + apply_mq_pop( + init, + db_index as usize, + &key, + StreamId { + ms: last_delivered.0, + seq: last_delivered.1, + }, + claimed, + dlq, + ); + stats.pop += 1; + } + None => warn_skip_mq_record(shard_id, "MqPop", payload, stats), + }, + WalRecordType::MqAck => match crate::mq::wal::decode_mq_ack(payload) { + Some((db_index, key, ms, seq)) => { + apply_mq_ack(init, db_index as usize, &key, StreamId { ms, seq }); + stats.ack += 1; + } + None => warn_skip_mq_record(shard_id, "MqAck", payload, stats), + }, + WalRecordType::MqTrigger => match crate::mq::wal::decode_mq_trigger(payload) { + Some((trig_key, queue_key, callback_cmd, debounce_ms)) => { + apply_mq_trigger(init, trig_key, queue_key, callback_cmd, debounce_ms); + stats.trigger += 1; + } + None => warn_skip_mq_record(shard_id, "MqTrigger", payload, stats), + }, + _ => {} + } +} + +/// Replay MQ WAL records to restore full durable-queue state: registry +/// config, stream content, consumer-group PEL/cursor, DLQ routing, and +/// trigger registrations (task #34 / Wave B stage 2a). /// /// Operates on `&mut [ShardSliceInit]` — called single-threaded at boot /// before shard threads are spawned. No locks needed. /// +/// # Ordering requirement +/// +/// MUST run AFTER any AOF-authority `db.clear()` wipe. `main.rs` calls this +/// near the end of the boot sequence, well after the AOF-authority +/// wipe-then-replay block — see `tests::test_replay_mq_wal_survives_prior_db_clear` +/// for a direct regression proof, and +/// `tests/crash_recovery_mq_effects.rs::mq_effect_records_survive_kill9` for +/// the live kill-9 round trip. MQ.* commands are intercepted before the +/// generic AOF-logging dispatch path, so under `--appendonly yes` the wipe +/// discards every durable Stream unconditionally; only this replay's own +/// effect-record log rebuilds it. +/// +/// # Ordering WITHIN the log +/// +/// Records are applied ONE AT A TIME, in WAL order (LSN order, preserved by +/// `replay_wal_v3_file`'s sequential per-file scan + the caller's +/// lexicographically sorted file list) — NOT collected into a map and +/// bulk-applied at the end. PEL membership and the consumer group's +/// `last_delivered_id` are only correct if interleaved MqPush/MqPop/MqAck +/// records are replayed in their original relative order (e.g. a POP that +/// claims ids 1-3, an ACK of id 1, then a LATER POP that claims ids 4-5 must +/// leave the PEL at `{2, 3, 4, 5}` with `last_delivered_id = 5` — batching +/// "all pops then all acks" would get this wrong whenever an id is +/// claimed-then-acked-then-a-later-pop-claims-more). +/// +/// # WAL v3 framing +/// /// WAL v3 segments for a shard live in `shard-{id}/wal-v3/` (matching /// `replay_workspace_wal` / `replay_graph_wal` / `recovery.rs`) — NOT /// directly under `shard-{id}/`, which `std::fs::read_dir` (non-recursive) -/// would silently scan as empty. Every cross-thread `wal_append` blob is -/// also re-wrapped as `WalRecordType::Command` by the shard event-loop -/// drain (`event_loop.rs`), so `MqCreate`/`MqAck` records are nested inside -/// a Command payload on disk; `handle_record` below is shared by both the -/// direct-type arm (for any legacy/self-written records) and the -/// Command-unwrap arm, mirroring `replay_workspace_wal`. +/// would silently scan as empty (task #42). Every cross-thread `wal_append` +/// blob SHOULD arrive with its real type directly post-K1a (no `Command` +/// wrapper) — the nested-unwrap arm below is defensive-only, covering any +/// stray pre-K1a segment still on disk. pub fn replay_mq_wal( inits: &mut [crate::shard::slice::ShardSliceInit], persistence_dir: &std::path::Path, ) { - use std::collections::HashMap; - use crate::persistence::wal_v3::record::{WalRecord, WalRecordType, read_wal_v3_record}; - use crate::storage::stream::StreamId; for init in inits.iter_mut() { let shard_id = init.shard_id; @@ -443,120 +791,66 @@ pub fn replay_mq_wal( continue; } - let mut durable_configs: HashMap, u32> = HashMap::new(); - let mut ack_count = 0u64; + let mut stats = MqReplayStats::default(); - let mut handle_record = |record_type: WalRecordType, payload: &[u8]| match record_type { - WalRecordType::MqCreate => { - if let Some((queue_key, max_delivery_count)) = - crate::mq::wal::decode_mq_create(payload) - { - durable_configs.insert(queue_key, max_delivery_count); - } - } - WalRecordType::MqAck => { - if crate::mq::wal::decode_mq_ack(payload).is_some() { - ack_count += 1; + { + let mut handle_record = |record_type: WalRecordType, payload: &[u8]| { + apply_mq_wal_record(init, shard_id, record_type, payload, &mut stats); + }; + + let on_command = &mut |record: &WalRecord| match record.record_type { + WalRecordType::MqCreate + | WalRecordType::MqAck + | WalRecordType::MqPush + | WalRecordType::MqPop + | WalRecordType::MqTrigger => { + handle_record(record.record_type, &record.payload); } - } - _ => {} - }; - - let on_command = &mut |record: &WalRecord| match record.record_type { - WalRecordType::MqCreate | WalRecordType::MqAck => { - handle_record(record.record_type, &record.payload); - } - WalRecordType::Command => { - if let Some(inner) = read_wal_v3_record(&record.payload) { - match inner.record_type { - WalRecordType::MqCreate | WalRecordType::MqAck => { - handle_record(inner.record_type, &inner.payload); + WalRecordType::Command => { + if let Some(inner) = read_wal_v3_record(&record.payload) { + match inner.record_type { + WalRecordType::MqCreate + | WalRecordType::MqAck + | WalRecordType::MqPush + | WalRecordType::MqPop + | WalRecordType::MqTrigger => { + handle_record(inner.record_type, &inner.payload); + } + _ => {} } - _ => {} } } - } - _ => {} - }; - let on_fpi = &mut |_: &WalRecord| {}; - - if let Ok(entries) = std::fs::read_dir(&wal_dir) { - let mut wal_files: Vec<_> = entries - .filter_map(|e| e.ok()) - .filter(|e| e.file_name().to_str().is_some_and(|n| n.ends_with(".wal"))) - .map(|e| e.path()) - .collect(); - wal_files.sort(); - - for wal_file in &wal_files { - let _ = crate::persistence::wal_v3::replay::replay_wal_v3_file( - wal_file, 0, on_command, on_fpi, - ); - } - } - - if !durable_configs.is_empty() { - let reg = init - .durable_queue_registry - .get_or_insert_with(|| Box::new(crate::mq::DurableQueueRegistry::new())); - for (queue_key_bytes, max_delivery_count) in &durable_configs { - let key = bytes::Bytes::copy_from_slice(queue_key_bytes); - let config = crate::mq::DurableStreamConfig::new(key.clone(), *max_delivery_count); - reg.insert(key, config); - } - } + _ => {} + }; + let on_fpi = &mut |_: &WalRecord| {}; + + if let Ok(entries) = std::fs::read_dir(&wal_dir) { + let mut wal_files: Vec<_> = entries + .filter_map(|e| e.ok()) + .filter(|e| e.file_name().to_str().is_some_and(|n| n.ends_with(".wal"))) + .map(|e| e.path()) + .collect(); + wal_files.sort(); - // Cursor-rollback for each durable queue using db 0. - for (queue_key_bytes, max_dc) in &durable_configs { - let key_bytes = bytes::Bytes::copy_from_slice(queue_key_bytes); - // db 0 is the first database in the slice. - if let Some(db) = init.databases.get_mut(0) { - if let Ok(Some(stream)) = db.get_stream_mut(&key_bytes) { - stream.durable = true; - stream.max_delivery_count = *max_dc; - - let group_name = bytes::Bytes::from_static(b"__mq_consumers"); - if let Some(group) = stream.groups.get_mut(&group_name) { - if let Some((min_pel_id, _)) = group.pel.iter().next() { - let rollback_target = if min_pel_id.seq > 0 { - StreamId { - ms: min_pel_id.ms, - seq: min_pel_id.seq - 1, - } - } else if min_pel_id.ms > 0 { - StreamId { - ms: min_pel_id.ms - 1, - seq: u64::MAX, - } - } else { - StreamId::ZERO - }; - - tracing::info!( - "Shard {}: MQ cursor-rollback for queue {:?}: \ - last_delivered_id {}-{} -> {}-{} (PEL size: {})", - shard_id, - String::from_utf8_lossy(queue_key_bytes), - group.last_delivered_id.ms, - group.last_delivered_id.seq, - rollback_target.ms, - rollback_target.seq, - group.pel.len(), - ); - - group.last_delivered_id = rollback_target; - } - } + for wal_file in &wal_files { + let _ = crate::persistence::wal_v3::replay::replay_wal_v3_file( + wal_file, 0, on_command, on_fpi, + ); } } } - if !durable_configs.is_empty() { + if stats.total() > 0 || stats.skipped > 0 { tracing::info!( - "Shard {}: replayed {} MQ queue configs, {} ack records", + "Shard {}: replayed MQ WAL — {} create, {} push, {} pop, {} ack, \ + {} trigger record(s), {} skipped (malformed/unsupported version)", shard_id, - durable_configs.len(), - ack_count, + stats.create, + stats.push, + stats.pop, + stats.ack, + stats.trigger, + stats.skipped, ); } } @@ -1151,86 +1445,324 @@ mod tests { writer.flush_sync().expect("flush wal segment"); } + // ── Wave B stage 2a (task #34): full-lifecycle effect-record replay ──── + // + // Supersedes the old `test_replay_mq_wal_restores_registry_and_rolls_back_cursor`, + // which asserted the PRE-task-#34 "blind cursor-rollback by PEL-count" + // heuristic (roll back to just before the min PEL id, regardless of + // which ids were actually acked). That heuristic is gone: MqAck now + // replays BY ID via `Stream::xack`, and MqPush/MqPop give replay the + // actual content + claim/DLQ outcomes needed to reconstruct state + // exactly, without depending on whatever partial content a `.rrdshard` + // snapshot happened to preserve. + #[test] - fn test_replay_mq_wal_restores_registry_and_rolls_back_cursor() { - use crate::mq::wal::{encode_mq_ack, encode_mq_create}; + fn test_replay_mq_wal_restores_full_lifecycle_by_id() { + use crate::mq::wal::{encode_mq_ack, encode_mq_create, encode_mq_pop, encode_mq_push}; use crate::persistence::wal_v3::record::WalRecordType; - use crate::storage::stream::{PendingEntry, StreamId}; + use crate::storage::stream::StreamId; let tmp = tempfile::tempdir().expect("tempdir"); let queue_key = b"orders".to_vec(); let group_name = bytes::Bytes::from_static(b"__mq_consumers"); + // Simulate the AOF-authority wipe: db starts completely empty (no + // .rrdshard content survived — matches `--appendonly yes` reality). let dbs = vec![vec![Database::new()]]; let (_shared, mut inits) = ShardDatabases::new(dbs); - { - let db = inits[0].databases.get_mut(0).expect("db 0"); - let stream = db.get_or_create_stream(&queue_key).expect("create stream"); - stream - .create_group(group_name.clone(), StreamId::ZERO) - .expect("create group"); - assert!( - !stream.durable, - "sanity: a .rrdshard-restored stream starts non-durable \ - (rdb.rs's TYPE_STREAM body has no durable/max_delivery_count bytes)" - ); - let group = stream.groups.get_mut(&group_name).expect("group"); - // Simulate a crash mid-delivery: one message claimed but not yet - // acked survived (via the KV plane) in the PEL. - group.last_delivered_id = StreamId { ms: 100, seq: 5 }; - group.pel.insert( - StreamId { ms: 100, seq: 5 }, - PendingEntry { - consumer: bytes::Bytes::from_static(b"__mq_default"), - delivery_time: 0, - delivery_count: 1, - }, - ); - } - // Real WAL v3 bytes, nested exactly as the shard event-loop drain - // produces them (bug (b): a naive `record.record_type` match never - // sees these, since the outer type is always Command). + let f = |v: &[u8]| { + ( + bytes::Bytes::from_static(b"seq"), + bytes::Bytes::copy_from_slice(v), + ) + }; write_mq_wal_records( tmp.path(), 0, &[ - (WalRecordType::MqCreate, encode_mq_create(&queue_key, 7)), - (WalRecordType::MqAck, encode_mq_ack(&queue_key, 100, 5)), + (WalRecordType::MqCreate, encode_mq_create(0, &queue_key, 5)), + ( + WalRecordType::MqPush, + encode_mq_push(0, &queue_key, 1, 0, &[f(b"1")]), + ), + ( + WalRecordType::MqPush, + encode_mq_push(0, &queue_key, 1, 1, &[f(b"2")]), + ), + ( + WalRecordType::MqPush, + encode_mq_push(0, &queue_key, 1, 2, &[f(b"3")]), + ), + // POP claims ids (1,0) and (1,1) with delivery_count=1; + // resulting last_delivered_id = (1,1); no DLQ routing. + ( + WalRecordType::MqPop, + encode_mq_pop(0, &queue_key, (1, 1), &[(1, 0, 1), (1, 1, 1)], &[]), + ), + // ACK only (1,0) — (1,1) must remain pending. + (WalRecordType::MqAck, encode_mq_ack(0, &queue_key, 1, 0)), ], ); - // Written at `/shard-0/wal-v3/...` (bug (a): the pre-fix - // function scanned `/shard-0/` directly, a non-recursive - // `read_dir` over an otherwise-empty directory). replay_mq_wal(&mut inits, tmp.path()); let reg = inits[0] .durable_queue_registry .as_ref() .expect("MqCreate record must populate durable_queue_registry"); - let config = reg - .get(&queue_key) - .expect("queue must be registered after replay"); - assert_eq!(config.max_delivery_count, 7); + assert_eq!( + reg.get(&queue_key).expect("registered").max_delivery_count, + 5 + ); let db = inits[0].databases.get_mut(0).expect("db 0"); let stream = db .get_stream_mut(&queue_key) .expect("get_stream_mut") - .expect("stream must still exist"); - assert!( - stream.durable, - "replay must restore durable=true from the MqCreate WAL record" + .expect("stream must exist — rebuilt entirely from the WAL"); + assert!(stream.durable, "MqCreate replay must restore durable=true"); + assert_eq!(stream.max_delivery_count, 5); + assert_eq!( + stream.entries.len(), + 3, + "all 3 MqPush records must rebuild stream content from nothing" + ); + assert_eq!( + stream.entries.get(&StreamId { ms: 1, seq: 2 }), + Some(&vec![f(b"3")]), + "field content must match exactly, including the never-popped 3rd entry" ); - assert_eq!(stream.max_delivery_count, 7); let group = stream.groups.get(&group_name).expect("group"); assert_eq!( group.last_delivered_id, - StreamId { ms: 100, seq: 4 }, - "cursor-rollback must rewind last_delivered_id to just before the \ - sole PEL entry (ms=100,seq=5) so MQ.POP redelivers it" + StreamId { ms: 1, seq: 1 }, + "cursor must land exactly where the MqPop record said, not a \ + count-derived heuristic" + ); + assert_eq!( + group.pel.keys().collect::>(), + vec![&StreamId { ms: 1, seq: 1 }], + "only the un-acked id (1,1) may remain pending -- ack-by-id must \ + NOT roll back (1,0) too" + ); + } + + #[test] + fn test_replay_mq_wal_dlq_routing_restored() { + use crate::mq::wal::{encode_mq_create, encode_mq_pop, encode_mq_push}; + use crate::persistence::wal_v3::record::WalRecordType; + + let tmp = tempfile::tempdir().expect("tempdir"); + let queue_key = b"dlqtestq".to_vec(); + + let dbs = vec![vec![Database::new()]]; + let (_shared, mut inits) = ShardDatabases::new(dbs); + + let f = |v: &[u8]| { + ( + bytes::Bytes::from_static(b"f"), + bytes::Bytes::copy_from_slice(v), + ) + }; + write_mq_wal_records( + tmp.path(), + 0, + &[ + (WalRecordType::MqCreate, encode_mq_create(0, &queue_key, 1)), + ( + WalRecordType::MqPush, + encode_mq_push(0, &queue_key, 1, 0, &[f(b"a")]), + ), + // MAXDELIVERY 1: the claim immediately routes to DLQ, + // assigned id (2,0) in the sibling DLQ stream. + ( + WalRecordType::MqPop, + encode_mq_pop(0, &queue_key, (1, 0), &[(1, 0, 1)], &[(1, 0, 2, 0)]), + ), + ], + ); + + replay_mq_wal(&mut inits, tmp.path()); + + let mut dlq_key = queue_key.clone(); + dlq_key.extend_from_slice(b"::mq:dlq"); + let db = inits[0].databases.get_mut(0).expect("db 0"); + let dlq_stream = db + .get_stream_mut(&dlq_key) + .expect("get_stream_mut") + .expect("DLQ stream must exist after replay"); + assert_eq!(dlq_stream.entries.len(), 1); + assert_eq!( + dlq_stream + .entries + .get(&crate::storage::stream::StreamId { ms: 2, seq: 0 }), + Some(&vec![f(b"a")]), + "DLQ entry must land at its ORIGINAL assigned id with original fields" + ); + + let main_stream = db + .get_stream_mut(&queue_key) + .expect("get_stream_mut") + .expect("main stream must exist"); + let group = main_stream + .groups + .get(bytes::Bytes::from_static(b"__mq_consumers").as_ref()) + .expect("group"); + assert!( + group.pel.is_empty(), + "DLQ-routed entry must NOT remain pending in the main PEL" + ); + } + + #[test] + fn test_replay_mq_wal_trigger_registered() { + use crate::mq::wal::encode_mq_trigger; + use crate::persistence::wal_v3::record::WalRecordType; + + let tmp = tempfile::tempdir().expect("tempdir"); + let dbs = vec![vec![Database::new()]]; + let (_shared, mut inits) = ShardDatabases::new(dbs); + + write_mq_wal_records( + tmp.path(), + 0, + &[( + WalRecordType::MqTrigger, + encode_mq_trigger(b"trigq", b"trigq", b"FIRED", 200), + )], + ); + + replay_mq_wal(&mut inits, tmp.path()); + + let reg = inits[0] + .trigger_registry + .as_ref() + .expect("MqTrigger record must populate trigger_registry"); + let entry = reg.get(b"trigq").expect("trigger must be registered"); + assert_eq!(entry.queue_key.as_ref(), b"trigq"); + assert_eq!(entry.callback_cmd.as_ref(), b"FIRED"); + assert_eq!(entry.debounce_ms, 200); + assert_eq!( + entry.pending_fire_ms, 0, + "replay must never arm a pending fire -- only live MQ.PUSH does" + ); + } + + #[test] + fn test_replay_mq_wal_survives_prior_db_clear() { + // Direct regression proof for the ordering requirement documented on + // `replay_mq_wal`: it must be safe to call AFTER an AOF-authority + // `db.clear()` wipe (main.rs's ordering), because that's exactly the + // scenario `--appendonly yes` produces in production. Simulates the + // wipe explicitly rather than depending on main.rs's call order. + use crate::mq::wal::{encode_mq_create, encode_mq_push}; + use crate::persistence::wal_v3::record::WalRecordType; + + let tmp = tempfile::tempdir().expect("tempdir"); + let queue_key = b"survives".to_vec(); + + let dbs = vec![vec![Database::new()]]; + let (_shared, mut inits) = ShardDatabases::new(dbs); + + // Pre-populate as if content existed before the "crash" (e.g. from + // an earlier boot in the same process) — then wipe it exactly like + // main.rs's AOF-authority path does. + { + let db = inits[0].databases.get_mut(0).expect("db 0"); + let _ = db.get_or_create_stream(&queue_key); + db.clear(); + assert!( + db.get_stream_mut(&queue_key).unwrap().is_none(), + "sanity: db.clear() must actually wipe the stream" + ); + } + + write_mq_wal_records( + tmp.path(), + 0, + &[ + (WalRecordType::MqCreate, encode_mq_create(0, &queue_key, 3)), + ( + WalRecordType::MqPush, + encode_mq_push( + 0, + &queue_key, + 1, + 0, + &[( + bytes::Bytes::from_static(b"f"), + bytes::Bytes::from_static(b"v"), + )], + ), + ), + ], + ); + + replay_mq_wal(&mut inits, tmp.path()); + + let db = inits[0].databases.get_mut(0).expect("db 0"); + let stream = db + .get_stream_mut(&queue_key) + .expect("get_stream_mut") + .expect( + "RED: replay_mq_wal must rebuild the stream from its own \ + effect-record log even after an earlier db.clear() wipe", + ); + assert!(stream.durable); + assert_eq!(stream.entries.len(), 1); + } + + #[test] + fn test_replay_mq_wal_unknown_version_skipped_not_fatal() { + // A corrupted/future-version MqCreate record must be skipped (not + // panic, not abort the rest of the scan) — the well-formed MqPush + // record right after it must still apply. + use crate::mq::wal::encode_mq_push; + use crate::persistence::wal_v3::record::WalRecordType; + + let tmp = tempfile::tempdir().expect("tempdir"); + let queue_key = b"q".to_vec(); + let dbs = vec![vec![Database::new()]]; + let (_shared, mut inits) = ShardDatabases::new(dbs); + + let mut bogus_create = crate::mq::wal::encode_mq_create(0, &queue_key, 1); + bogus_create[0] = 200; // unsupported future version + + write_mq_wal_records( + tmp.path(), + 0, + &[ + (WalRecordType::MqCreate, bogus_create), + ( + WalRecordType::MqPush, + encode_mq_push( + 0, + &queue_key, + 1, + 0, + &[( + bytes::Bytes::from_static(b"f"), + bytes::Bytes::from_static(b"v"), + )], + ), + ), + ], + ); + + // Must not panic. + replay_mq_wal(&mut inits, tmp.path()); + + assert!( + inits[0].durable_queue_registry.is_none(), + "the malformed MqCreate must be skipped, not applied" + ); + let db = inits[0].databases.get_mut(0).expect("db 0"); + assert_eq!( + db.get_or_create_stream(&queue_key).unwrap().entries.len(), + 1, + "a well-formed record after a skipped malformed one must still apply" ); } From 227adaf492775d7334384bdc6959c2607b88ff38 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 10:12:23 +0700 Subject: [PATCH 4/7] feat(mq): emit MqPush WAL records at MQ.PUBLISH TXN materialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MQ.PUBLISH materializes its queued intents at TXN commit time on two separate legs — the self-fold path (the committing connection's own shard owns the queue) and the foreign-shard path (a ShardMessage::MqCommand hop to the owning shard) — each duplicated across the monoio and tokio connection handlers. Neither leg emitted a WAL record for the resulting stream entry, so a durable queue populated exclusively via MQ.PUBLISH inside a MULTI/EXEC block had no durability at all, independent of the mq_exec.rs handle_push fix (previous commits) which only covers the direct MQ.PUSH command path. Fixes all four call sites: - src/server/conn/handler_monoio/txn.rs (self-fold leg) - src/server/conn/handler_sharded/txn.rs (tokio self-fold leg) - src/shard/spsc_handler.rs (MqTxnMaterialize handler, shared by both runtimes' foreign-shard leg) Each site builds the MqPush payload inside the same with_shard_db closure that assigns the message id and calls `stream.add`, collects the payloads, then emits them via mq_exec::wal_append_on_slice after the closure returns — same pattern as handle_push in mq_exec.rs. author: Tin Dang --- src/server/conn/handler_monoio/txn.rs | 43 ++++++++++++++++++++------ src/server/conn/handler_sharded/txn.rs | 41 ++++++++++++++++++------ src/shard/spsc_handler.rs | 23 +++++++++++++- 3 files changed, 88 insertions(+), 19 deletions(-) diff --git a/src/server/conn/handler_monoio/txn.rs b/src/server/conn/handler_monoio/txn.rs index df069e59..620dd9cf 100644 --- a/src/server/conn/handler_monoio/txn.rs +++ b/src/server/conn/handler_monoio/txn.rs @@ -137,17 +137,42 @@ pub(super) async fn try_handle_txn_commit( let mut mq_lost: Option<(usize, usize)> = None; // (shard, intents) for (owner, intents) in by_shard { if owner == ctx.shard_id { - // Self: apply locally via slice. - crate::shard::slice::with_shard_db(conn.selected_db as usize, |db| { - for intent in &intents { - if let Ok(Some(stream)) = db.get_stream_mut(&intent.queue_key) { - if stream.durable { - let msg_id = stream.next_auto_id(); - stream.add(msg_id, intent.fields.clone()); + // Self: apply locally via slice. Collect WAL + // payloads inside the closure (encoding is a pure + // function call, not a nested `with_shard*` call) + // and emit them AFTER the closure returns — each + // MqPush record captures the ASSIGNED id so + // replay is outcome-deterministic (Wave B stage + // 2a: MQ.PUBLISH materialization durability). + let db_index = conn.selected_db; + let payloads: Vec> = + crate::shard::slice::with_shard_db(db_index, |db| { + let mut payloads = Vec::with_capacity(intents.len()); + for intent in &intents { + if let Ok(Some(stream)) = + db.get_stream_mut(&intent.queue_key) + { + if stream.durable { + let msg_id = stream.next_auto_id(); + payloads.push(crate::mq::wal::encode_mq_push( + db_index as u32, + &intent.queue_key, + msg_id.ms, + msg_id.seq, + &intent.fields, + )); + stream.add(msg_id, intent.fields.clone()); + } } } - } - }); + payloads + }); + for payload in payloads { + crate::shard::mq_exec::wal_append_on_slice( + crate::persistence::wal_v3::record::WalRecordType::MqPush, + Bytes::from(payload), + ); + } } else { // Foreign: send MqTxnMaterialize hop and await ack. let intent_count = intents.len(); diff --git a/src/server/conn/handler_sharded/txn.rs b/src/server/conn/handler_sharded/txn.rs index 39ddfbf8..c04ee9e1 100644 --- a/src/server/conn/handler_sharded/txn.rs +++ b/src/server/conn/handler_sharded/txn.rs @@ -145,18 +145,41 @@ pub(super) async fn try_handle_txn_commit( foreign.entry(owner).or_default().push(intent); } } - // Apply self-shard intents synchronously (no borrow across .await). + // Apply self-shard intents synchronously (no borrow + // across .await). Collect WAL payloads inside the + // closure (encoding is a pure function call, not a + // nested `with_shard*` call) and emit them AFTER the + // closure returns — each MqPush record captures the + // ASSIGNED id so replay is outcome-deterministic (Wave B + // stage 2a: MQ.PUBLISH materialization durability). if !self_intents.is_empty() { - crate::shard::slice::with_shard_db(conn.selected_db, |db| { - for intent in &self_intents { - if let Ok(Some(stream)) = db.get_stream_mut(&intent.queue_key) { - if stream.durable { - let msg_id = stream.next_auto_id(); - stream.add(msg_id, intent.fields.clone()); + let db_index = conn.selected_db; + let payloads: Vec> = + crate::shard::slice::with_shard_db(db_index, |db| { + let mut payloads = Vec::with_capacity(self_intents.len()); + for intent in &self_intents { + if let Ok(Some(stream)) = db.get_stream_mut(&intent.queue_key) { + if stream.durable { + let msg_id = stream.next_auto_id(); + payloads.push(crate::mq::wal::encode_mq_push( + db_index as u32, + &intent.queue_key, + msg_id.ms, + msg_id.seq, + &intent.fields, + )); + stream.add(msg_id, intent.fields.clone()); + } } } - } - }); + payloads + }); + for payload in payloads { + crate::shard::mq_exec::wal_append_on_slice( + crate::persistence::wal_v3::record::WalRecordType::MqPush, + Bytes::from(payload), + ); + } } // Send MqTxnMaterialize to each foreign shard and await all // acks. The commit is already WAL-durable at this point, so diff --git a/src/shard/spsc_handler.rs b/src/shard/spsc_handler.rs index 9baac835..dcbb0630 100644 --- a/src/shard/spsc_handler.rs +++ b/src/shard/spsc_handler.rs @@ -2505,16 +2505,37 @@ pub(crate) fn handle_shard_message_shared( // TXN.COMMIT MQ-intent materialize: fold deferred MQ.PUBLISH messages // onto the owner shard. Mirrors txn.rs:160–167 exactly: // for intent in intents: get_stream_mut → durable-check → add. - crate::shard::slice::with_shard_db(db_index, |db| { + // Wave B stage 2a: also emits one MqPush WAL record per applied + // intent (durability plane only — this is the FOREIGN-shard leg + // of MQ.PUBLISH materialization; the self-shard leg is handled + // directly in txn.rs). Payloads are collected inside the + // closure (encoding is a pure function call) and emitted after + // it returns. + let payloads: Vec> = crate::shard::slice::with_shard_db(db_index, |db| { + let mut payloads = Vec::with_capacity(intents.len()); for intent in &intents { if let Ok(Some(stream)) = db.get_stream_mut(&intent.queue_key) { if stream.durable { let msg_id = stream.next_auto_id(); + payloads.push(crate::mq::wal::encode_mq_push( + db_index as u32, + &intent.queue_key, + msg_id.ms, + msg_id.seq, + &intent.fields, + )); stream.add(msg_id, intent.fields.clone()); } } } + payloads }); + for payload in payloads { + crate::shard::mq_exec::wal_append_on_slice( + crate::persistence::wal_v3::record::WalRecordType::MqPush, + bytes::Bytes::from(payload), + ); + } // Ignore send failure: receiver dropped means the TXN coordinator // has already given up (e.g. client disconnect mid-commit). let _ = reply_tx.send(()); From 66d974c2b161c9612b8c4a27ffbccc767ef6d91b Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 10:12:31 +0700 Subject: [PATCH 5/7] test(mq): kill-9 crash test + fuzz target for MQ effect record durability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests/crash_recovery_mq_effects.rs::mq_effect_records_survive_kill9, the RED/GREEN proof for the whole Wave B stage 2a change: under `--appendonly yes`, exercises CREATE -> PUSH x5 -> POP(3) -> ACK(2 of 3) on one queue, a second CREATE/PUSH x2/POP(2) pair on another queue that forces immediate DLQ routing (MAXDELIVERY 1), and a TRIGGER registration on a third queue; kills -9 the server; restarts on the same --dir; and asserts stream content (XLEN/XRANGE field values), the consumer-group delivery cursor (POP resumes at msg 4, not a re-delivery of msg 1..3), the PEL restored by id (XPENDING reports exactly msg 3 pending, not all 3 claimed messages), DLQ routing (MQ DLQLEN), and trigger re-arming (a fresh PUSH notifies mq:trigger: with the original callback payload) all survive. Pre-fix, every one of these assertions fails: the AOF-authority db.clear() wipes the keyspace on restart and nothing replayed it back. Measured WAL footprint (release binary, single shard, --appendonly yes): CREATE + 5xPUSH + POP(3) = 7 effect records total 576 bytes on disk (64-byte v3 segment header + records), average ~82 bytes/record — 68 bytes per PUSH (48-byte payload + 20-byte v3 framing/CRC overhead), ~132 bytes for a 3-claim POP (112-byte payload + framing). Adds fuzz/fuzz_targets/mq_wal_record.rs (registered in fuzz/Cargo.toml and both fuzz-pr/fuzz-nightly matrices in .github/workflows/fuzz.yml), fuzzing all five MQ WAL decoders directly against attacker/corruption-controlled bytes — every decoder is contracted to return None on malformed or unsupported-version input, never panic or read out of bounds. Updates tests/crash_recovery_temporal_mq.rs's module doc comment (which previously explained why a live MQ round trip was NOT possible) to point at the new test now that the gap is closed, and fixes two stale references to a unit test name that this change's replay rewrite superseded. CHANGELOG.md [Unreleased] entry added per the Lint gate's requirement. author: Tin Dang --- .github/workflows/fuzz.yml | 2 + CHANGELOG.md | 44 +++ fuzz/Cargo.lock | 195 +++++++---- fuzz/Cargo.toml | 5 + fuzz/fuzz_targets/mq_wal_record.rs | 26 ++ tests/crash_recovery_mq_effects.rs | 489 ++++++++++++++++++++++++++++ tests/crash_recovery_temporal_mq.rs | 38 ++- 7 files changed, 730 insertions(+), 69 deletions(-) create mode 100644 fuzz/fuzz_targets/mq_wal_record.rs create mode 100644 tests/crash_recovery_mq_effects.rs diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 29318da8..e1c7a3a5 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -35,6 +35,7 @@ jobs: - csr_from_bytes - graph_props_record - fts_query_parse + - mq_wal_record steps: - uses: actions/checkout@v7 - uses: dtolnay/rust-toolchain@nightly @@ -86,6 +87,7 @@ jobs: - csr_from_bytes - graph_props_record - fts_query_parse + - mq_wal_record steps: - uses: actions/checkout@v7 - uses: dtolnay/rust-toolchain@nightly diff --git a/CHANGELOG.md b/CHANGELOG.md index 444599d2..78fd8247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,50 @@ linked repo-root files (`BENCHMARK.md`, `RELEASES.md`, the GitHub Pages deploy had failed on every main push since 2026-07-08. Converted the 5 links to absolute GitHub blob URLs; strict build verified clean locally. Docs-only. +### Fixed — MQ effect records now survive kill-9 (Wave B stage 2a, task #34) + +`MQ.CREATE/PUSH/POP/ACK/TRIGGER` intercept before the generic AOF-logging +dispatch path (they route via `execute_mq_on_owner` in +`src/shard/mq_exec.rs`), so under `--appendonly yes` the AOF-authority +recovery (`db.clear()` on every shard, rebuild solely from the AOF manifest) +silently discarded every durable `Stream`, consumer-group PEL, DLQ routing +decision, and trigger registration on every restart — regardless of whether +`replay_mq_wal` itself was correct. Two additional pre-existing defects in +`replay_mq_wal` made it dangerous even when reached: `MqAck` records were +applied by COUNT (rolling the whole PEL back to a snapshot cursor) instead +of by ID, and every MQ payload hardcoded db index 0. + +Fixed by giving each MQ mutation its own versioned WAL v3 effect record, +emitted at the owner-shard execution site: `MqPush` (0x72), `MqPop` (0x73), +and `MqTrigger` (0x74) are new discriminants; `MqCreate` (0x70) and `MqAck` +(0x71) keep their existing discriminants but move to a versioned, +db-index-carrying payload (precedent: the `XactCommit` 0x51→0x53 format +freeze — WAL v3 segments are short-lived with no cross-version contract). +Every decoder in the new `src/mq/wal.rs` module returns `None` on a +malformed OR unsupported-version payload; `replay_mq_wal` skip-and-warns +(`tracing::warn!`) rather than aborting the scan. `MqPop` now carries the +full claim set (id + delivery_count per claimed message) plus any DLQ +routing decisions (source id → assigned DLQ id), so replay reconstructs the +consumer group's PEL and `last_delivered_id` exactly instead of guessing. +`MqAck` now applies via `Stream::xack` by id — idempotent, and immune to +the old count-based rollback bug. `MQ.PUBLISH`'s TXN materialization hop +also emits `MqPush` records, at both the self-fold and foreign-shard legs, +on both the monoio and tokio connection handlers. + +Trigger registrations are durable/replayed as opaque data — replay never +*fires* a trigger; only a live `MQ.PUSH`'s debounce arming does +(`src/shard/timers.rs::fire_pending_mq_triggers`). + +New RED/GREEN kill-9 crash test: `tests/crash_recovery_mq_effects.rs` +(`--ignored`, needs a built binary) exercises the full lifecycle — +CREATE → PUSH×5 → POP(3) → ACK(2 of 3) → a second CREATE/PUSH/POP pair that +forces immediate DLQ routing → a TRIGGER registration — kill -9, restart on +the same `--dir`, and asserts stream content, delivery cursor, PEL-by-id, +DLQ routing, and trigger re-arming all survive. New fuzz target +`mq_wal_record` covers the five new op-blob decoders (`fuzz/fuzz_targets/`, +registered in both `fuzz-pr` and `fuzz-nightly` CI matrices). + +Out of scope (stage 2b+): replication emission/apply for the MQ plane. ### Changed — WAL v3 `wal_append` channel now preserves the caller's REAL record type end-to-end (K1a, storage-kernel M1 stage 1) diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 899ac5d8..165b94a7 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -2,18 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "ahash" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" -dependencies = [ - "cfg-if", - "once_cell", - "version_check", - "zerocopy", -] - [[package]] name = "aho-corasick" version = "1.1.4" @@ -144,6 +132,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "bitflags" version = "2.11.0" @@ -430,7 +424,7 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e0e367e4e7da84520dedcac1901e4da967309406d1e51017ae1abfb97adbd38" dependencies = [ - "bitflags", + "bitflags 2.11.0", "block2", "libc", "objc2", @@ -464,6 +458,17 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "evmap" +version = "11.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b8874945f036109c72242964c1174cf99434e30cfa45bf45fedc983f50046f8" +dependencies = [ + "hashbag", + "left-right", + "smallvec", +] + [[package]] name = "fastrand" version = "2.4.1" @@ -503,6 +508,12 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" +[[package]] +name = "foldhash" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" + [[package]] name = "fs_extra" version = "1.3.0" @@ -606,6 +617,21 @@ dependencies = [ "slab", ] +[[package]] +name = "generator" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3b854b0e584ead1a33f18b2fcad7cf7be18b3875c78816b753639aa501513ae" +dependencies = [ + "cc", + "cfg-if", + "libc", + "log", + "rustversion", + "windows-link", + "windows-result", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -664,6 +690,12 @@ dependencies = [ "tracing", ] +[[package]] +name = "hashbag" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7040a10f52cba493ddb09926e15d10a9d8a28043708a405931fe4c6f19fac064" + [[package]] name = "hashbrown" version = "0.14.5" @@ -676,7 +708,7 @@ version = "0.15.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" dependencies = [ - "foldhash", + "foldhash 0.1.5", ] [[package]] @@ -684,6 +716,9 @@ name = "hashbrown" version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" +dependencies = [ + "foldhash 0.2.0", +] [[package]] name = "heck" @@ -817,13 +852,23 @@ dependencies = [ "serde_core", ] +[[package]] +name = "io-uring" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "595a0399f411a508feb2ec1e970a4a30c249351e30208960d58298de8660b0e5" +dependencies = [ + "bitflags 1.3.2", + "libc", +] + [[package]] name = "io-uring" version = "0.7.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fdd7bddefd0a8833b88a4b68f90dae22c7450d11b354198baee3874fd811b344" dependencies = [ - "bitflags", + "bitflags 2.11.0", "cfg-if", "libc", ] @@ -878,6 +923,17 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" +[[package]] +name = "left-right" +version = "0.11.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f0c21e4c8ff95f487fb34e6f9182875f42c84cef966d29216bf115d9bba835a" +dependencies = [ + "crossbeam-utils", + "loom", + "slab", +] + [[package]] name = "levenshtein_automata" version = "0.2.1" @@ -966,6 +1022,19 @@ dependencies = [ "logos-codegen", ] +[[package]] +name = "loom" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "419e0dc8046cb947daa77eb95ae174acfbddb7673b4151f56d1eed8e93fbfaca" +dependencies = [ + "cfg-if", + "generator", + "scoped-tls", + "tracing", + "tracing-subscriber", +] + [[package]] name = "lua-src" version = "550.0.0" @@ -1029,21 +1098,22 @@ dependencies = [ [[package]] name = "metrics" -version = "0.24.3" +version = "0.24.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d5312e9ba3771cfa961b585728215e3d972c950a3eed9252aa093d6301277e8" +checksum = "89550ee9f79e88fef3119de263694973a8adb26c21d75322164fb8c493039fe2" dependencies = [ - "ahash", "portable-atomic", + "rapidhash", ] [[package]] name = "metrics-exporter-prometheus" -version = "0.16.2" +version = "0.18.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd7399781913e5393588a8d8c6a2867bf85fb38eaf2502fdce465aad2dc6f034" +checksum = "1db0d8f1fc9e62caebd0319e11eaec5822b0186c171568f0480b46a0137f9108" dependencies = [ "base64", + "evmap", "http-body-util", "hyper", "hyper-util", @@ -1052,24 +1122,25 @@ dependencies = [ "metrics", "metrics-util", "quanta", - "thiserror 1.0.69", + "thiserror", "tokio", "tracing", ] [[package]] name = "metrics-util" -version = "0.19.1" +version = "0.20.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8496cc523d1f94c1385dd8f0f0c2c480b2b8aeccb5b7e4485ad6365523ae376" +checksum = "96f8722f8562635f92f8ed992f26df0532266eb03d5202607c20c0d7e9745e13" dependencies = [ "crossbeam-epoch", "crossbeam-utils", - "hashbrown 0.15.5", + "hashbrown 0.16.1", "metrics", "quanta", "rand 0.9.4", "rand_xoshiro", + "rapidhash", "sketches-ddsketch", ] @@ -1125,7 +1196,7 @@ dependencies = [ [[package]] name = "moon" -version = "0.3.0" +version = "0.6.0" dependencies = [ "anyhow", "arc-swap", @@ -1150,7 +1221,8 @@ dependencies = [ "http-body-util", "hyper", "hyper-util", - "io-uring", + "io-uring 0.6.4", + "io-uring 0.7.11", "itoa", "levenshtein_automata", "libc", @@ -1173,6 +1245,7 @@ dependencies = [ "rust-stemmers", "rustls", "rustls-pemfile", + "ryu", "serde", "serde_json", "sha1_smol", @@ -1181,7 +1254,7 @@ dependencies = [ "smallvec", "socket2", "stop-words", - "thiserror 2.0.18", + "thiserror", "tikv-jemalloc-ctl", "tikv-jemallocator", "tokio", @@ -1211,7 +1284,7 @@ version = "0.31.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d6d0705320c1e6ba1d912b5e37cf18071b6c2e9b7fa8215a1e8a7651966f5d3" dependencies = [ - "bitflags", + "bitflags 2.11.0", "cfg-if", "cfg_aliases", "libc", @@ -1506,13 +1579,22 @@ dependencies = [ "rand_core 0.9.5", ] +[[package]] +name = "rapidhash" +version = "4.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5da7e78a036ce858e8d55b7e7dc8ba3a88b78350fd2155d3591bbd966b58589e" +dependencies = [ + "rustversion", +] + [[package]] name = "raw-cpuid" version = "11.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "498cd0dc59d73224351ee52a95fee0f1a617a2eae0e7d9d720cc622c73a54186" dependencies = [ - "bitflags", + "bitflags 2.11.0", ] [[package]] @@ -1541,7 +1623,7 @@ version = "0.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" dependencies = [ - "bitflags", + "bitflags 2.11.0", ] [[package]] @@ -1577,9 +1659,9 @@ dependencies = [ [[package]] name = "ringbuf" -version = "0.4.8" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe47b720588c8702e34b5979cb3271a8b1842c7cb6f57408efa70c779363488c" +checksum = "2d3ecbcab081b935fb9c618b07654924f27686b4aac8818e700580a83eedcb7f" dependencies = [ "crossbeam-utils", "portable-atomic", @@ -1627,7 +1709,7 @@ version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" dependencies = [ - "bitflags", + "bitflags 2.11.0", "errno", "libc", "linux-raw-sys", @@ -1685,6 +1767,18 @@ version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" +[[package]] +name = "ryu" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" + +[[package]] +name = "scoped-tls" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294" + [[package]] name = "scopeguard" version = "1.2.0" @@ -1879,33 +1973,13 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "thiserror" -version = "1.0.69" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" -dependencies = [ - "thiserror-impl 1.0.69", -] - [[package]] name = "thiserror" version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl 2.0.18", -] - -[[package]] -name = "thiserror-impl" -version = "1.0.69" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" -dependencies = [ - "proc-macro2", - "quote", - "syn", + "thiserror-impl", ] [[package]] @@ -2289,7 +2363,7 @@ version = "0.244.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47b807c72e1bac69382b3a6fb3dbe8ea4c0ed87ff5629b8685ae6b9a611028fe" dependencies = [ - "bitflags", + "bitflags 2.11.0", "hashbrown 0.15.5", "indexmap", "semver", @@ -2342,6 +2416,15 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-result" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" +dependencies = [ + "windows-link", +] + [[package]] name = "windows-sys" version = "0.52.0" @@ -2482,7 +2565,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d66ea20e9553b30172b5e831994e35fbde2d165325bec84fc43dbf6f4eb9cb2" dependencies = [ "anyhow", - "bitflags", + "bitflags 2.11.0", "indexmap", "log", "serde", diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 628a3a87..41501e78 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -83,3 +83,8 @@ members = ["."] name = "graph_props_record" path = "fuzz_targets/graph_props_record.rs" doc = false + +[[bin]] +name = "mq_wal_record" +path = "fuzz_targets/mq_wal_record.rs" +doc = false diff --git a/fuzz/fuzz_targets/mq_wal_record.rs b/fuzz/fuzz_targets/mq_wal_record.rs new file mode 100644 index 00000000..8673410a --- /dev/null +++ b/fuzz/fuzz_targets/mq_wal_record.rs @@ -0,0 +1,26 @@ +#![no_main] +use libfuzzer_sys::fuzz_target; + +use moon::mq::wal::{ + decode_mq_ack, decode_mq_create, decode_mq_pop, decode_mq_push, decode_mq_trigger, + peek_version, +}; + +/// Fuzz the MQ WAL v3 op-blob decoders (Wave B stage 2a / task #34). +/// +/// `data` is fed to all five decoders directly -- exactly what +/// `src/shard/shared_databases.rs::apply_mq_wal_record` does with a raw WAL +/// record payload straight off disk (attacker/corruption-controlled: a +/// truncated write, a torn page, or a future-version payload written by a +/// newer binary that this build must never trust blindly). Every decoder is +/// documented to return `None` on ANY malformed input or unsupported +/// version byte -- NEVER panic, NEVER read out of bounds. Any panic or OOB +/// access here is a bug. +fuzz_target!(|data: &[u8]| { + let _ = peek_version(data); + let _ = decode_mq_create(data); + let _ = decode_mq_ack(data); + let _ = decode_mq_push(data); + let _ = decode_mq_pop(data); + let _ = decode_mq_trigger(data); +}); diff --git a/tests/crash_recovery_mq_effects.rs b/tests/crash_recovery_mq_effects.rs new file mode 100644 index 00000000..b55c872a --- /dev/null +++ b/tests/crash_recovery_mq_effects.rs @@ -0,0 +1,489 @@ +//! Wave B stage 2a (task #34): MQ effect-record WAL durability + replay, +//! live kill-9 round trip. +//! +//! `tests/crash_recovery_temporal_mq.rs` documents why a live MQ round trip +//! was NOT possible before this change: MQ.* commands are intercepted before +//! the generic AOF-logging dispatch path, so under `--appendonly yes` the +//! AOF-authority recovery (`main.rs`: `db.clear()` on every shard, rebuild +//! solely from the AOF manifest) discarded every durable Stream on every +//! restart, regardless of whether `replay_mq_wal`'s two task #42 defects +//! were fixed. This stage closes that gap: MQ.CREATE/PUSH/POP/ACK/TRIGGER +//! each emit their own versioned WAL v3 effect record at the owner-shard +//! execution site (`src/shard/mq_exec.rs`), and `replay_mq_wal` (which runs +//! AFTER the AOF-authority wipe in `main.rs`, see the ordering assertion in +//! `src/shard/shared_databases.rs::tests::test_replay_mq_wal_survives_prior_db_clear`) +//! rebuilds the full stream content, consumer-group PEL, DLQ routing, and +//! trigger registrations from that log alone. +//! +//! RED (pre-fix) behavior: every assertion below about restored content +//! fails — XLEN/XRANGE/XPENDING/DLQLEN/POP-cursor all read an EMPTY stream +//! after restart (AOF-authority `db.clear()` wiped it and nothing replayed +//! it back), and the trigger notification never fires (TriggerRegistry was +//! never persisted at all pre-fix). +//! +//! Run with (release binary required): +//! cargo build --release +//! cargo test --release --test crash_recovery_mq_effects -- --ignored --test-threads=1 + +#![allow(clippy::unwrap_used)] + +mod common; + +use std::io::{Read, Write}; +use std::net::{TcpStream, ToSocketAddrs}; +use std::process::{Child, Command}; +use std::time::{Duration, Instant}; + +// --------------------------------------------------------------------------- +// Binary resolution — honours MOON_BIN, falls back to release then debug. +// Pattern: tests/shardslice_live.rs::find_moon_binary +// --------------------------------------------------------------------------- + +fn find_moon_binary() -> std::path::PathBuf { + if let Ok(bin) = std::env::var("MOON_BIN") { + let p = std::path::PathBuf::from(bin); + if p.exists() { + return p; + } + } + std::path::PathBuf::from(env!("CARGO_BIN_EXE_moon")) +} + +// --------------------------------------------------------------------------- +// Server spawn helpers (pattern: tests/crash_recovery_temporal_mq.rs) +// --------------------------------------------------------------------------- + +fn spawn_moon(dir: &std::path::Path, shards: u32, extra: &[&str]) -> (Child, u16) { + common::spawn_listening(|port| { + let mut args: Vec = vec![ + "--port".into(), + port.to_string(), + "--dir".into(), + dir.to_string_lossy().into_owned(), + "--shards".into(), + shards.to_string(), + ]; + for &e in extra { + args.push(e.into()); + } + Command::new(find_moon_binary()) + .args(&args) + .stdout(std::fs::File::create(dir.join("moon.stdout.log")).expect("create stdout log")) + .stderr(std::fs::File::create(dir.join("moon.stderr.log")).expect("create stderr log")) + .env("RUST_LOG", "moon=info") + .spawn() + .unwrap_or_else(|e| { + panic!( + "Failed to spawn moon binary at '{}': {e}. Build with \ + `cargo build [--release]` or set MOON_BIN.", + find_moon_binary().display() + ) + }) + }) +} + +/// RAII guard: SIGKILLs the server process when dropped (`Child::kill` is +/// SIGKILL on Unix — no graceful shutdown, matching the "kill -9" scenario). +struct ServerGuard(Child); + +impl Drop for ServerGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +fn connect(port: u16, deadline: Duration) -> TcpStream { + let addr = format!("127.0.0.1:{port}") + .to_socket_addrs() + .expect("parse addr") + .next() + .expect("one addr"); + let start = Instant::now(); + loop { + match TcpStream::connect_timeout(&addr, Duration::from_millis(200)) { + Ok(s) => { + s.set_read_timeout(Some(Duration::from_secs(10))).ok(); + s.set_write_timeout(Some(Duration::from_secs(10))).ok(); + return s; + } + Err(_) if start.elapsed() < deadline => { + std::thread::sleep(Duration::from_millis(50)); + } + Err(e) => panic!("server never accepted on port {port}: {e}"), + } + } +} + +fn wait_ready(port: u16) -> TcpStream { + let mut s = connect(port, Duration::from_secs(30)); + let start = Instant::now(); + loop { + s.write_all(b"PING\r\n").expect("write PING"); + let mut buf = [0u8; 64]; + if let Ok(n) = s.read(&mut buf) { + if n > 0 && buf[..n].windows(4).any(|w| w == b"PONG") { + return s; + } + } + assert!( + start.elapsed() < Duration::from_secs(15), + "server accepted TCP but never answered PING on port {port}" + ); + std::thread::sleep(Duration::from_millis(100)); + s = connect(port, Duration::from_secs(5)); + } +} + +// --------------------------------------------------------------------------- +// Minimal RESP2 client (self-contained; copied from tests/shardslice_live.rs) +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, PartialEq)] +enum Resp { + Simple(String), + Error(String), + Int(i64), + Bulk(Option>), + Array(Option>), +} + +impl Resp { + fn flat(&self) -> String { + match self { + Resp::Simple(s) | Resp::Error(s) => s.clone(), + Resp::Int(i) => i.to_string(), + Resp::Bulk(Some(b)) => String::from_utf8_lossy(b).into_owned(), + Resp::Bulk(None) => "".into(), + Resp::Array(Some(items)) => items.iter().map(Resp::flat).collect::>().join(" "), + Resp::Array(None) => "".into(), + } + } +} + +struct Conn { + s: TcpStream, + buf: Vec, + pos: usize, +} + +impl Conn { + fn new(s: TcpStream) -> Self { + Conn { + s, + buf: Vec::with_capacity(16 * 1024), + pos: 0, + } + } + + fn open(port: u16) -> Self { + Conn::new(connect(port, Duration::from_secs(10))) + } + + fn cmd(&mut self, parts: &[&[u8]]) -> Resp { + let mut req = Vec::with_capacity(128); + req.extend_from_slice(format!("*{}\r\n", parts.len()).as_bytes()); + for p in parts { + req.extend_from_slice(format!("${}\r\n", p.len()).as_bytes()); + req.extend_from_slice(p); + req.extend_from_slice(b"\r\n"); + } + self.s.write_all(&req).expect("write cmd"); + self.frame() + } + + fn cmd_s(&mut self, parts: &[&str]) -> Resp { + let v: Vec<&[u8]> = parts.iter().map(|p| p.as_bytes()).collect(); + self.cmd(&v) + } + + fn fill(&mut self) { + let mut chunk = [0u8; 16 * 1024]; + let n = self.s.read(&mut chunk).expect("read from server"); + assert!(n > 0, "connection closed mid-frame"); + self.buf.extend_from_slice(&chunk[..n]); + } + + fn line(&mut self) -> String { + loop { + if let Some(rel) = self.buf[self.pos..].windows(2).position(|w| w == b"\r\n") { + let line = + String::from_utf8_lossy(&self.buf[self.pos..self.pos + rel]).into_owned(); + self.pos += rel + 2; + return line; + } + self.fill(); + } + } + + fn exact(&mut self, n: usize) -> Vec { + while self.buf.len() - self.pos < n + 2 { + self.fill(); + } + let out = self.buf[self.pos..self.pos + n].to_vec(); + self.pos += n + 2; + out + } + + /// Read one RESP frame. Used both for command replies and for + /// unsolicited pub/sub pushes on a SUBSCRIBEd connection. + fn frame(&mut self) -> Resp { + if self.pos > 0 && self.pos == self.buf.len() { + self.buf.clear(); + self.pos = 0; + } + let line = self.line(); + let (tag, rest) = line.split_at(1); + match tag { + "+" => Resp::Simple(rest.to_string()), + "-" => Resp::Error(rest.to_string()), + ":" => Resp::Int(rest.parse().unwrap_or(0)), + "$" => { + let n: i64 = rest.parse().unwrap_or(-1); + if n < 0 { + Resp::Bulk(None) + } else { + Resp::Bulk(Some(self.exact(n as usize))) + } + } + "*" => { + let n: i64 = rest.parse().unwrap_or(-1); + if n < 0 { + Resp::Array(None) + } else { + let mut items = Vec::with_capacity(n as usize); + for _ in 0..n { + items.push(self.frame()); + } + Resp::Array(Some(items)) + } + } + other => panic!("unexpected RESP tag {other:?} in line {line:?}"), + } + } +} + +fn as_array(r: &Resp) -> &[Resp] { + match r { + Resp::Array(Some(items)) => items, + other => panic!("expected array, got {other:?}"), + } +} + +fn as_int(r: &Resp) -> i64 { + match r { + Resp::Int(i) => *i, + other => panic!("expected integer, got {other:?}"), + } +} + +// --------------------------------------------------------------------------- +// MQ effect-record durability: full lifecycle survives kill -9 under +// `--appendonly yes` (the AOF-authority path that previously discarded +// every durable Stream unconditionally, see the module doc comment). +// --------------------------------------------------------------------------- + +#[test] +#[ignore] // Requires built release binary; run explicitly. +fn mq_effect_records_survive_kill9() { + let dir = tempfile::tempdir().expect("tempdir"); + let extra: &[&str] = &["--appendonly", "yes", "--disk-free-min-pct", "0"]; + + // --- Round 1: build up durable MQ state across 3 queues, kill -9. --- + { + let (child, port) = spawn_moon(dir.path(), 1, extra); + let _guard = ServerGuard(child); + drop(wait_ready(port)); + + let mut c = Conn::open(port); + + // `ordersq`: MAXDELIVERY 0 disables DLQ routing entirely (the + // `mdc > 0` guard in `handle_pop`) AND keeps `request_count == count` + // (no over-claim padding — `request_count = count + mdc`), so POP + // claims EXACTLY the requested count with no side effects on + // messages beyond it. Exercises PUSH content, POP cursor, and + // ACK-by-id (subset acked, rest stays pending in the PEL). + assert_eq!( + c.cmd_s(&["MQ", "CREATE", "ordersq", "MAXDELIVERY", "0"]), + Resp::Simple("OK".into()) + ); + for i in 1..=5 { + let seq = i.to_string(); + let reply = c.cmd_s(&["MQ", "PUSH", "ordersq", "seq", &seq]); + match reply { + Resp::Bulk(Some(_)) => {} + other => panic!("MQ PUSH must return the assigned id: {other:?}"), + } + } + // Claim the first 3 (seq 1..3); last_delivered_id cursor should land + // on msg 3's id. + let popped = c.cmd_s(&["MQ", "POP", "ordersq", "COUNT", "3"]); + let popped_items = as_array(&popped); + assert_eq!(popped_items.len(), 3, "must claim exactly 3 messages"); + let claimed_ids: Vec = popped_items + .iter() + .map(|entry| as_array(entry)[0].flat()) + .collect(); + let claimed_seq_fields: Vec = popped_items + .iter() + .map(|entry| as_array(&as_array(entry)[1])[1].flat()) + .collect(); + assert_eq!( + claimed_seq_fields, + vec!["1", "2", "3"], + "claim order must match push order (proves field content, not just count)" + ); + + // ACK the first two, leaving the third pending in the PEL. + let acked = c.cmd_s(&["MQ", "ACK", "ordersq", &claimed_ids[0], &claimed_ids[1]]); + assert_eq!(as_int(&acked), 2, "must ack exactly 2 messages"); + + // `dlqtestq`: MAXDELIVERY 1 forces immediate DLQ routing on first + // claim (read_group_new always claims with delivery_count=1). + assert_eq!( + c.cmd_s(&["MQ", "CREATE", "dlqtestq", "MAXDELIVERY", "1"]), + Resp::Simple("OK".into()) + ); + c.cmd_s(&["MQ", "PUSH", "dlqtestq", "f", "a"]); + c.cmd_s(&["MQ", "PUSH", "dlqtestq", "f", "b"]); + let dlq_popped = c.cmd_s(&["MQ", "POP", "dlqtestq", "COUNT", "2"]); + // Both entries route straight to the DLQ (delivery_count 1 >= mdc 1), + // so the POP reply itself is empty. + assert_eq!(as_array(&dlq_popped).len(), 0); + assert_eq!(as_int(&c.cmd_s(&["MQ", "DLQLEN", "dlqtestq"])), 2); + + // `trigq`: register a debounced trigger. Not yet armed (no PUSH), + // so `pending_fire_ms` is 0 at crash time — restoring the + // registration itself (queue_key/callback/debounce) is what's + // under test, not an in-flight timer. + assert_eq!( + c.cmd_s(&["MQ", "CREATE", "trigq"]), + Resp::Simple("OK".into()) + ); + assert_eq!( + c.cmd_s(&["MQ", "TRIGGER", "trigq", "FIRED", "DEBOUNCE", "200"]), + Resp::Simple("OK".into()) + ); + + // Let the 1ms WAL flush tick drain every MQ effect record before the + // kill -- mirrors tests/crash_recovery_temporal_mq.rs's settle wait. + std::thread::sleep(Duration::from_millis(1500)); + // _guard dropped here -> SIGKILL + } + + // --- Round 2: restart on the SAME dir. Assert every plane restored. --- + { + let (child2, port2) = spawn_moon(dir.path(), 1, extra); + let _guard2 = ServerGuard(child2); + drop(wait_ready(port2)); + + let mut c = Conn::open(port2); + + // (1) Stream CONTENT: all 5 pushed messages must still exist with + // their original field values, in id order -- proves MqPush replay + // rebuilt `entries` from the WAL after the AOF-authority db.clear(). + let xlen = c.cmd_s(&["XLEN", "ordersq"]); + assert_eq!( + as_int(&xlen), + 5, + "RED: all 5 MQ.PUSH'd messages must survive kill-9 (AOF-authority \ + db.clear() wipes the keyspace; only a correct MqPush WAL replay \ + rebuilds it)" + ); + let range = c.cmd_s(&["XRANGE", "ordersq", "-", "+"]); + let entries = as_array(&range); + assert_eq!(entries.len(), 5); + let seqs: Vec = entries + .iter() + .map(|e| { + let fields = as_array(&as_array(e)[1]); + fields[1].flat() // [field, value] pairs; "seq" is the only field + }) + .collect(); + assert_eq!( + seqs, + vec!["1", "2", "3", "4", "5"], + "field VALUES must survive verbatim, in original id order" + ); + + // (2) ACK-subset restored: msg 3 (never acked) must be the ONLY + // entry left pending after Round 1 -- proves MqAck replay applied + // by ID (removing exactly msg 1 & msg 2 from the PEL), not by a + // count-based heuristic. MUST run BEFORE the cursor-resume POP below + // -- MQ.POP is a claiming read (it inserts every newly-claimed id + // into the PEL), so checking XPENDING after it would conflate "ack + // replay worked" with "the live POP below also added msg 4 + msg 5 + // to the pending set", making 3 the RIGHT answer for the wrong + // reason (or masking a real ack-replay regression entirely). + let pending = c.cmd_s(&["XPENDING", "ordersq", "__mq_consumers"]); + let pending_fields = as_array(&pending); + assert_eq!( + as_int(&pending_fields[0]), + 1, + "RED: exactly 1 message (msg 3) must still be pending after \ + restart -- ack-by-count would roll back ALL 3 claimed messages \ + to pending instead of respecting the 2 that were actually acked" + ); + + // (3) Cursor position: POP after restart must resume at msg 4, NOT + // re-deliver msg 1..3 -- proves the consumer group's + // `last_delivered_id` was restored from the MqPop effect record. + let resumed = c.cmd_s(&["MQ", "POP", "ordersq", "COUNT", "10"]); + let resumed_items = as_array(&resumed); + assert_eq!( + resumed_items.len(), + 2, + "RED: cursor must resume after msg 3 (2 remaining: msg 4, msg 5) \ + -- a lost/rolled-back cursor would either re-deliver msg 1..3 \ + (count > 2) or (if the stream itself was lost) return 0" + ); + let resumed_seqs: Vec = resumed_items + .iter() + .map(|entry| { + let fields = as_array(&as_array(entry)[1]); + fields[1].flat() + }) + .collect(); + assert_eq!(resumed_seqs, vec!["4", "5"]); + + // (4) DLQ routing restored: both dlqtestq entries must still be in + // the dead-letter stream -- proves MqPop's DLQ routing decision + // (source id -> assigned DLQ id) replayed deterministically. + assert_eq!( + as_int(&c.cmd_s(&["MQ", "DLQLEN", "dlqtestq"])), + 2, + "RED: DLQ-routed entries must survive restart" + ); + + // (5) Trigger registration restored: subscribe to the trigger's + // notification channel, arm it with a fresh PUSH, and wait for the + // debounce window to elapse. If the TriggerRegistry entry did not + // survive, PUSH finds no registered trigger for `trigq` and nothing + // is ever published on `mq:trigger:trigq`. + let mut sub = Conn::open(port2); + let sub_reply = sub.cmd_s(&["SUBSCRIBE", "mq:trigger:trigq"]); + assert_eq!(as_array(&sub_reply)[0].flat(), "subscribe"); + + c.cmd_s(&["MQ", "PUSH", "trigq", "f", "v"]); + + // Debounce window is 200ms; allow generous slack for a loaded CI box. + sub.s + .set_read_timeout(Some(Duration::from_secs(10))) + .expect("set read timeout"); + let push_frame = sub.frame(); + let push_items = as_array(&push_frame); + assert_eq!( + push_items[0].flat(), + "message", + "RED: trigger registration must survive restart -- without it, \ + MQ.PUSH never re-arms a pending fire and no notification is \ + ever published on mq:trigger:trigq (got {push_frame:?})" + ); + assert_eq!(push_items[1].flat(), "mq:trigger:trigq"); + assert_eq!( + push_items[2].flat(), + "FIRED", + "callback_cmd payload must survive verbatim" + ); + } +} diff --git a/tests/crash_recovery_temporal_mq.rs b/tests/crash_recovery_temporal_mq.rs index 9f32451f..54b91fac 100644 --- a/tests/crash_recovery_temporal_mq.rs +++ b/tests/crash_recovery_temporal_mq.rs @@ -28,10 +28,11 @@ //! back with `durable=false` (`Stream::new()`'s default) UNLESS //! `replay_mq_wal` re-applies the `MqCreate` WAL record. //! -//! The MQ replay fix is proven by two white-box unit tests instead of a -//! live-server round trip here — `src/shard/shared_databases.rs` -//! `tests::test_replay_mq_wal_restores_registry_and_rolls_back_cursor` and -//! `tests::test_replay_mq_wal_missing_wal_v3_subdir_is_a_noop`. Reason: a +//! The MQ replay fix (task #42's two defects: wrong dir + blind to nested +//! `Command` framing) was originally proven by two white-box unit tests +//! instead of a live-server round trip here — `src/shard/shared_databases.rs` +//! `tests::test_replay_mq_wal_missing_wal_v3_subdir_is_a_noop` (still +//! present) and a since-replaced cursor-rollback-by-count test. Reason: a //! live MQ.CREATE -> kill -9 -> restart -> MQ.PUSH round trip requires //! `--appendonly yes` (WAL v3 only exists when `appendonly_enabled`, see //! `event_loop.rs`'s `wal_shard_dir` gate), but `--appendonly yes` triggers @@ -47,10 +48,19 @@ //! change fixes. Making it observable end-to-end needs either MQ AOF //! logging or a `.rrdshard`-only (non-AOF) recovery path — both are //! restructuring work forbidden by task #42's minimal-fix scope. The unit -//! tests instead drive `replay_mq_wal` directly against real WAL v3 bytes +//! tests instead drove `replay_mq_wal` directly against real WAL v3 bytes //! written with the production encoder (`write_wal_v3_record` + //! `WalWriterV3`), which isolates exactly the two fixed defects. //! +//! Wave B stage 2a (task #34) closed the deeper gap this comment describes: +//! MQ.CREATE/PUSH/POP/ACK/TRIGGER each now emit their own versioned WAL v3 +//! effect record at the owner-shard execution site +//! (`src/shard/mq_exec.rs`), so the AOF-authority wipe above no longer loses +//! MQ state — `replay_mq_wal` rebuilds it afterward. See +//! `tests/crash_recovery_mq_effects.rs::mq_effect_records_survive_kill9` for +//! the live kill-9 round trip this file's comment says was unreachable; it +//! is now the reachable, faithful proof for MQ that TEMPORAL already had. +//! //! TEMPORAL: `TEMPORAL.INVALIDATE` sets `valid_to` on the node/edge living //! in the graph's mutable `write_buf` and drains a `GraphTemporal` WAL //! record (`src/command/temporal.rs::apply_invalidate`). The node itself is @@ -304,14 +314,16 @@ impl Conn { } // --------------------------------------------------------------------------- -// MQ.CREATE / MQ.ACK durability: see the module doc comment above for why -// this is proven by white-box unit tests in -// `src/shard/shared_databases.rs` (`test_replay_mq_wal_restores_registry_and_rolls_back_cursor`, -// `test_replay_mq_wal_missing_wal_v3_subdir_is_a_noop`) rather than a live -// server round trip in this file — a live MQ.CREATE round trip requires -// `--appendonly yes`, which triggers an unrelated, out-of-scope -// AOF-authority wipe that discards the Stream (MQ has zero AOF durability) -// regardless of whether `replay_mq_wal` itself is correct. +// MQ.CREATE / MQ.ACK durability: originally proven only by white-box unit +// tests in `src/shard/shared_databases.rs` +// (`test_replay_mq_wal_missing_wal_v3_subdir_is_a_noop` and others since +// superseded) rather than a live server round trip in this file, because a +// live MQ.CREATE round trip required `--appendonly yes`, which triggered an +// AOF-authority wipe that discarded the Stream (MQ had zero AOF durability +// at the time) regardless of whether `replay_mq_wal` itself was correct. +// Wave B stage 2a (task #34) closed that gap — see the module doc comment +// above and `tests/crash_recovery_mq_effects.rs` for the now-reachable live +// round trip. // --------------------------------------------------------------------------- // --------------------------------------------------------------------------- From 2af70899a82fd936cbc744fc67b56e017bc28b52 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 10:49:45 +0700 Subject: [PATCH 6/7] =?UTF-8?q?fix(persistence):=20WAL=20recycle=20plane?= =?UTF-8?q?=20guard=20=E2=80=94=20never=20delete=20sole-copy=20WS/MQ/tempo?= =?UTF-8?q?ral=20history?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of the stage-2a branch (verdict SHIP-WITH-FIXES) confirmed a P0: every WAL recycler — autovacuum Pass C in disk-offload mode, the checkpoint protocol's recycle_segments_before, and the admin VACUUM path — deletes sealed segments against an LSN floor that only covers KV pages and the graph store. The workspace/MQ/temporal planes have no snapshot format in ANY mode, so a disk-offload deployment under --max-wal-size pressure permanently lost MQ/WS/temporal history during normal operation — no crash required. Stage 2a raised the blast radius from queue definitions to full stream content/PEL/DLQ/trigger history. Fix is central, in the recyclers themselves (kernel principle: recycle takes the min across planes): segment_holds_plane_history() walks a sealed segment's record-type bytes and any segment holding a plane record is skipped by BOTH recycle_aggressive and recycle_segments_before, regardless of caller. Fail-closed: unreadable file, non-v3 header, unknown (future) record type, or torn tail all keep the segment; a record_len==0 tail is normal zero-padding. Blocked segments are reported via RecycleStats.segments_blocked_plane; the autovacuum disk-offload arm feeds them into reclamation_wal_recycle_blocked_no_checkpoint_total with a rate-limited warn. Pure-KV segments recycle exactly as before (covered by the pre-existing disk-offload test, kept green). Also from the review round: - P2 regression tests: pre-versioning MqCreate/MqAck payloads (written only by dev builds between PR #286 and this branch) fail closed under the new versioned decoders, including the key_len-low-byte==1 collision shapes (1- and 257-byte keys). - P1 filed as task #46 + CHANGELOG caveat: durable MQ streams deleted via generic DEL/UNLINK/FLUSHALL are resurrected with full content by replay — needs an MqDrop tombstone plane (same class as the fixed vector/KV cold-plane resurrection). - CHANGELOG: measured WAL footprint added (~68 B/small PUSH, ~132 B per 3-claim POP, 576 B for the crash-test lifecycle). Red/green: test_recycle_aggressive_keeps_plane_history_segments and test_recycle_segments_before_keeps_plane_history_segments failed against the unguarded recyclers (compile-red on the stats field, then assertion-red), green after; test_pass_c_disk_offload_keeps_plane_segments_and_counts_blocked covers the autovacuum wiring + counter. author: Tin Dang --- CHANGELOG.md | 25 +++++ src/mq/wal.rs | 56 ++++++++++ src/persistence/wal_v3/segment.rs | 169 ++++++++++++++++++++++++++++++ src/shard/autovacuum.rs | 103 +++++++++++++++++- 4 files changed, 348 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78fd8247..2688547e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,31 @@ DLQ routing, and trigger re-arming all survive. New fuzz target `mq_wal_record` covers the five new op-blob decoders (`fuzz/fuzz_targets/`, registered in both `fuzz-pr` and `fuzz-nightly` CI matrices). +Measured WAL footprint (release, single shard): CREATE + 5×PUSH + POP(3) += 7 records, 576 bytes on disk including the 64-byte segment header — +~68 bytes per small PUSH (48-byte payload + 20-byte framing/CRC), ~132 +bytes for a 3-claim POP. + +**WAL recycle plane guard (adversarial-review fix):** `recycle_aggressive` +and `recycle_segments_before` now refuse to delete any sealed segment +holding workspace/MQ/temporal records — those planes have no snapshot +format in ANY mode, so the WAL is their sole durable copy and no caller's +LSN floor (autovacuum Pass C in disk-offload mode, the checkpoint +protocol, admin `VACUUM`) makes such a segment safe. Previously, +disk-offload deployments under `--max-wal-size` pressure would silently +and permanently lose MQ/WS/temporal history during normal operation, no +crash required. Kept segments are counted in +`reclamation_wal_recycle_blocked_no_checkpoint_total` and warned +(rate-limited); WAL size may exceed `--max-wal-size` for plane-heavy +workloads until plane checkpointing lands (storage-kernel M3). The +guard fails closed: unreadable/torn/unknown-type segments are kept. + +Known limitation (pre-existing, now tracked): durable MQ streams deleted +via generic `DEL`/`UNLINK`/`FLUSHALL`/`FLUSHDB` are resurrected — now with +full content — by MQ WAL replay after a restart; there is no MQ tombstone +record yet (same bug class as the fixed vector/KV cold-plane resurrection; +follow-up task filed). + Out of scope (stage 2b+): replication emission/apply for the MQ plane. ### Changed — WAL v3 `wal_append` channel now preserves the caller's REAL record type end-to-end (K1a, storage-kernel M1 stage 1) diff --git a/src/mq/wal.rs b/src/mq/wal.rs index 871660fa..d96efbe6 100644 --- a/src/mq/wal.rs +++ b/src/mq/wal.rs @@ -677,4 +677,60 @@ mod tests { fn test_peek_version_empty() { assert_eq!(peek_version(&[]), None); } + + /// Adversarial-review P2: pre-versioning (unversioned) MqCreate/MqAck + /// payloads written by dev builds between PR #286 and the stage-2a + /// version bump must FAIL CLOSED (`None` → skip-and-warn at replay), + /// never decode as garbage. The dangerous shape is a legacy record + /// whose `key_len` low byte equals `MQ_WAL_VERSION` (1): its first + /// byte passes the version check and the remaining bytes get + /// reinterpreted at shifted offsets. + #[test] + fn test_legacy_unversioned_payloads_fail_closed() { + // Legacy MqCreate: [key_len:u32][key][mdc:u32] with key_len = 1. + // First byte = 0x01 == MQ_WAL_VERSION → version check passes; the + // reinterpreted db_index/key_len fields must then fail the + // structural length checks. + let mut legacy_create = Vec::new(); + legacy_create.extend_from_slice(&1u32.to_le_bytes()); + legacy_create.push(b'q'); + legacy_create.extend_from_slice(&3u32.to_le_bytes()); + assert_eq!( + decode_mq_create(&legacy_create), + None, + "legacy 1-byte-key MqCreate must fail closed, not decode as garbage" + ); + + // Same shape with key_len = 257 (low byte still 0x01). + let key = vec![b'k'; 257]; + let mut legacy_create_257 = Vec::new(); + legacy_create_257.extend_from_slice(&257u32.to_le_bytes()); + legacy_create_257.extend_from_slice(&key); + legacy_create_257.extend_from_slice(&3u32.to_le_bytes()); + assert_eq!( + decode_mq_create(&legacy_create_257), + None, + "legacy 257-byte-key MqCreate must fail closed" + ); + + // Legacy MqAck: [key_len:u32][key][ms:u64][seq:u64], key_len = 1. + let mut legacy_ack = Vec::new(); + legacy_ack.extend_from_slice(&1u32.to_le_bytes()); + legacy_ack.push(b'q'); + legacy_ack.extend_from_slice(&123u64.to_le_bytes()); + legacy_ack.extend_from_slice(&7u64.to_le_bytes()); + assert_eq!( + decode_mq_ack(&legacy_ack), + None, + "legacy 1-byte-key MqAck must fail closed" + ); + + // Legacy layouts whose first byte is NOT the version (key_len 2) + // fail at the version gate directly. + let mut legacy_create_2 = Vec::new(); + legacy_create_2.extend_from_slice(&2u32.to_le_bytes()); + legacy_create_2.extend_from_slice(b"qq"); + legacy_create_2.extend_from_slice(&3u32.to_le_bytes()); + assert_eq!(decode_mq_create(&legacy_create_2), None); + } } diff --git a/src/persistence/wal_v3/segment.rs b/src/persistence/wal_v3/segment.rs index ef5bde2f..1847dac8 100644 --- a/src/persistence/wal_v3/segment.rs +++ b/src/persistence/wal_v3/segment.rs @@ -101,6 +101,77 @@ pub struct RecycleStats { pub segments_recycled: usize, /// Total bytes freed from disk. pub bytes_reclaimed: u64, + /// LSN-eligible segments KEPT because they hold sole-copy plane history + /// (see [`segment_holds_plane_history`]). Callers surface this via the + /// `RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL` counter. + pub segments_blocked_plane: usize, +} + +/// Return `true` when the sealed segment at `path` contains at least one +/// record whose ONLY durable copy is the WAL itself. +/// +/// The workspace / MQ / temporal planes have no snapshot or checkpoint +/// format in ANY mode (their replay is WAL-only — see task #43 and the +/// Wave B stage 2a review): the disk-offload checkpoint covers KV pages and +/// the graph store, nothing else. A segment holding such a record must +/// never be recycled regardless of the caller's LSN floor, or that history +/// is permanently lost on the next restart. `GraphTemporal` is included +/// conservatively: autovacuum's recycle floor (`current_lsn`) is not tied +/// to a completed graph checkpoint. +/// +/// Fail-closed contract: an unreadable file, a non-v3 header, an unknown +/// record-type byte (a future plane type this build predates), or a +/// malformed/torn record tail all return `true` (keep the segment). A +/// crash-torn tail can only occur in the previously-active segment of an +/// earlier process generation — blocking that one segment is bounded and +/// beats guessing. A `record_len == 0` tail is normal zero-padding → end +/// of records. +fn segment_holds_plane_history(path: &Path) -> bool { + use super::record::WalRecordType; + + let data = match fs::read(path) { + Ok(d) => d, + Err(_) => return true, + }; + if data.len() < WAL_V3_HEADER_SIZE || &data[..6] != WAL_V3_MAGIC || data[6] != WAL_V3_VERSION { + return true; + } + let mut offset = WAL_V3_HEADER_SIZE; + while offset + 4 <= data.len() { + let record_len = u32::from_le_bytes([ + data[offset], + data[offset + 1], + data[offset + 2], + data[offset + 3], + ]) as usize; + if record_len == 0 { + return false; // zero-padded tail — clean end of records + } + // Minimum framing: len(4) + header(12) + crc(4). + if record_len < 20 || offset + record_len > data.len() { + return true; // torn/malformed sealed tail — fail closed + } + let plane = match WalRecordType::from_u8(data[offset + 12]) { + Some( + WalRecordType::TemporalUpsert + | WalRecordType::GraphTemporal + | WalRecordType::WorkspaceCreate + | WalRecordType::WorkspaceDrop + | WalRecordType::MqCreate + | WalRecordType::MqAck + | WalRecordType::MqPush + | WalRecordType::MqPop + | WalRecordType::MqTrigger, + ) => true, + Some(_) => false, + None => return true, // unknown (future) type — fail closed + }; + if plane { + return true; + } + offset += record_len; + } + false } /// WAL v3 writer with segmented files, per-record LSN, and batched fsync. @@ -496,6 +567,7 @@ impl WalWriterV3 { let mut segments_recycled = 0usize; let mut bytes_reclaimed = 0u64; + let mut segments_blocked_plane = 0usize; for i in 0..all_segments.len() { let seg = &all_segments[i]; @@ -510,6 +582,14 @@ impl WalWriterV3 { if next_base == 0 || next_base > redo_lsn { continue; } + // Plane guard: WS/MQ/temporal records have no snapshot in any + // mode — the WAL is their sole durable copy, so no LSN floor + // makes this segment safe to delete. See + // `segment_holds_plane_history` for the fail-closed contract. + if segment_holds_plane_history(&seg.path) { + segments_blocked_plane += 1; + continue; + } // Aggressive: skip the min_wal_bytes floor check. match fs::remove_file(&seg.path) { Ok(()) => { @@ -533,6 +613,7 @@ impl WalWriterV3 { Ok(RecycleStats { segments_recycled, bytes_reclaimed, + segments_blocked_plane, }) } @@ -698,6 +779,16 @@ impl WalWriterV3 { if next_base == 0 || next_base > redo_lsn { continue; } + // Plane guard: the checkpoint floor covers KV pages + graph + // only — WS/MQ/temporal history has no snapshot in any mode. + // See `segment_holds_plane_history` (fail-closed). + if segment_holds_plane_history(&seg.path) { + tracing::debug!( + "WAL recycle: keeping segment {:?} — holds sole-copy plane history", + seg.path + ); + continue; + } // Check min_wal_bytes floor: stop if removing this segment would // drop total WAL below the minimum. if total_wal_size.saturating_sub(seg.file_size) < self.min_wal_bytes { @@ -1152,6 +1243,84 @@ mod tests { assert_eq!(after, before - recycled); } + /// Adversarial-review P0 (Wave B stage 2a): a sealed segment holding + /// plane records (WS/MQ/temporal — sole durable copy, no snapshot format + /// in ANY mode) must survive `recycle_aggressive`, no matter the LSN + /// floor the caller passes. Pure-KV segments must still be recycled. + #[test] + fn test_recycle_aggressive_keeps_plane_history_segments() { + let tmp = tempfile::tempdir().unwrap(); + let wal_dir = tmp.path().join("wal"); + let mut writer = WalWriterV3::new(0, &wal_dir, 512).unwrap(); + writer.set_wal_bounds(0, u64::MAX); + + // First record = an MqPush plane record → lands in segment 1. + writer.append(WalRecordType::MqPush, b"\x01mq-plane-payload"); + // Then enough KV commands to seal several pure-Command segments. + for i in 0..60 { + writer.append(WalRecordType::Command, b"SET key val"); + if (i + 1) % 3 == 0 { + writer.flush_sync().unwrap(); + } + } + writer.flush_sync().unwrap(); + let active_seq = writer.current_segment_sequence(); + assert!(active_seq >= 4, "need several sealed segments"); + + let stats = writer.recycle_aggressive(writer.current_lsn()).unwrap(); + + // Segment 1 (holds the MqPush record) must survive. + let plane_path = WalSegment::segment_path(&wal_dir, 1); + assert!( + plane_path.exists(), + "segment holding sole-copy MQ plane history must never be recycled" + ); + assert!( + stats.segments_blocked_plane >= 1, + "blocked plane segment must be observable in RecycleStats" + ); + // Pure-Command sealed segments must still be recycled (the guard + // must not turn into a blanket recycle freeze). + assert!( + stats.segments_recycled >= 1, + "pure-KV sealed segments must still be recycled" + ); + let seg2_path = WalSegment::segment_path(&wal_dir, 2); + assert!( + !seg2_path.exists(), + "pure-Command segment 2 should have been recycled" + ); + } + + /// Same guard for the checkpoint-path recycler: `recycle_segments_before` + /// must skip plane-history segments too (it is called with a real + /// checkpoint floor, but the checkpoint covers KV pages + graph only — + /// planes have no snapshot). + #[test] + fn test_recycle_segments_before_keeps_plane_history_segments() { + let tmp = tempfile::tempdir().unwrap(); + let wal_dir = tmp.path().join("wal"); + let mut writer = WalWriterV3::new(0, &wal_dir, 512).unwrap(); + writer.set_wal_bounds(0, u64::MAX); + + writer.append(WalRecordType::WorkspaceCreate, b"ws-plane-payload"); + for i in 0..60 { + writer.append(WalRecordType::Command, b"SET key val"); + if (i + 1) % 3 == 0 { + writer.flush_sync().unwrap(); + } + } + writer.flush_sync().unwrap(); + + let recycled = writer.recycle_segments_before(u64::MAX).unwrap(); + assert!(recycled >= 1, "pure-KV segments must still be recycled"); + let plane_path = WalSegment::segment_path(&wal_dir, 1); + assert!( + plane_path.exists(), + "segment holding sole-copy WS plane history must never be recycled" + ); + } + #[test] fn test_recycle_respects_min_wal_size() { let tmp = tempfile::tempdir().unwrap(); diff --git a/src/shard/autovacuum.rs b/src/shard/autovacuum.rs index eed834c0..165a8b3f 100644 --- a/src/shard/autovacuum.rs +++ b/src/shard/autovacuum.rs @@ -231,8 +231,11 @@ impl AutovacuumDaemon { /// "assume every sealed segment is durable elsewhere") as its floor — /// true only when a checkpoint backs it. When `false`, Pass C does /// NOT recycle; it logs a rate-limited warning and increments - /// `RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL` instead. Disk-offload - /// mode behavior is unchanged from before this fix. + /// `RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL` instead. In + /// disk-offload mode, recycling proceeds — but `recycle_aggressive` + /// itself keeps any segment holding WS/MQ/temporal plane records + /// (sole durable copy in EVERY mode; Wave B stage 2a review fix), + /// surfacing them via the same blocked counter. /// * `manifest_retain_epochs` / `manifest_retain_secs` — P1 config. /// * `max_immutable_segments` — trigger threshold for vector compact pass. /// * `shutdown_requested` — if true, abort immediately after the current pass. @@ -357,7 +360,12 @@ impl AutovacuumDaemon { // a real redo_lsn and persist_graph_at_checkpoint // snapshots the graph write-buffer, so recycling // everything before current_lsn is backed by an - // actual durable floor. UNCHANGED from before task #43. + // actual durable floor — for KV and graph. The + // WS/MQ/temporal planes have NO snapshot in this mode + // either; `recycle_aggressive` itself skips any + // segment holding such records (Wave B stage 2a + // review fix) and reports them via + // `segments_blocked_plane`. let redo_lsn = wal.current_lsn(); match wal.recycle_aggressive(redo_lsn) { Ok(recycled) => { @@ -371,6 +379,26 @@ impl AutovacuumDaemon { "autovacuum: pass C (WAL recycle) freed segments" ); } + if recycled.segments_blocked_plane > 0 { + crate::command::info_reclamation::RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL + .fetch_add(recycled.segments_blocked_plane as u64, Ordering::Relaxed); + let should_warn = + self.last_wal_recycle_warn_at.map_or(true, |t| { + now.duration_since(t) >= Duration::from_secs(300) + }); + if should_warn { + tracing::warn!( + segments_blocked = recycled.segments_blocked_plane, + "autovacuum: pass C kept WAL segment(s) holding \ + workspace/MQ/temporal history — these planes have \ + no snapshot format, the WAL is their sole durable \ + copy, so they are excluded from recycling and WAL \ + size may stay above --max-wal-size until plane \ + checkpointing lands (storage-kernel M3)" + ); + self.last_wal_recycle_warn_at = Some(now); + } + } } Err(e) => { tracing::warn!( @@ -937,8 +965,73 @@ mod tests { ); } - /// Disk-offload mode is unchanged by task #43: Pass C still recycles - /// aggressively against `current_lsn()`, exactly as before this fix. + /// Wave B stage 2a review fix: in disk-offload mode Pass C recycles + /// pure-KV segments but keeps any segment holding plane records + /// (WS/MQ/temporal — no snapshot format in any mode), counting them in + /// `RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL`. + #[test] + fn test_pass_c_disk_offload_keeps_plane_segments_and_counts_blocked() { + use crate::persistence::wal_v3::record::WalRecordType; + let tmp = tempfile::tempdir().unwrap(); + let wal_dir = tmp.path().join("wal"); + let mut writer = + crate::persistence::wal_v3::segment::WalWriterV3::new(0, &wal_dir, 512).unwrap(); + // Plane record first (lands in segment 1), then KV filler. + writer.append(WalRecordType::MqPush, b"\x01mq-plane-payload"); + for i in 0..80 { + writer.append(WalRecordType::Command, b"SET key val filler"); + if (i + 1) % 3 == 0 { + writer.flush_sync().unwrap(); + } + } + writer.flush_sync().unwrap(); + let before = count_wal_segments(&wal_dir); + let before_blocked = + crate::command::info_reclamation::RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL + .load(Ordering::Relaxed); + + let mut vector_store = crate::vector::store::VectorStore::new(); + #[cfg(feature = "graph")] + let mut graph_store = crate::graph::store::GraphStore::new(); + let mut daemon = AutovacuumDaemon::new(AutovacuumConfig::default()); + daemon.run_tick( + &mut vector_store, + #[cfg(feature = "graph")] + &mut graph_store, + None, + Some(&mut writer), + 256, // max_wal_bytes: tiny, so the ceiling is already breached + true, // disk_offload_enabled + 0, + 0, + usize::MAX, + usize::MAX, + 1.0, + false, + ); + + let after = count_wal_segments(&wal_dir); + assert!( + after < before, + "pure-KV sealed segments must still be recycled (before={before}, after={after})" + ); + let plane_path = crate::persistence::wal_v3::segment::WalSegment::segment_path(&wal_dir, 1); + assert!( + plane_path.exists(), + "segment 1 (holds the MqPush plane record) must survive Pass C" + ); + let after_blocked = + crate::command::info_reclamation::RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL + .load(Ordering::Relaxed); + assert!( + after_blocked > before_blocked, + "blocked plane segment must be observable via the RECL counter" + ); + } + + /// Disk-offload mode recycling of PURE-KV history is unchanged by task + /// #43 and by the stage-2a plane guard: Pass C still recycles + /// Command-only segments against `current_lsn()`. #[test] fn test_pass_c_disk_offload_mode_recycles_unchanged() { let tmp = tempfile::tempdir().unwrap(); From a6c8d1154bab300cc011a7cd890ecfbab3860254 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sun, 12 Jul 2026 11:08:26 +0700 Subject: [PATCH 7/7] =?UTF-8?q?fix(mq):=20make=20wal=5Fappend=20channel=20?= =?UTF-8?q?drops=20LOUD=20=E2=80=94=20error=20log=20+=20INFO=20counter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit (PR #291, both txn.rs call sites): wal_append_on_slice used try_send and discarded the result, so a full/disconnected channel could silently drop an MqPush record AFTER the stream mutation was applied — TXN.COMMIT succeeds, restart recovery loses the message, nobody knows. Blocking is not an option here: every caller runs ON the shard thread, which is also the channel's sole consumer (1ms-tick drain in event_loop.rs) — waiting on a full channel would deadlock the very drain that empties it. Overflow requires >4096 records in a single tick, and the mutation cannot be unwound mid-commit, so the correct policy is observability: on try_send failure emit tracing::error! and increment the new reclamation_wal_append_channel_dropped_total INFO counter (# Reclamation section, field count 29 → 30 with test updated). Operators alert on any increase; zero means the durability guarantee held. Failure-policy rationale documented on the helper. author: Tin Dang --- src/command/info_reclamation.rs | 23 +++++++++++++++++------ src/shard/mq_exec.rs | 25 +++++++++++++++++++++---- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/command/info_reclamation.rs b/src/command/info_reclamation.rs index 0beb7060..de2118f7 100644 --- a/src/command/info_reclamation.rs +++ b/src/command/info_reclamation.rs @@ -1,7 +1,7 @@ //! `# Reclamation` INFO section — observability foundation for Wave-1 production //! reclamation hardening (P10, v0.1.13). //! -//! All 29 fields are plumbed here. Wave-2 agents fill in real values by +//! All 30 fields are plumbed here. Wave-2 agents fill in real values by //! incrementing the public atomics exported from this module. Until then, every //! field that lacks a real source emits `0` or `-1` as a sentinel, with a //! `// TODO(P10→Wave2):` comment marking the wire point. @@ -144,6 +144,14 @@ pub static RECL_AUTOVACUUM_THROTTLED_DUE_TO_LOAD: AtomicU64 = AtomicU64::new(0); /// configured ceiling; it never means data was lost. pub static RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL: AtomicU64 = AtomicU64::new(0); +/// Cumulative count of plane WAL records (MQ/WS/temporal) DROPPED because the +/// shard's `wal_append` channel was full at enqueue time (capacity 4096, +/// drained every 1ms by the same shard thread — blocking there would deadlock +/// the drain, so `try_send` is structural). Non-zero means an already-applied +/// in-memory mutation is MISSING from crash recovery: a real durability gap. +/// Alert on any increase. See `mq_exec::wal_append_on_slice`. +pub static RECL_WAL_APPEND_CHANNEL_DROPPED_TOTAL: AtomicU64 = AtomicU64::new(0); + /// Graph plan-cache hit count (cumulative). Used to compute hit_ratio. /// TODO(P10→Wave2): wire from PlanCache::get() on hit path. pub static RECL_PLAN_CACHE_HITS: AtomicU64 = AtomicU64::new(0); @@ -167,7 +175,7 @@ pub static RECL_DELETE_PENDING_VISIBLE_LSN: AtomicI64 = AtomicI64::new(-1); /// Append the `# Reclamation` INFO section to `buf`. /// -/// Reads all 29 fields from the module-level atomics above using `Relaxed` +/// Reads all 30 fields from the module-level atomics above using `Relaxed` /// ordering — same pattern as `crate::vector::metrics`. No allocation beyond /// the caller's pre-sized `String`. /// @@ -200,10 +208,12 @@ pub fn write_reclamation_section(buf: &mut String) { buf, "reclamation_wal_bytes:{}\r\n\ reclamation_wal_segments:{}\r\n\ - reclamation_wal_recycle_blocked_no_checkpoint_total:{}\r\n", + reclamation_wal_recycle_blocked_no_checkpoint_total:{}\r\n\ + reclamation_wal_append_channel_dropped_total:{}\r\n", RECL_WAL_BYTES.load(Ordering::Relaxed), RECL_WAL_SEGMENTS.load(Ordering::Relaxed), - RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL.load(Ordering::Relaxed) + RECL_WAL_RECYCLE_BLOCKED_NO_CHECKPOINT_TOTAL.load(Ordering::Relaxed), + RECL_WAL_APPEND_CHANNEL_DROPPED_TOTAL.load(Ordering::Relaxed) ); // -- Write stall: OR of disk-pressure (MA12), segment-backlog (MA1), and @@ -373,9 +383,9 @@ pub fn write_reclamation_section(buf: &mut String) { mod tests { use super::*; - /// All 29 required field keys must appear in the reclamation section output. + /// All 30 required field keys must appear in the reclamation section output. /// - /// RED: fails until `write_reclamation_section` emits all 29 fields. + /// RED: fails until `write_reclamation_section` emits all 30 fields. #[test] fn info_reclamation_contains_all_required_fields() { let mut buf = String::with_capacity(2048); @@ -413,6 +423,7 @@ mod tests { "reclamation_plan_cache_hit_ratio:", "reclamation_plan_cache_evictions_total:", "reclamation_delete_pending_visible_lsn:", + "reclamation_wal_append_channel_dropped_total:", ]; for field in required_fields { diff --git a/src/shard/mq_exec.rs b/src/shard/mq_exec.rs index 5d04e31b..7e027b34 100644 --- a/src/shard/mq_exec.rs +++ b/src/shard/mq_exec.rs @@ -188,9 +188,15 @@ fn derive_trig_key(key_prefix: &Bytes, raw_queue_key: &Bytes) -> Bytes { /// pre-framing); the event-loop drain calls `wal.append(record_type, &payload)` /// directly. /// -/// Mirrors `ShardDatabases::wal_append` semantics — `try_send` failures are -/// ignored (the channel is bounded; under extreme backpressure the record is -/// dropped, same as the existing lock-path). +/// Failure policy (CodeRabbit PR #291): `try_send` is structurally required +/// here — every caller runs ON the shard thread, which is also the channel's +/// sole consumer (the 1ms-tick drain in `event_loop.rs`), so blocking on a +/// full channel would deadlock the drain that empties it. Overflow therefore +/// needs >4096 records enqueued within a single tick. When it DOES happen +/// the mutation has already been applied and cannot be unwound mid-commit, +/// so the record loss is made LOUD instead of silent: `tracing::error!` + +/// the `RECL_WAL_APPEND_CHANNEL_DROPPED_TOTAL` INFO counter, giving +/// operators a durability-gap signal to alert on. /// /// `pub(crate)`: also called from `src/server/conn/handler_monoio/txn.rs` and /// `src/server/conn/handler_sharded/txn.rs` (MQ.PUBLISH self-fold) and @@ -203,7 +209,18 @@ pub(crate) fn wal_append_on_slice( ) { crate::shard::slice::with_shard(|s| { if let Some(ref tx) = s.wal_append_tx { - let _ = tx.try_send((record_type, payload)); + if tx.try_send((record_type, payload)).is_err() { + crate::command::info_reclamation::RECL_WAL_APPEND_CHANNEL_DROPPED_TOTAL + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); + tracing::error!( + ?record_type, + "WAL append channel rejected a plane record AFTER the \ + in-memory mutation was applied — this record will be \ + MISSING from crash recovery. The channel is drained by \ + this same shard thread every 1ms (capacity 4096), so \ + this indicates a pathological single-tick burst." + ); + } } }); }