fix: durability wave 1 — rewrite-window append overflow, degraded latch, WAL mid-chain tear policy (#452, #54) - #454
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis change adds bounded AOF rewrite-overflow buffering, sticky append-drop health tracking, shared reason-DEL backpressure budgets, persistence metrics, and WAL v3 salvage handling. It also adds unit, integration, and recovery tests. ChangesDurability and recovery hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoFix durability wave 1: AOF rewrite overflow, degraded latch, WAL mid-chain tears
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/persistence/aof/writer_task.rs (1)
1324-1332: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not discard overflow records while this writer remains active.
A producer can spill an append after
overflow.arm()and beforewriter.flush()fails.disarm_dropping()then discards that append, but this branch returns to the writer loop. An EverySec client can already have received+OK.If the old incremental file remains usable, drain the overflow into it. Otherwise latch the writer failure and stop accepting successful appends.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/persistence/aof/writer_task.rs` around lines 1324 - 1332, Update the flush-error branch in the writer loop around writer.flush() so overflow records produced after overflow.arm() are never discarded while the writer remains active. If the old incremental file is still usable, drain overflow into it; otherwise latch the writer failure and stop accepting successful appends, ensuring no +OK append can be lost before the loop exits.
🧹 Nitpick comments (1)
src/persistence/aof/rewrite_overflow.rs (1)
99-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNote the approximate nature of the byte cap.
try_spillaccounts only payload bytes. The 12-byte framed header, theSELECTprefix records emitted at drain time, and theVec<AofMessage>element overhead are not counted. Actual resident memory therefore exceedsmax_bytesunder many small appends. The overshoot is bounded and the design intent is a soft guard, so document the cap as approximate rather than changing the accounting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/persistence/aof/rewrite_overflow.rs` around lines 99 - 120, Document in the overflow buffer implementation that the max_bytes limit enforced by try_spill is an approximate soft cap, since accounting covers payload bytes but excludes framing, SELECT prefixes, and Vec<AofMessage> overhead. Do not change the existing byte accounting or spill behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 36-38: The changelog claim about every dropped acked append is
inconsistent with try_send_append_no_spill, which increments
AOF_BACKPRESSURE_DROPPED without latching aof_last_append_status:err. Route this
drop through the shared accounting helper so it performs the required latch, or
narrow the changelog wording to exclude this path.
In `@src/persistence/aof/mod.rs`:
- Around line 115-122: Make the helper mandatory and align all documentation
with that contract: update record_reason_del_dropped in
src/replication/reason_del.rs to use record_append_dropped while preserving its
reason-DEL-specific counter, close the direct-counter bypass in
try_send_append_no_spill in src/persistence/aof/pool.rs, and retain the
universal helper wording in src/persistence/aof/mod.rs. Update CHANGELOG.md
lines 36-38 only after every dropped acked append routes through the helper.
In `@src/persistence/aof/pool.rs`:
- Line 689: Update send_append_backpressure and try_send_append_sync to match
try_send_append’s spill-first behavior: attempt
overflow_for(shard_id).try_spill(...) before enqueueing, and retry spilling when
the append channel is full. Preserve each method’s existing acknowledgement and
error behavior while enabling durable EverySec and Always appends to use delayed
overflow acknowledgements.
- Around line 508-517: Update the dropped-append handling near the AOF
backpressure warning to call record_append_dropped(1) instead of incrementing
AOF_BACKPRESSURE_DROPPED directly, so the append status latch is updated. Apply
the same record_append_dropped(1) accounting to the disconnected-channel path.
- Around line 475-480: Update both spill-first append paths around spill_first
and try_spill to distinguish a full overflow buffer from a disarmed overflow.
When try_spill rejects because capacity is exhausted, preserve the spill-first
ordering by returning the failure without calling try_send_append_no_spill; only
fall back to the channel when the overflow is disarmed.
In `@src/persistence/aof/rewrite_overflow.rs`:
- Around line 159-248: Track the number of spilled messages that remain
undrained when the closure in the rewrite overflow drain returns an error,
rather than allowing them to be dropped unaccounted. Expose that count to the
outer error path and call super::record_append_dropped for the undrained
remainder, preserving the existing logging and reset behavior while ensuring
failed drains update the dropped-write metrics.
In `@src/persistence/aof/writer_task.rs`:
- Around line 581-586: The overflow-drain error handlers around
overflow.finish_raw and the listed analogous sites must not continue processing
after a write failure. Transition the writer into its non-acknowledging failed
state, and either return the uncommitted spill records or call
record_append_dropped for every record that cannot be recovered before exiting
the rewrite flow.
- Around line 840-848: The overflow-drain blocks around finish_raw must not lose
newly queued Rewrite or RewriteSharded control messages. After
overflow.finish_raw completes, explicitly clear AOF_REWRITE_IN_PROGRESS as
needed and preserve/reprocess any rewrite request consumed during draining so a
subsequent rewrite is started instead of leaving the flag set without an active
rewrite; apply the same fix to both drain paths.
In `@src/persistence/wal_v3/replay.rs`:
- Around line 408-413: Update the WAL replay handling around the zero-byte
`data` check so an empty segment is accepted only when it is the final segment;
when later `.wal` segments exist, apply the existing default-fail or
explicit-salvage behavior instead of returning `torn: false`. Add coverage for
an empty `000000000002.wal` between valid first and third segments.
- Around line 342-366: Move the existing stop_at_lsn cutoff check in the WAL
replay flow before the mid-chain tear handling around result.torn, so replay
returns successfully once the valid prefix reaches the target and does not
inspect later corruption. Keep InvalidData rejection for tears only when records
beyond the tear are required, and add a regression test covering a target
record, a following corrupt record, and a later segment that verifies only the
target prefix is applied.
In `@src/replication/reason_del.rs`:
- Around line 253-259: Update the drop-accounting flow around
record_reason_del_dropped, record_bytes_conn, and record_effect_write so
reason-DEL drops log only the relevant key, not the serialized RESP payload. Add
or reuse a sibling accounting helper that shares the counter and latch but emits
a caller-neutral message with record length for generic/script-effect drops.
Ensure record_bytes_conn selects the appropriate helper based on its caller
context.
- Around line 139-147: Update record_reason_del_dropped to remove the truncated
raw-byte value from the tracing::error! call and log only a non-reversible
identifier, such as a stable hash of the provided key/serialized record.
Preserve the existing dropped counter and saturation error context, ensuring no
key or value bytes are emitted at error level.
In `@tests/recovery_matrix_w1.rs`:
- Around line 215-222: Update the recovery test’s flood coordination before
SIGKILL so it retries or continues until the persistence diagnostic
aof_rewrite_overflow_spilled reports a value greater than zero. Parse and assert
that spill count before killing the process, ensuring the test exercises the
overflow path rather than merely validating normal recovery.
---
Outside diff comments:
In `@src/persistence/aof/writer_task.rs`:
- Around line 1324-1332: Update the flush-error branch in the writer loop around
writer.flush() so overflow records produced after overflow.arm() are never
discarded while the writer remains active. If the old incremental file is still
usable, drain overflow into it; otherwise latch the writer failure and stop
accepting successful appends, ensuring no +OK append can be lost before the loop
exits.
---
Nitpick comments:
In `@src/persistence/aof/rewrite_overflow.rs`:
- Around line 99-120: Document in the overflow buffer implementation that the
max_bytes limit enforced by try_spill is an approximate soft cap, since
accounting covers payload bytes but excludes framing, SELECT prefixes, and
Vec<AofMessage> overhead. Do not change the existing byte accounting or spill
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e4623a4d-0ff5-4e4c-9068-d2c048158ee7
📒 Files selected for processing (14)
CHANGELOG.mdsrc/command/connection.rssrc/command/persistence.rssrc/persistence/aof/group_commit.rssrc/persistence/aof/mod.rssrc/persistence/aof/pool.rssrc/persistence/aof/rewrite.rssrc/persistence/aof/rewrite_overflow.rssrc/persistence/aof/writer_task.rssrc/persistence/wal_v3/replay.rssrc/replication/reason_del.rssrc/shard/spsc_handler.rstests/aof_auto_rewrite.rstests/recovery_matrix_w1.rs
Comment on lines
+342
to
+366
| if result.torn && idx + 1 < segments.len() { | ||
| // Mid-chain tear: records beyond the hole exist in later | ||
| // segments (#452.2). | ||
| if salvage { | ||
| tracing::error!( | ||
| "WAL v3 replay: mid-chain tear in {} with {} later segment(s) — \ | ||
| MOON_WAL_SALVAGE=1 set, CONTINUING past the hole; recovered state \ | ||
| may be missing acked writes that later operations depended on", | ||
| seg_path.display(), | ||
| segments.len() - idx - 1, | ||
| ); | ||
| } else { | ||
| return Err(std::io::Error::new( | ||
| std::io::ErrorKind::InvalidData, | ||
| format!( | ||
| "WAL v3 mid-chain tear: segment {} ends in a corrupt/truncated record \ | ||
| but {} later segment(s) exist — applying them would silently replay \ | ||
| operations from after a hole. Refusing to recover. Restore from a \ | ||
| backup/replica, or set MOON_WAL_SALVAGE=1 to accept partial recovery.", | ||
| seg_path.display(), | ||
| segments.len() - idx - 1, | ||
| ), | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check stop_at_lsn before the mid-chain tear error.
If the valid prefix already reaches stop_at_lsn, this code returns InvalidData for a later corrupt record that the recovery target does not require. Move the existing cutoff check before the mid-chain tear check. Reject the replay only when the target requires records after the tear.
Add a test with a valid record at the target, a corrupt following record, and a later segment. The replay should succeed and apply only the target prefix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/persistence/wal_v3/replay.rs` around lines 342 - 366, Move the existing
stop_at_lsn cutoff check in the WAL replay flow before the mid-chain tear
handling around result.torn, so replay returns successfully once the valid
prefix reaches the target and does not inspect later corruption. Keep
InvalidData rejection for tears only when records beyond the tear are required,
and add a regression test covering a target record, a following corrupt record,
and a later segment that verifies only the target prefix is applied.
Comment on lines
+408
to
+413
| // A zero-byte file is a benign fresh segment; a partial header is a | ||
| // tear (#452.2). | ||
| return Ok(WalV3ReplayResult { | ||
| torn: !data.is_empty(), | ||
| ..WalV3ReplayResult::default() | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject a zero-byte segment when later segments exist.
A zero-byte file returns torn: false, so directory replay silently applies later segments after an empty middle segment. A zero-byte file is only a benign fresh segment when it is final. If later .wal files exist, treat the empty file as a mid-chain gap and apply the same default-fail or explicit-salvage policy.
Add coverage for 000000000002.wal being empty between valid first and third segments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/persistence/wal_v3/replay.rs` around lines 408 - 413, Update the WAL
replay handling around the zero-byte `data` check so an empty segment is
accepted only when it is the final segment; when later `.wal` segments exist,
apply the existing default-fail or explicit-salvage behavior instead of
returning `torn: false`. Add coverage for an empty `000000000002.wal` between
valid first and third segments.
Comment on lines
+139
to
147
| fn record_reason_del_dropped(key: &[u8]) { | ||
| crate::persistence::aof::AOF_REASON_DEL_DROPPED | ||
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | ||
| crate::persistence::aof::AOF_LAST_APPEND_OK.store(false, std::sync::atomic::Ordering::Relaxed); | ||
| tracing::error!( | ||
| "reason-DEL LOST for key {:?}: AOF writer saturated past the escalated bound — \ | ||
| restart replay will RESURRECT this key unless a rewrite completes first", | ||
| String::from_utf8_lossy(&key[..key.len().min(64)]), | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log raw key bytes at error level.
record_reason_del_dropped writes up to 64 bytes of the key into the log. Keys frequently contain user identifiers or other personal data, so this exports PII into log sinks on a path that fires under writer saturation (potentially at high volume). Log a non-reversible identifier instead, and keep the counter as the alerting signal.
Note: record_bytes_conn passes a full serialized RESP record here, not a key, so the current form can also emit value bytes. See the separate comment on lines 254-259.
🔐 Proposed fix: hash the key instead of printing it
fn record_reason_del_dropped(key: &[u8]) {
crate::persistence::aof::AOF_REASON_DEL_DROPPED
.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
crate::persistence::aof::AOF_LAST_APPEND_OK.store(false, std::sync::atomic::Ordering::Relaxed);
tracing::error!(
- "reason-DEL LOST for key {:?}: AOF writer saturated past the escalated bound — \
- restart replay will RESURRECT this key unless a rewrite completes first",
- String::from_utf8_lossy(&key[..key.len().min(64)]),
+ "reason-DEL LOST (key_len={}, key_hash={:016x}): AOF writer saturated past the \
+ escalated bound — restart replay will RESURRECT this key unless a rewrite \
+ completes first",
+ key.len(),
+ {
+ use std::hash::{Hash, Hasher};
+ let mut h = std::collections::hash_map::DefaultHasher::new();
+ key.hash(&mut h);
+ h.finish()
+ },
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn record_reason_del_dropped(key: &[u8]) { | |
| crate::persistence::aof::AOF_REASON_DEL_DROPPED | |
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | |
| crate::persistence::aof::AOF_LAST_APPEND_OK.store(false, std::sync::atomic::Ordering::Relaxed); | |
| tracing::error!( | |
| "reason-DEL LOST for key {:?}: AOF writer saturated past the escalated bound — \ | |
| restart replay will RESURRECT this key unless a rewrite completes first", | |
| String::from_utf8_lossy(&key[..key.len().min(64)]), | |
| ); | |
| fn record_reason_del_dropped(key: &[u8]) { | |
| crate::persistence::aof::AOF_REASON_DEL_DROPPED | |
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | |
| crate::persistence::aof::AOF_LAST_APPEND_OK.store(false, std::sync::atomic::Ordering::Relaxed); | |
| tracing::error!( | |
| "reason-DEL LOST (key_len={}, key_hash={:016x}): AOF writer saturated past the \ | |
| escalated bound — restart replay will RESURRECT this key unless a rewrite \ | |
| completes first", | |
| key.len(), | |
| { | |
| use std::hash::{Hash, Hasher}; | |
| let mut h = std::collections::hash_map::DefaultHasher::new(); | |
| key.hash(&mut h); | |
| h.finish() | |
| }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/replication/reason_del.rs` around lines 139 - 147, Update
record_reason_del_dropped to remove the truncated raw-byte value from the
tracing::error! call and log only a non-reversible identifier, such as a stable
hash of the provided key/serialized record. Preserve the existing dropped
counter and saturation error context, ensuring no key or value bytes are emitted
at error level.
Comment on lines
253
to
+259
| if let Some(pool) = aof_pool { | ||
| let mut budget = AOF_SPSC_BACKPRESSURE_BOUND; | ||
| let _ = pool.send_append_bounded_blocking(shard_id, 0, db, bytes, &mut budget); | ||
| // #452.4: escalated bound + fail-loud accounting — see | ||
| // `record_reason_del`'s comment for the resurrection rationale. | ||
| let mut budget = crate::persistence::aof::AOF_REASON_DEL_BACKPRESSURE_BOUND; | ||
| if !pool.send_append_bounded_blocking(shard_id, 0, db, bytes.clone(), &mut budget) { | ||
| record_reason_del_dropped(&bytes); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass a key, not the serialized record, and use a caller-neutral message.
record_bytes_conn calls record_reason_del_dropped(&bytes). bytes is the complete serialized RESP record, not a key. Two consequences:
- The log line claims "reason-DEL LOST for key <...>" while printing the whole command frame, including argument values. That widens the data exposure described in the comment on lines 139-147.
record_bytes_connalso servesrecord_effect_write, which carries script write effects, not reason-DELs. The message misattributes those drops.
Split the accounting from the message, or pass an explicit context label plus the key where one exists.
♻️ Proposed shape
- if let Some(pool) = aof_pool {
- // `#452.4`: escalated bound + fail-loud accounting — see
- // `record_reason_del`'s comment for the resurrection rationale.
- let mut budget = crate::persistence::aof::AOF_REASON_DEL_BACKPRESSURE_BOUND;
- if !pool.send_append_bounded_blocking(shard_id, 0, db, bytes.clone(), &mut budget) {
- record_reason_del_dropped(&bytes);
- }
- }
+ if let Some(pool) = aof_pool {
+ // `#452.4`: escalated bound + fail-loud accounting — see
+ // `record_reason_del`'s comment for the resurrection rationale.
+ let mut budget = crate::persistence::aof::AOF_REASON_DEL_BACKPRESSURE_BOUND;
+ let record_len = bytes.len();
+ if !pool.send_append_bounded_blocking(shard_id, 0, db, bytes, &mut budget) {
+ record_conn_record_dropped(record_len);
+ }
+ }Add a sibling helper that shares the counter and latch but logs a record length instead of payload bytes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/replication/reason_del.rs` around lines 253 - 259, Update the
drop-accounting flow around record_reason_del_dropped, record_bytes_conn, and
record_effect_write so reason-DEL drops log only the relevant key, not the
serialized RESP payload. Add or reuse a sibling accounting helper that shares
the counter and latch but emits a caller-neutral message with record length for
generic/script-effect drops. Ensure record_bytes_conn selects the appropriate
helper based on its caller context.
Comment on lines
+215
to
+222
| // Diagnostics (not an assert: older binaries lack the field): how much | ||
| // the rewrite overflow actually spilled this run. | ||
| let spilled = cli(port, &["INFO", "persistence"]) | ||
| .lines() | ||
| .find(|l| l.starts_with("aof_rewrite_overflow_spilled:")) | ||
| .map(|l| l.to_string()) | ||
| .unwrap_or_else(|| "aof_rewrite_overflow_spilled:<absent>".into()); | ||
| eprintln!("[recovery_matrix_w1] {spilled}"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Assert that this test entered the overflow path.
aof_rewrite_overflow_spilled is diagnostic only. If the rewrite finishes before the flood saturates the writer channel, this test validates normal recovery and can pass without exercising the regression.
Coordinate or retry the flood until the spill count is greater than zero, then assert it before SIGKILL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/recovery_matrix_w1.rs` around lines 215 - 222, Update the recovery
test’s flood coordination before SIGKILL so it retries or continues until the
persistence diagnostic aof_rewrite_overflow_spilled reports a value greater than
zero. Parse and assert that spill count before killing the process, ensuring the
test exercises the overflow path rather than merely validating normal recovery.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+58
to
+61
| armed: std::sync::atomic::AtomicBool, | ||
| /// Spilled messages in enqueue order. Only `Append` / `AppendSync` are | ||
| /// ever pushed. | ||
| buf: parking_lot::Mutex<Vec<AofMessage>>, |
There was a problem hiding this comment.
1. rewriteoverflow uses mutex 📘 Rule violation ➹ Performance
RewriteOverflow buffers per-append spill messages via a mutex-protected Vec, which introduces lock-based producer→consumer handoff on the append path during rewrites. This violates the hot-path requirement to use lock-free flume channels for pipeline communication and can add contention under sustained load.
Agent Prompt
## Issue description
`RewriteOverflow` uses `parking_lot::Mutex<Vec<AofMessage>>` as a queue between producers and the writer during rewrite folds. The compliance rule requires lock-free `flume::Sender`/`Receiver` (or equivalent) for hot-path pipeline messaging.
## Issue Context
This buffer is fed per-append when rewrites are in progress (and potentially when `spill_first()` is true), so it is part of the request/write hot path under sustained load.
## Fix Focus Areas
- src/persistence/aof/rewrite_overflow.rs[55-65]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
TinDang97
added a commit
that referenced
this pull request
Aug 8, 2026
…sarial review Pre-merge hardening of the #452/#54 durability wave, addressing every finding from the adversarial review of PR #454: - P0.1 snapshot cut: RewriteOverflow::mark_cut records the buffer length at the fold's atomic snapshot instant (AofFold handler for sharded folds, under all db write guards for the single-writer fold). A committed finish discards [0..cut) — those entries' effects are in the new base, and replaying them onto it would double-apply non-idempotent commands (INCR/APPEND/LPUSH). Aborted finishes write everything (the rolled-back old base predates all spills). Parked AppendSync acks for discarded pre-cut entries resolve Synced after the finish fsync. - P0.2 producer gating: every enqueue leg (try_send_append, send_append_bounded_blocking, send_append_backpressure, try_send_append_sync) checks spill_first BEFORE touching the channel — the fold's own phase-1/3 drains free slots mid-fold, so an ungated producer could place a newer record ahead of older buffered ones. - Cap semantics: try_spill returns SpillReject::Disarmed(msg) (fall through to the normal path) vs CapExceeded (record the loss and drop — inserting into the channel while the buffer is non-empty would invert same-key replay order). - U1 unwind safety: arm_scoped returns an ArmGuard; a fold panic (release profile unwinds) disarms with drop accounting instead of leaving the overflow armed forever silently eating up to 256 MiB. - P1.4: boot callers now classify mid-chain tears (is_mid_chain_tear) and abort startup (exit 70) instead of falling back to legacy recovery, at all four WAL v3 replay sites. - P2.5: all cap/shutdown/disconnect drop paths route through record_append_dropped so the degraded latch can never miss a loss. - P2.6: finish drain is two-phase — channel (bounded by its length, outside the buffer lock) before buffer, disarm in the empty-swap round under the lock; bytes not being reset per swap keeps spill_first true across rounds. - P2.7: a finish that observes shutdown_requested warns loudly. - P2.8: reason-DEL backpressure budget is shared per eviction/expiry sweep (one 500ms bound total), not minted per victim key — a 1000-victim OOM sweep against a hung disk used to block the shard event loop for 1000 x 500ms. Red/green coverage: snapshot-cut discard/abort/rearm, gated backpressure + AppendSync spill paths, ArmGuard unwind + no-op-after- finish, shared-sweep budget fast-fail, and is_mid_chain_tear classification pinned in the existing tear test. Refs: #452 #54, PR #454 review author: Tin Dang
…rites mid-fold (#452.1) While an AOF rewrite fold runs, the writer thread is out of its recv loop, so the bounded (10k) append channel saturates under sustained write load and try_send_append / send_append_bounded_blocking dropped acked records — lost even on a clean restart. #433 made rewrites automatic, so the trigger condition became default-on in production. Fix: a per-writer RewriteOverflow spill buffer (aof_rewrite_buf equivalent), armed by the writer task around every rewrite fold: - Producers spill channel-overflow appends into the buffer instead of dropping (capped at 256 MiB; beyond the cap the pre-existing fail-loud drop path is unchanged). - Ordering invariant: once spilling, keep spilling (a producer never bypasses older buffered items via a freed channel slot); at drain time the channel (older) is written before the buffer (newer), under the buffer lock end-to-end. - The writer drains channel-then-buffer into the committed incr immediately after the fold — all six fold arms wired (monoio/tokio × TopLevel Rewrite / RewriteSharded / per-shard), with the committed- generation file from the fold's phase-8 barrier, so an aborted rewrite drains into the rolled-back OLD incr correctly. - Spilled AppendSync acks park until the post-drain boundary fsync (issue #140 discipline) and resolve Synced/FsyncFailed. - Fatal writer-exit paths disarm loudly (records counted as dropped, never silently buffered forever). - INFO persistence gains aof_rewrite_overflow_spilled. The stale "known limitation (F6 behind --experimental-per-shard- rewrite)" doc block is replaced with the closed-by-overflow contract. RewriteOverflow lives in its own module (rewrite.rs was at the 1500-line limit). Red/green: pool_bounded_blocking_spills_instead_of_ dropping_during_rewrite pins the old drop as baseline and the new spill as the contract, plus ordering, cap, disarm, and AppendSync-ack tests. Refs #452, #433, #54 author: Tin Dang
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/persistence/aof/pool.rs (1)
828-834: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the empty-arm
matchwithis_ok().The
Err(_) => {}arm carries no logic.if ... .is_ok()states the same intent and avoids a possibleclippy::single_matchdiagnostic, which the repository requires to stay at zero warnings.Also note that
self.overflow_for(shard_id)is resolved a second time here; the localovfbinding from line 813 is already in scope.♻️ Proposed refactor
Err(flume::TrySendError::Full(returned)) => { // `#452.1`: channel saturated — spill if a fold is in // progress (armed) before falling to the counted drop. - match self.overflow_for(shard_id).try_spill(returned) { - Ok(()) => return ack_rx, - Err(_) => {} - } + if ovf.try_spill(returned).is_ok() { + return ack_rx; + }As per coding guidelines: "Run and maintain formatting, clippy with zero warnings".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/persistence/aof/pool.rs` around lines 828 - 834, In the TrySendError::Full branch, replace the match around overflow_for(shard_id).try_spill(returned) with an is_ok() conditional that returns ack_rx on success and otherwise continues to the existing counted-drop path. Reuse the already scoped ovf binding instead of resolving self.overflow_for(shard_id) again.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/persistence/aof/pool.rs`:
- Around line 705-709: Update the disconnected-writer handling in the async
append paths around the sender try_send match and ack_rx resolution to call
super::record_append_dropped, matching send_append_bounded_blocking and
try_send_append_no_spill. Ensure dead-writer appends increment the drop metric
and propagate the failed append status instead of returning WriteFailed directly
or allowing RecvError without recording the drop.
In `@src/persistence/aof/rewrite_overflow.rs`:
- Around line 285-383: Track the number of records remaining in the local
spilled batch while the rewrite-overflow drain processes it, updating the
counter as each message is successfully handled. In the drain error path, add
this in-flight remainder to the records still in self.buf before calling
super::record_append_dropped, then clear both sources and preserve the existing
disarm/reset behavior so lost appends mark the failure status.
In `@src/shard/shared_databases.rs`:
- Around line 1205-1208: Update replay_graph_wal to apply the mid-chain tear
policy across graph WAL segments, preferably by using replay_wal_v3_dir or by
rejecting a torn non-final segment before replay continues. Add a regression
test covering a torn first segment followed by a valid segment, asserting
startup aborts and no graph commands are applied.
In `@src/shard/spsc_handler.rs`:
- Around line 959-962: Make reason_del_budget batch-scoped rather than
command-scoped in all four affected sites: src/shard/spsc_handler.rs lines
959-962, 1163-1166, 1620-1623, and 1825-1828. In each batch arm, move its
initialization beside aof_budget and before the corresponding command loop,
preserving one shared AOF_REASON_DEL_BACKPRESSURE_BOUND budget per batch.
---
Nitpick comments:
In `@src/persistence/aof/pool.rs`:
- Around line 828-834: In the TrySendError::Full branch, replace the match
around overflow_for(shard_id).try_spill(returned) with an is_ok() conditional
that returns ack_rx on success and otherwise continues to the existing
counted-drop path. Reuse the already scoped ovf binding instead of resolving
self.overflow_for(shard_id) again.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3983b34d-d343-4597-827c-dc3f7e247556
📒 Files selected for processing (13)
CHANGELOG.mdsrc/persistence/aof/pool.rssrc/persistence/aof/rewrite.rssrc/persistence/aof/rewrite_overflow.rssrc/persistence/aof/writer_task.rssrc/persistence/recovery.rssrc/persistence/wal_v3/replay.rssrc/replication/reason_del.rssrc/shard/mod.rssrc/shard/persistence_tick.rssrc/shard/shared_databases.rssrc/shard/spsc_handler.rssrc/shard/timers.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- src/replication/reason_del.rs
- src/persistence/aof/writer_task.rs
Comment on lines
+1205
to
+1208
| // #452.2: mid-chain tear ⇒ refuse to boot (see recovery.rs). | ||
| if crate::persistence::wal_v3::replay::is_mid_chain_tear(&e) { | ||
| crate::persistence::wal_v3::replay::abort_boot_on_mid_chain_tear(shard_id, &e); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Apply the mid-chain tear policy to legacy graph replay.
replay_graph_wal at Lines 1075-1094 replays each file with replay_wal_v3_file and ignores WalV3ReplayResult.torn. A torn non-final graph WAL segment can therefore continue into later segments. The collected graph commands then apply after a missing history range.
Use replay_wal_v3_dir in replay_graph_wal, or enforce the same non-final tear check there. Add a regression test with a torn first segment and a valid later segment. Verify that startup aborts and no graph commands apply.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shard/shared_databases.rs` around lines 1205 - 1208, Update
replay_graph_wal to apply the mid-chain tear policy across graph WAL segments,
preferably by using replay_wal_v3_dir or by rejecting a torn non-final segment
before replay continues. Add a regression test covering a torn first segment
followed by a valid segment, asserting startup aborts and no graph commands are
applied.
Comment on lines
+959
to
+962
| // #454 P2.8: ONE shared backpressure bound for this entire sweep | ||
| // (per-key minting could stall the shard bound x victim-count). | ||
| let mut reason_del_budget = | ||
| crate::persistence::aof::AOF_REASON_DEL_BACKPRESSURE_BOUND; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
reason_del_budget is minted per command instead of per batch in all four batch arms. Each arm already hoists aof_budget above its command loop so the batch shares one bound (the PR #211 fix). The new reason_del_budget is declared inside the loop, so an N-command batch with evict_active true can stall the shard event loop for N × AOF_REASON_DEL_BACKPRESSURE_BOUND.
src/shard/spsc_handler.rs#L959-L962: move the declaration next toaof_budgetat line 885, above the loop at line 886.src/shard/spsc_handler.rs#L1163-L1166: move the declaration next toaof_budgetat line 1088, above the loop at line 1089.src/shard/spsc_handler.rs#L1620-L1623: move the declaration next toaof_budgetat line 1546, above the loop at line 1547.src/shard/spsc_handler.rs#L1825-L1828: move the declaration next toaof_budgetat line 1750, above the loop at line 1751.
📍 Affects 1 file
src/shard/spsc_handler.rs#L959-L962(this comment)src/shard/spsc_handler.rs#L1163-L1166src/shard/spsc_handler.rs#L1620-L1623src/shard/spsc_handler.rs#L1825-L1828
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shard/spsc_handler.rs` around lines 959 - 962, Make reason_del_budget
batch-scoped rather than command-scoped in all four affected sites:
src/shard/spsc_handler.rs lines 959-962, 1163-1166, 1620-1623, and 1825-1828. In
each batch arm, move its initialization beside aof_budget and before the
corresponding command loop, preserving one shared
AOF_REASON_DEL_BACKPRESSURE_BOUND budget per batch.
…d reason-DEL bound (#452.4) send_append_bounded_blocking dropped acked records after budget exhaustion with only an error! and a rolling counter — nothing transitioned the server into a visible degraded state, and eviction/expiry reason-DELs took the same 5ms bound as ordinary appends even though a dropped reason-DEL makes restart replay RESURRECT a key clients were told is gone. - New sticky latch AOF_LAST_APPEND_OK, exposed as aof_last_append_status:{ok|err} in INFO persistence. Every drop site now routes through record_append_dropped() (counter + latch), including the rewrite-overflow fatal disarm path. Deliberately never resets: the AOF stays short one acked record for this generation's lifetime; the reset-on-clean-rewrite follow-up stays tracked in #452. - Reason-DELs get a 100x escalated bound (500ms, AOF_REASON_DEL_BACKPRESSURE_BOUND) — not unbounded (a dead writer must not hang the shard; a deferred-retry queue would reorder DEL-after-SET on replay) — and a dedicated fail-loud counter aof_reason_del_dropped + resurrection-warning error! at both emission contexts (event-loop and conn-handler legs). Red/green: dropped_append_latches_degraded_status (pool) and record_reason_del_drop_is_counted_and_latches_degraded (pays the real 500ms bound against a full capacity-1 channel). Refs #452 author: Tin Dang
…playing past the hole (#452.2) WAL v3 replay stopped at the first corrupt record WITHIN a segment but kept applying every LATER segment — silently replaying operations from after a hole (a lost SET followed by an applied INCR/DEL). Plain crash tails are safe (rotation fsyncs the old segment before opening the next), so a mid-chain tear can only mean on-disk corruption — the class InnoDB/RocksDB treat as fatal-by-default. - WalV3ReplayResult gains a `torn` flag, set on every torn-tail shape (corrupt record, truncated header tail, partial file header; a zero-byte segment stays benign). - replay_wal_v3_dir_until refuses (InvalidData, actionable message) when a torn segment is followed by later segments. A tear in the FINAL segment keeps the pre-existing keep-the-valid-prefix behavior. - MOON_WAL_SALVAGE=1 downgrades the abort to a loud error! and continues — operator-driven partial recovery only, never default. The internal _with_salvage variant takes the decision as a parameter so tests don't touch process-global env. Red/green: mid-chain tear aborts with only the pre-hole prefix applied (the pre-fix replay applied lsn 3 after the hole), salvage continues, tail tear stays benign. Full lib suite green (4514). Refs #452, #54 author: Tin Dang
…oad e2e (#54) Adds tests/recovery_matrix_w1.rs, the whole-stack leg for the #452 durability fixes: two connections flood >channel-capacity (30k) pipelined SET bursts against a live BGREWRITEAOF fold over a 600k-key dataset, then SIGKILL + restart and assert exact recovery (DBSIZE + spot keys), with a strict every-reply-must-be-+OK harness so the fail-loud -MOONERR drop path can never masquerade as success. Merge-base A/B on the dev host: main fails 1-2 of 3 runs (~5.6k bounded-blocking drops per hit on the cross-shard leg); the fixed binary is stably green on macOS and the Linux VM (monoio). The race is probabilistic on fast disks — the deterministic drop/spill contract is unit-level in rewrite_overflow; this is the regression net and the recovery-exactness proof. Harness notes: auto-rewrite is disabled because a post-flood auto fold would fold live memory into a fresh base and heal the loss before the kill, masking the bug. Also: CHANGELOG entry for the wave; #[allow(dead_code)] + doc for the pre-existing wait_for_compaction helper warning in aof_auto_rewrite (unblocks clippy --all-targets -D warnings). Refs #54, #452 author: Tin Dang
…sarial review Pre-merge hardening of the #452/#54 durability wave, addressing every finding from the adversarial review of PR #454: - P0.1 snapshot cut: RewriteOverflow::mark_cut records the buffer length at the fold's atomic snapshot instant (AofFold handler for sharded folds, under all db write guards for the single-writer fold). A committed finish discards [0..cut) — those entries' effects are in the new base, and replaying them onto it would double-apply non-idempotent commands (INCR/APPEND/LPUSH). Aborted finishes write everything (the rolled-back old base predates all spills). Parked AppendSync acks for discarded pre-cut entries resolve Synced after the finish fsync. - P0.2 producer gating: every enqueue leg (try_send_append, send_append_bounded_blocking, send_append_backpressure, try_send_append_sync) checks spill_first BEFORE touching the channel — the fold's own phase-1/3 drains free slots mid-fold, so an ungated producer could place a newer record ahead of older buffered ones. - Cap semantics: try_spill returns SpillReject::Disarmed(msg) (fall through to the normal path) vs CapExceeded (record the loss and drop — inserting into the channel while the buffer is non-empty would invert same-key replay order). - U1 unwind safety: arm_scoped returns an ArmGuard; a fold panic (release profile unwinds) disarms with drop accounting instead of leaving the overflow armed forever silently eating up to 256 MiB. - P1.4: boot callers now classify mid-chain tears (is_mid_chain_tear) and abort startup (exit 70) instead of falling back to legacy recovery, at all four WAL v3 replay sites. - P2.5: all cap/shutdown/disconnect drop paths route through record_append_dropped so the degraded latch can never miss a loss. - P2.6: finish drain is two-phase — channel (bounded by its length, outside the buffer lock) before buffer, disarm in the empty-swap round under the lock; bytes not being reset per swap keeps spill_first true across rounds. - P2.7: a finish that observes shutdown_requested warns loudly. - P2.8: reason-DEL backpressure budget is shared per eviction/expiry sweep (one 500ms bound total), not minted per victim key — a 1000-victim OOM sweep against a hung disk used to block the shard event loop for 1000 x 500ms. Red/green coverage: snapshot-cut discard/abort/rearm, gated backpressure + AppendSync spill paths, ArmGuard unwind + no-op-after- finish, shared-sweep budget fast-fail, and is_mid_chain_tear classification pinned in the existing tear test. Refs: #452 #54, PR #454 review author: Tin Dang
…-order and accounting closures Second adversarial pass on the rewrite-overflow work refuted the first fix round with one new P0 and three narrower defects; all four are closed here (the two remaining architectural findings are tracked in issue #455): - P0 abort-treated-as-commit: do_rewrite_per_shard returned Ok(()) on the coordinator-ABORT path (rollback to the OLD incr after a sibling shard failed), and both per-shard writer arms mapped is_ok() to committed=true — finish then DISCARDED pre-cut spills whose effects exist only in the PRUNED base and resolved their parked AppendSync acks Synced. Permanent, acked data loss on any multi-shard fold abort under write load. The fold now returns Ok(bool) ("new generation committed") and the arms pass matches!(res, Ok(true)). - Same-key replay inversion in the empty-buffer finish window: an entry admitted to the channel after finish captured its bound (only possible while the buffer is empty) was drained AFTER newer spilled entries. finish now re-drains the channel (bounded by its current length) before writing every swap round — channel entries at any swap instant are provably older than that round's buffer. - Ungated ordered leg: try_send_append_ordered (cross-shard TXN plumbing) bypassed the spill-first gate and dropped silently on Full/Disconnected. Now gated + cap/full/disconnect drops routed through record_append_dropped. - Taken-batch loss accounting: a mid-batch write error in the finish drain lost the swapped batch's un-written remainder without accounting (drain(..) drop cleared it; the outer error path only counts what is still in the buffer). The remainder is now counted before the error propagates. The stale "enqueued in-guard" claim on do_rewrite_single's mark_cut is corrected to document the residual mutation-to-enqueue suspension gap (issue #455), which also predates this wave on the channel-count side. Refs: #452 #54 #455, PR #454 re-verification author: Tin Dang
TinDang97
force-pushed
the
fix/w1-durability-452
branch
from
August 8, 2026 08:39
b5cf67e to
7a17104
Compare
TinDang97
added a commit
that referenced
this pull request
Aug 8, 2026
… writes (#456) Patch release rolling up the 2026-08 deep-review wave (#453) and durability wave 1 (#452/#54, PR #454), both merged after two-round adversarial review. Headline: AOF rewrites no longer drop acked writes under sustained pipelined load. A per-writer RewriteOverflow spill buffer (256 MiB cap, strict ordering, all six fold arms on both runtimes) buffers appends while the writer is mid-fold, with an exactly-once snapshot cut so a committed fold discards pre-snapshot spills (effects live in the new base) and an aborted fold writes everything. Merge-base A/B: main lost ~5.6k acked writes per hit; fixed is exact across SIGKILL + recovery (251k appends through the overflow in the release-gate e2e). The re-verify round closed an abort-treated-as-commit P0, a same-key replay-inversion window, an ungated ordered-append leg, and taken-batch loss accounting; residual architectural findings tracked in #455. Also: WAL v3 mid-chain tears abort boot (exit 70; MOON_WAL_SALVAGE=1 override) instead of silently replaying past a hole; sticky aof_last_append_status + per-writer aof_last_fsync_status latches; reason-DEL escalated backpressure with one shared bound per eviction sweep; manifest-sync failure latch; failed-spill victim re-insert; eviction metadata widened (LFU decay, LRU inversion, OBJECT IDLETIME wrap, WATCH ABA); cluster election acks received + inline fast path disabled in cluster mode; CLIENT TRACKING max_keys enforced. Validation: fmt + clippy x2 feature sets; macOS monoio 4541 + tokio 3705 and VM Linux 4563 lib tests; rewrite-under-pipelined-load e2e; PR CI green on both PRs pre-merge; crash-matrix nightly + ITERS=20 soak dispatched on the RC (b346910), green before tag. Rolls CHANGELOG [Unreleased] into [0.8.5], bumps Cargo.toml/lock, adds the RELEASES.md row, updates the README milestone table. author: Tin Dang
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wave 1 of the stable-release plan: the only open defect class that loses acked data. Closes the three durability items from #452 that became urgent when #433 made AOF rewrites automatic, plus the #54 recovery-matrix e2e leg that proves them.
Fixes
1. Rewrite-window append drops (#452.1) —
RewriteOverflowWhile a rewrite fold runs, the AOF writer thread is out of its recv loop, so the bounded (10k) append channel saturates under sustained pipelined writes and acked records were dropped — lost even on a clean restart. Since #433, rewrites fire automatically under exactly that load.
The fix is an
aof_rewrite_bufequivalent: a per-writerRewriteOverflowspill buffer, armed by the writer around every fold (all six arms: monoio/tokio × TopLevel Rewrite / RewriteSharded / per-shard):AppendSyncacks park until the post-drain boundary fsync (AOF: AppendSync acks Synced before rewrite-boundary fsync (appendfsync=always durability gap) #140 discipline).INFO persistencegainsaof_rewrite_overflow_spilled.2. Degraded-state latch + reason-DEL escalation (#452.4)
aof_last_append_status:errinINFO persistence— operators can alert on a health bit instead of diffing counters. Deliberately never resets (the AOF stays short one acked record for this generation's lifetime; reset-on-clean-rewrite is tracked in Durability follow-ups from 2026-08 deep review: rewrite-window append drops, WAL mid-chain tear policy, checkpoint data-file fsync #452).aof_reason_del_droppedcounter: a dropped reason-DEL makes restart replay resurrect data clients were told is gone. Not unbounded (a dead writer must not hang the shard) and not deferred-retry (would reorder DEL-after-SET on replay).3. WAL v3 mid-chain tear policy (#452.2)
Replay stopped at the first corrupt record within a segment but kept applying later segments — silently replaying operations from after a hole. Rotation fsyncs a segment before opening the next, so a mid-chain tear can only mean on-disk corruption (the class InnoDB/RocksDB treat as fatal-by-default). Replay now refuses (
InvalidDatawith an actionable message) when a torn segment has later segments;MOON_WAL_SALVAGE=1is the explicit operator override; a torn FINAL segment stays the benign crash-tail it always was.Verification
tests/recovery_matrix_w1.rs, new): 600k-key preload, two connections flood 30k-command pipelined bursts against a live BGREWRITEAOF fold, SIGKILL + restart, exact-recovery asserts with a strict every-reply-must-be-+OKharness. main fails 1–2 of 3 runs (~5.6k drops per hit); this branch is stably green on macOS and the Linux VM. The race is probabilistic on fast disks — the deterministic drop/spill/ordering contract is pinned by unit tests inrewrite_overflow.-MOONERR).cargo fmt --check;cargo clippy --all-targets -- -D warningson both feature sets (also fixes the pre-existingwait_for_compactiondead-code warning that blocked all-targets clippy); lib suites: 4514 (macOS monoio) / 3679 (tokio+jemalloc) / 4536 (Linux VM monoio); unit red/green for every behavioral change (spill ordering, cap, disarm, AppendSync acks, degraded latch, reason-DEL accounting, tear-policy × salvage × tail-tear).Refs #452, #54, #433. Follow-ups deliberately out of scope: latch reset on clean rewrite, checkpoint data-file fsync (#452 item 3), tracking expiry invalidation (#452 item 5 — wave 2).
author: Tin Dang
Summary by CodeRabbit
Reliability
Monitoring
Recovery
MOON_WAL_SALVAGE=1.