Skip to content

feat(replication): Wave A plane replication — eviction/expiry DELs + Lua write effects on both planes#285

Merged
pilotspacex-byte merged 7 commits into
mainfrom
feat/v0.7-plane-repl-wave-a
Jul 11, 2026
Merged

feat(replication): Wave A plane replication — eviction/expiry DELs + Lua write effects on both planes#285
pilotspacex-byte merged 7 commits into
mainfrom
feat/v0.7-plane-repl-wave-a

Conversation

@pilotspacex-byte

@pilotspacex-byte pilotspacex-byte commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Wave A of task #34 (RFC .planning/rfcs/plane-replication-design.md): the three GA-blocking write planes that mutated replicated state invisibly now emit records to BOTH planes (AOF + replication). Until this PR, a replica silently diverged — and in two cases the MASTER itself lost data across restart:

Plane Before this PR
Eviction plain-drops Replica keeps every key the master evicted (unbounded DBSIZE drift under memory pressure); evicted keys resurrect from AOF replay after restart
Active-expiry DELs Replica relies on its own clock/sweep; master emitted nothing
Lua redis.call write effects Reached neither plane: EVAL writes vanished on kill-9 restart AND never reached a replica (EVAL carries no WRITE flag; the bridge had no AOF/replication hook at all)

Design

  • record_reason_del (new src/replication/reason_del.rs): db-aware DEL record emitted at every genuine plain-drop — active-expiry sweep (expire_cycle gained a removed-key sink), background eviction tick, memory-pressure fallback, and the three write-path eviction gates (inline, generic, SPSC cross-shard). Same mechanics as ordinary writes: fused SELECT at num_shards>1, backlog append + shard-offset advance + deferred ReplicaLiveFanout, plus the AOF leg. Spill paths never emit — a key that lands in the cold tier is not a delete; is_plain_drop is computed from whether a durable spill actually happened (not from whether a spill context was passed — non-string victims can't spill and ARE plain drops).
  • Lua effects replication (src/scripting/bridge.rs): after each WRITE-flagged inner command returns non-error, the bridge emits the command verbatim to both planes (record_effect_write). EVAL/EVALSHA deliberately stay un-WRITE-flagged (pinned by a metadata unit test) so the generic gate can't double-log — eval_incr_no_double_apply proves exactly-once end to end. Partial scripts record their completed writes (effects semantics). The bridge's own bystander-eviction gate reports through the same helper.
  • SELECT inside scripts now errors loudly. Previously it silently wrote to the WRONG db (dispatch mutated the db index while the script kept writing the original Database). Real multi-db scripts are follow-up task chore(deps): bump tokio from 1.50.0 to 1.51.0 in the async-runtime group #38.
  • Fast-path preserved: all three emission helpers check "any replica/backlog/AOF wired?" BEFORE serializing — with nothing attached the new hooks cost one relaxed atomic load, no allocation.
  • Found and fixed en route: the monoio inline-protocol dispatch never fed the replication backlog (AOF-only) — inline writes now fall back to the full path once a replica has ever attached (fanout_hint_active).

Verification (red/green + standing replication gates)

  • RED first: 8 black-box scenarios written and confirmed failing on the feature assertions before any implementation (eviction parity s1+s4, EVAL parity s1+s4, EVAL kill-9 durability, evicted-key AOF resurrection, expiry offset-advance, SELECT-in-script) + 3 added during review (hash-eviction-under-disk-offload s1+s4, EVAL INCR no-double-apply). All 11 green on macOS AND Linux VM (3 consecutive sequential VM runs).
  • Adversarial review round (independent agent) found 3 real defects — non-string spill under-reporting (P0), allocate-before-gate, unwired Lua bystander gate — all fixed with their own red/green tests before this PR.
  • Regression: replication_multishard 9/9 · replication_streaming 7/7 · replication_hardening 5/5 · aof_multidb_kill9 4/4 · oom_bypass_closure 8/8 (spill ordering unregressed) — macOS + VM. Lib tests 4121 (monoio) + 3320 (tokio); clippy -D warnings both matrices; fmt.
  • VM A/B bench (OrbStack Linux, main 3e3b55a vs branch; s1+s4 × no-replica + replica-attached; 2 full reps + 3 targeted interleaved reps of the noisiest cell): no regression on any scenario (s4 replica SET P16 initially looked −18% but interleaved reps show identical distributions — VM noise). Exact master/replica DBSIZE parity in every replica-attached run.

Test-harness fixes shipped alongside (found by the VM runs)

  • tests/replication_planes.rs initially used fixed ports + spawned children before Guard construction; with SO_REUSEPORT a leaked server silently shares the port with the next run (three masters on one port observed). Now uses the test(harness): TOCTOU-safe shared port/spawn helpers across all 33 server-spawning suites #284 spawn_listening helpers.
  • Five scenario tails called guard.0.clear() — which drops the Child handles WITHOUT killing (std Child has no kill-on-drop), so every replica-attached test leaked both servers on both platforms. Removed; suite verified zero-leak.
  • expiry_del_propagates de-raced (200ms TTL raced replica catch-up under load; now 3s with an explicit baseline-capture guard).

Known limitations (documented in CHANGELOG)

  • redis.pcall('SELECT', ...) swallows the new error into a Lua table (loud only via redis.call).
  • Nondeterministic write commands inside scripts (SPOP etc.) replicate verbatim — pre-existing verbatim-replication class, first exposed for Lua here.
  • fanout_hint_active is sticky: once any replica has attached, the inline-protocol write fast path stays off for the process lifetime.
  • Lazy-expiry-on-read and hash-field TTL reaps don't emit (replica runs the same sweeps; bounded divergence, Redis-style batching).
  • Under runtime-tokio the new emission hooks are no-ops (master-side replication is monoio-only; pre-existing).

Wave B (WS./MQ. deterministic effect records) remains under task #34.

Summary by CodeRabbit

  • New Features

    • Lua script write effects now replicate and persist reliably through AOF.
    • Scripts using SELECT now fail clearly to prevent unintended database changes.
    • Expiration and memory-eviction deletions are consistently reflected across replicas and persistence.
  • Bug Fixes

    • Fixed incorrect handling of non-string values during disk-offload eviction.
    • Prevented evicted or expired keys from reappearing after restart.
    • Prevented duplicate application of writes issued from scripts.
  • Tests

    • Added coverage for replication, restart recovery, expiration, eviction, and scripting behavior.

8 black-box tests, all failing on the missing features by design:
eviction parity (s1+s4), EVAL effects parity (s1+s4), EVAL-write AOF
durability across kill-9, evicted-key resurrection from AOF replay,
expiry DEL emission (offset must advance), SELECT-in-script loud error.

author: Tin Dang <tindang.ht97@gmail.com>
…n-drops

Wave A part 1 (task #34): a key removed by active TTL expiry or by
memory-pressure eviction previously vanished from the master's own
keyspace without telling either durability plane. An attached replica
kept serving the stale key forever, and a kill -9 + restart against
--appendonly yes resurrected it from the AOF replay (the AOF recorded
the original SET but never the removal).

Add src/replication/reason_del.rs with two call shapes for the two
places a reason-based removal happens:

- record_reason_del — shard-event-loop context (background active
  expiry, background eviction tick). Reuses
  shard::spsc_handler::wal_append_and_fanout verbatim, the same
  mechanism ShardMessage::SwapDb already uses for a synthetic write
  with no originating client command: one call does the WAL v3 log,
  the replication backlog append + offset advance + deferred live
  fan-out, and the AOF pool append.

- record_reason_del_conn — connection-handler context (the monoio
  inline SET fast path and the generic per-command write-eviction
  gate). Mirrors handler_monoio::ft::record_local_write_db's mechanics
  (fused SELECT for multi-shard masters, emit-on-change SELECT
  tracking for single-shard masters, backlog append + offset advance
  + deferred ShardMessage::ReplicaLiveFanout push) and adds the AOF
  leg record_local_write_db omits, since a synthetic eviction DEL has
  no client command to hang the existing per-command AOF gate off of.

Wire both flavors into every genuine plain-drop site, while leaving
every hot-to-cold spill/tiering path emitting nothing (a spilled key
stays cold-readable, not deleted):

- server/expiration.rs: expire_cycle / expire_cycle_direct gain a
  removed-key sink parameter, invoked only for the whole-key TTL sweep
  (hash-field TTL reaps are out of scope for this wave).
- shard/timers.rs: run_active_expiry and run_eviction thread the
  shard-loop's WAL/backlog/fanout/AOF handles through to the sink.
- shard/persistence_tick.rs: run_eviction_tick and
  handle_memory_pressure thread the same handles to the sync-spill
  fallback branch that has no manifest (a true plain-drop); the
  durable-manifest branch is left calling the original non-reporting
  function since that victim is never actually deleted.
- storage/eviction.rs: evict_one_with_spill gains an on_plain_drop
  sink, fired only when spill was None for that victim (captured
  before spill is consumed). try_evict_if_needed_budget,
  try_evict_if_needed_with_spill_and_total_budget, and
  try_evict_if_needed_async_spill_with_total_budget each split into a
  _reporting variant plus a thin no-op-sink wrapper preserving the
  original public signature, so out-of-scope callers (Lua bridge,
  tokio-only handlers, storage/db_quota.rs's per-db quota path,
  shard/spsc_two_db.rs's cross-db COPY destination eviction) need no
  changes.
- server/conn/blocking.rs (try_inline_dispatch) and
  server/conn/handler_monoio/mod.rs (run_write_eviction_gate): the two
  connection-handler-context write-eviction gates now emit via
  record_reason_del_conn.
- shard/spsc_handler.rs: spsc_eviction_gate gains the same sink
  parameter; all 6 cross-shard write call sites wire it to
  record_reason_del.

Also fixes a pre-existing replication gap uncovered while chasing the
eviction-parity tests: try_inline_dispatch's monoio SET fast path only
ever fed the AOF, never the replication backlog/fan-out at all (an
attached replica never saw a lone `SET foo bar` under
--disk-offload disable, independent of eviction). can_inline_writes
now also requires !fanout_hint_active(), falling back to the fully-
replicating generic dispatch path once any replica has ever attached
— the same "fall back once the fast path can't do enough" precedent
ctx.spill_sender.is_none() already established for disk-offload.

Turns 4 of the committed RED tests in tests/replication_planes.rs
green: eviction_parity_shards1, eviction_parity_shards4,
evicted_keys_stay_dead_after_restart, expiry_del_propagates. The 4
Lua/EVAL scenarios (eval_effects_parity_shards1/4,
eval_writes_survive_restart, select_in_script_errors) are untouched
and remain red, owned by a follow-up agent in src/scripting/.

Verified: cargo test --release --lib (4115 passed) and
cargo test --no-default-features --features runtime-tokio,jemalloc
--lib (3316 passed) both clean; cargo clippy -D warnings clean on
both feature matrices; cargo fmt --check clean; replication_multishard
(9/9), replication_streaming (7/7), replication_hardening (5/5),
aof_multidb_kill9 (4/4), and oom_bypass_closure (8/8) all still green.

Known follow-ups left as documented no-op gaps, out of scope for this
wave: hash-field TTL reaps, Lua write effects, storage/db_quota.rs's
per-db maxmemory eviction, and spsc_two_db.rs's cross-db COPY
destination eviction do not yet emit reason-DEL records.

Refs: task #34, .planning/rfcs/plane-replication-design.md (Wave A)

author: Tin Dang <tindang.ht97@gmail.com>
…ut (Wave A part 2)

Part 1 (f543183) gave background expiry/eviction plain-drops a dual-plane
DEL emission path but left Lua scripting untouched. Before this commit, a
Lua script's `redis.call`/`redis.pcall` write effects reached NEITHER
durability plane: EVAL/EVALSHA deliberately carry no WRITE command-metadata
flag, so the generic per-command AOF/replication gate in handler_monoio
never saw them. A script's writes vanished on kill -9 + restart against
--appendonly yes, and an attached replica never observed a script's writes
at all. Separately, redis.call('SELECT', ...) inside a script silently
corrupted state: dispatch's SELECT handler mutated only a local variable,
so every subsequent write in the script kept landing in the ORIGINAL db
while looking like it had switched.

scripting::bridge::make_redis_call_fn now records every successfully-
executed, WRITE-flagged inner command to both planes itself, immediately
after each redis.call/redis.pcall returns — not batched to script end, so a
script that writes two keys and then errors on a third still durably
records the first two. New replication::reason_del::record_effect_write
(sharing a record_bytes_conn core refactored out of part 1's
record_reason_del_conn) does the emission, reusing the identical fused-
SELECT/backlog/offset/fan-out mechanics every other write path already
uses, recording the verbatim cmd+args the script invoked.

LuaEvictionCtx (built once per shard at Lua-VM setup, already caching the
OOM eviction handles) now also carries num_shards/repl_state/aof_pool for
this emission. Every production construction site picked up the three new
arguments: the 4 call sites in shard::conn_accept (tokio, monoio, and their
migrated-connection twins) plus
server::conn::core::ConnectionContext::build_lua_eviction_ctx, which is
shared by EVAL/EVALSHA and FCALL/FCALL_RO — both route through the same
bridge closure, so FCALL gets the identical effect emission with no extra
wiring.

redis.call('SELECT', ...) inside any script now fails loud with "ERR SELECT
inside scripts is not supported by moon yet" instead of silently corrupting
state, intercepted before dispatch so no partial write from the same call
ever lands. A real multi-db-scripts feature is a follow-up.

Deliberately did NOT flip EVAL/EVALSHA to WRITE in the command metadata
table: that would route the literal "EVAL <script> ..." invocation through
the generic per-command AOF/replication gate ON TOP OF the effect records
above, double-applying every write the script made (e.g. an INCR landing as
2 instead of 1). This invariant is guarded by a new unit test
(command::metadata::eval_evalsha_never_write_flagged) and a black-box
regression test (eval_incr_no_double_apply). FCALL, unlike EVAL, IS
WRITE-flagged in the metadata table (mirrors upstream Redis Functions, where
FCALL always requires write permission and FCALL_RO is the read-only
variant) — that flag is confirmed to only feed ACL/READONLY-replica gating,
since try_handle_functions always consumes FCALL with `continue` before the
generic AOF/replication block runs, so it rides the same single-emission
bridge path without risk of a second recording.

Turns the 4 remaining RED tests in tests/replication_planes.rs green:
eval_effects_parity_shards1, eval_effects_parity_shards4,
eval_writes_survive_restart, select_in_script_errors. Added
eval_incr_no_double_apply as the targeted regression guard requested for
this wave. All 9 tests in the suite now pass (the 5 from part 1 plus this
new one).

Known gap, unchanged from before this commit: a write-EVAL issued directly
against a read-only replica is not rejected (try_enforce_readonly only
gates on the WRITE metadata flag, which EVAL intentionally lacks). This
cannot cause replication divergence on its own (replicas never receive EVAL
itself over the wire, only the effect records), but a client misdirected to
write against a replica directly could locally corrupt that replica's
state. Tracked as a follow-up, out of Wave A's scope per the RFC.

Verified: cargo test --release --lib (4115 passed) and cargo test
--no-default-features --features runtime-tokio,jemalloc --lib (3316
passed) both clean; cargo clippy -D warnings clean on both feature
matrices; cargo fmt --check clean. replication_planes 9/9,
replication_multishard 9/9, replication_streaming 7/7, replication_hardening
5/5, aof_multidb_kill9 4/4 all green. Scripting coverage: 13 EVAL/EVALSHA/
SCRIPT/sandbox integration tests (tests/integration.rs, runtime-tokio) and
9 FUNCTION/FCALL/FCALL_RO integration tests (tests/functions_fcall.rs)
green, confirming FCALL's shared bridge path is unaffected.

Refs: task #34, .planning/rfcs/plane-replication-design.md (Wave A)

author: Tin Dang <tindang.ht97@gmail.com>
…roof guards

The suite hardcoded ports 17301-17382 and spawned children before Guard
construction: a panic in await_ready leaked raw Children, and because moon
binds SO_REUSEPORT a leaked server silently SHARES the fixed port with the
next run and splits its connections (observed on the Linux VM: three
masters on port 17301 after two failed runs, cascading 5/9 failures).
Now spawns via common::spawn_listening into the guard immediately; only
the two same-port restart legs keep direct fixed-port spawns.

author: Tin Dang <tindang.ht97@gmail.com>
…ecord allocs

Three defects found in an adversarial review of Wave A (plane replication,
task #34), fixed with red/green tests where the code path is reachable via
a unit test, and black-box replica-convergence tests where it isn't.

1. Non-string eviction victims under a live `SpillContext` were dropped
   with no cold copy AND no DEL record. `storage::eviction::
   evict_one_with_spill` snapshotted `is_plain_drop = spill.is_none()`
   BEFORE its spill body ran, but that body only ever spills
   `RedisValueRef::String` values -- a Hash/List/Set/ZSet victim picked
   under a `SpillContext` took neither branch of the `is_string` check (no
   bytes written anywhere) yet fell through to the unconditional
   `db.remove` unreported, because `is_plain_drop` was already `false`.
   Fixed by tracking whether THIS victim was actually spilled (`spilled`,
   set only inside the branch that performs the write) instead of
   snapshotting the precondition. Also threaded the real
   `record_reason_del` sink into `persistence_tick::handle_memory_pressure`'s
   durable-spill branch, which previously passed a hardcoded no-op sink.

   Black-box testing this surfaced that the narrowly-described
   `SpillContext` path is dead code via any current CLI configuration
   (`spill_thread`/`shard_manifest` are co-gated on the same
   `disk_offload_enabled()` check). The two call sites an ordinary write
   actually reaches under `--disk-offload enable` --
   `handler_monoio::run_write_eviction_gate` and
   `spsc_handler::spsc_eviction_gate` -- had the identical unwired-sink bug
   in their spill-sender branch and are fixed the same way.

2. `record_reason_del`/`record_reason_del_conn`/`record_effect_write`
   allocated a fully serialized RESP record BEFORE checking whether any
   replica or AOF pool was even wired -- paid on every background-tick DEL
   and script write effect on the most common deployment shape (standalone,
   no replica, `--appendonly no`), only to be discarded a few instructions
   later. Fixed by hoisting the existing has-work predicate
   (`wal_fanout_has_work` / a new `conn_has_work`) to the top of all three
   entry points, before any serialization. Behavior when there IS work is
   unchanged (same predicate, same downstream call).

3. Lua's bystander-eviction gate (`scripting::bridge::LuaEvictionCtx::gate`)
   called the non-reporting eviction variants, so a script-triggered
   bystander eviction never reached the AOF/replication planes. New
   `try_evict_if_needed_async_spill_budget_reporting` wrapper; `gate` now
   threads a real sink through to `record_reason_del_conn`.

Verification: replication_planes (11/11, sequential, --test-threads 1;
one of four full-suite runs saw the pre-existing, untouched
`expiry_del_propagates` timing flake -- confirmed unrelated, same function
at base commit 152a39c, passes 3/3 in isolation), oom_bypass_closure
(8/8, no spill-path regression), cargo clippy -D warnings clean on both
the default and --no-default-features --features runtime-tokio,jemalloc
matrices, cargo fmt --check clean, cargo test --lib green on both matrices
(4121 passed / 3320 passed, 0 failed).

Also documents three known Wave A limitations in CHANGELOG.md:
redis.pcall('SELECT', ...) swallowing the rejection error, nondeterministic
write commands replicating verbatim from scripts, and fanout_hint_active's
sticky one-way-latch semantics.

author: Tin Dang <tindang.ht97@gmail.com>
…replica catch-up)

The 200ms TTL raced the replica's catch-up under load (~1-in-4 on a busy
host): keys expired before dbsize(replica) reached N, failing the SETUP
invariant, never the feature. TTL now 3s with an explicit guard that the
baseline offset was captured before any TTL could fire.

author: Tin Dang <tindang.ht97@gmail.com>
…ery replica-attached server

Five scenario tails cleared the Guard's Vec<Child> after passing: clearing
drops the Child HANDLES (std Child never kills on drop) so Guard::drop later
iterated an empty vec — every replica-attached test leaked both servers on
macOS AND Linux (48 orphans accumulated on the VM across three suite runs,
verified guarded pids == leaked pids, GUARD drop never logged). Suite green
11/11 with zero leaked processes after removal.

author: Tin Dang <tindang.ht97@gmail.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Wave A durability reporting for plain drops, expirations, evictions, and Lua write effects across replication and AOF paths. It also rejects SELECT inside scripts and adds unit, integration, restart, parity, and double-apply regression tests.

Changes

Wave A durability and replication

Layer / File(s) Summary
Plain-drop eviction reporting
src/storage/eviction.rs, src/storage/db_quota.rs
Adds reporting eviction APIs, correctly distinguishes spills from plain drops, and emits callbacks for victims without durable cold-tier copies.
DEL and script-effect emission
src/replication/reason_del.rs, src/replication/mod.rs
Adds shared replication/AOF emission for deletion records and verbatim Lua write effects, including backlog, offsets, fanout, and database selection handling.
Expiry and background eviction wiring
src/server/expiration.rs, src/shard/timers.rs, src/shard/persistence_tick.rs, src/shard/event_loop.rs
Threads durability context through active expiry, scheduled eviction, and memory-pressure cascades, reporting removed keys to both planes.
Connection and cross-shard write paths
src/server/conn/*, src/shard/spsc_handler.rs, src/shard/spsc_two_db.rs
Updates inline, monoio, SPSC, and cross-database eviction paths to use reporting callbacks or explicit no-op sinks.
Lua write-effect replication and SELECT rejection
src/scripting/bridge.rs, src/server/conn/core.rs, src/shard/conn_accept.rs
Passes replication/AOF handles into Lua contexts, reports bystander evictions, emits successful inner write effects, and rejects SELECT inside scripts.
Regression coverage and release notes
CHANGELOG.md, tests/replication_planes.rs, src/command/hash/mod.rs, src/command/metadata.rs
Documents Wave A behavior and adds coverage for expiry callbacks, metadata guards, replication parity, restart durability, SELECT rejection, and double application.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant LuaBridge
  participant Database
  participant DurabilityPlanes
  Client->>LuaBridge: EVAL or EVALSHA
  LuaBridge->>Database: execute inner redis.call write
  Database-->>LuaBridge: successful command result
  LuaBridge->>DurabilityPlanes: emit verbatim write effect
  LuaBridge->>DurabilityPlanes: report any plain-drop eviction
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main change: Wave A replication of eviction/expiry DELs and Lua write effects.
Description check ✅ Passed The PR description covers the required summary and notes sections with substantial detail; checklist and performance impact are missing but non-critical.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/v0.7-plane-repl-wave-a

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/shard/event_loop.rs (1)

1654-1660: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider hoisting the repeated wal_kv_log_mode → bool computation.

The same match wal_kv_log_mode { On => true, Off => false, Auto => !appendonly_enabled || !cdc_registry.is_empty() } is inlined at ~7 call sites in this function (run_active_expiry, run_eviction_tick, and the drain_spsc_shared calls). A local closure recomputed just before each dispatch (so it still reflects the live cdc_registry state) would remove the duplication and prevent the Auto formula from drifting across sites.

let wal_kv_log_now = || match wal_kv_log_mode {
    crate::config::WalKvLogMode::On => true,
    crate::config::WalKvLogMode::Off => false,
    crate::config::WalKvLogMode::Auto => !appendonly_enabled || !cdc_registry.is_empty(),
};
🤖 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/event_loop.rs` around lines 1654 - 1660, In the enclosing
event-loop function, hoist the repeated wal_kv_log_mode-to-bool match into a
local closure such as wal_kv_log_now, then replace the inlined matches at
run_active_expiry, run_eviction_tick, and each drain_spsc_shared dispatch with
calls to that closure. Invoke it immediately before each dispatch so Auto
continues reading the current cdc_registry state.
🤖 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 11-12: Update the Wave A summary in CHANGELOG.md to state “Four
defects” instead of “Three defects,” leaving the four documented defect entries
unchanged.
- Around line 157-160: Update the Wave-A-scoped gaps entry in the changelog to
remove the resolved Lua `redis.call` write-effects item, or explicitly qualify
it as a historical part-1 limitation; retain the other unresolved limitations
unchanged.
- Around line 207-211: Update the replication_planes.rs changelog entry to
report all 11 ignored scenarios, including the two disk-offload hash-parity
tests, instead of stating that all 9 tests pass.

---

Nitpick comments:
In `@src/shard/event_loop.rs`:
- Around line 1654-1660: In the enclosing event-loop function, hoist the
repeated wal_kv_log_mode-to-bool match into a local closure such as
wal_kv_log_now, then replace the inlined matches at run_active_expiry,
run_eviction_tick, and each drain_spsc_shared dispatch with calls to that
closure. Invoke it immediately before each dispatch so Auto continues reading
the current cdc_registry state.
🪄 Autofix (Beta)

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

Run ID: 2cc6d45b-765c-4ae7-b417-23547c54b6da

📥 Commits

Reviewing files that changed from the base of the PR and between 3e3b55a and d0c92fa.

📒 Files selected for processing (19)
  • CHANGELOG.md
  • src/command/hash/mod.rs
  • src/command/metadata.rs
  • src/replication/mod.rs
  • src/replication/reason_del.rs
  • src/scripting/bridge.rs
  • src/server/conn/blocking.rs
  • src/server/conn/core.rs
  • src/server/conn/handler_monoio/mod.rs
  • src/server/expiration.rs
  • src/shard/conn_accept.rs
  • src/shard/event_loop.rs
  • src/shard/persistence_tick.rs
  • src/shard/spsc_handler.rs
  • src/shard/spsc_two_db.rs
  • src/shard/timers.rs
  • src/storage/db_quota.rs
  • src/storage/eviction.rs
  • tests/replication_planes.rs

Comment thread CHANGELOG.md
Comment on lines +11 to +12
Three defects found reviewing Wave A (plane replication) before merge, all
fixed on top of the branch's part 1/2/3 work below.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the defect count.

The section says “Three defects” but documents four top-level defects beginning on Lines 14, 31, 49, and 65.

🤖 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 `@CHANGELOG.md` around lines 11 - 12, Update the Wave A summary in CHANGELOG.md
to state “Four defects” instead of “Three defects,” leaving the four documented
defect entries unchanged.

Comment thread CHANGELOG.md
Comment on lines +157 to +160
- Known Wave-A-scoped gaps (documented, not silent): hash-field TTL reaps,
Lua `redis.call` write effects, per-db quota (`--db-maxmemory`) eviction,
and cross-db `COPY ... DB n` destination eviction do not yet emit —
tracked as follow-ups.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the resolved Lua-effects gap from the part-1 limitations.

This says Lua redis.call write effects “do not yet emit,” but the part-2 entry below documents that they now emit to both planes. Qualify this as a historical part-1 limitation or remove that item.

🤖 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 `@CHANGELOG.md` around lines 157 - 160, Update the Wave-A-scoped gaps entry in
the changelog to remove the resolved Lua `redis.call` write-effects item, or
explicitly qualify it as a historical part-1 limitation; retain the other
unresolved limitations unchanged.

Comment thread CHANGELOG.md
Comment on lines +207 to +211
- Turns the 4 remaining RED tests in `tests/replication_planes.rs` green:
`eval_effects_parity_shards1`, `eval_effects_parity_shards4`,
`eval_writes_survive_restart`, `select_in_script_errors`. All 9 tests in
the suite (the 5 from part 1 plus the new `eval_incr_no_double_apply`)
pass.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the regression-suite count.

tests/replication_planes.rs currently defines 11 ignored scenarios, including the two disk-offload hash-parity tests added above, not nine.

🤖 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 `@CHANGELOG.md` around lines 207 - 211, Update the replication_planes.rs
changelog entry to report all 11 ignored scenarios, including the two
disk-offload hash-parity tests, instead of stating that all 9 tests pass.

@pilotspacex-byte pilotspacex-byte merged commit 35cb315 into main Jul 11, 2026
20 of 21 checks passed
@TinDang97 TinDang97 deleted the feat/v0.7-plane-repl-wave-a branch July 11, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants