feat(replication): v0.7 R0+R0.5 — replica stream apply + vector/text index-plane sync#278
Merged
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThe PR adds configurable replication backlog sizing, closes inline PSYNC registration races, carries vector/text index definitions through RDB AUX fields, and implements single-shard replica application of full-resync snapshots and live RESP streams with index-maintenance hooks. ChangesReplication configuration and state
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant Master
participant Replica
participant Shard
participant Database
Master->>Replica: FULLRESYNC RDB payload
Replica->>Shard: load_snapshot
Shard->>Database: load RDB and index definitions
Master->>Replica: live RESP command stream
Replica->>Shard: drain and apply commands
Shard->>Database: update keys and indexes
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
TinDang97
added a commit
that referenced
this pull request
Jul 11, 2026
…zers PR #278 CI (Check + Check macOS) failed with E0063: the new `repl_backlog_size` ServerConfig field (--repl-backlog-size, default 1 MiB) was missing from four struct literals that only compile under --features runtime-tokio,jemalloc — invisible to default-feature builds: - tests/mq_integration.rs - tests/txn_kv_wiring.rs - tests/workspace_integration.rs (x2) All four now set `repl_backlog_size: 1024 * 1024` (the CLI default). Verified: `cargo check --tests --no-default-features --features runtime-tokio,jemalloc` and default-feature `cargo check --tests` both clean. author: Tin Dang
…nd-to-end Foundational fix for the v0.7 replication milestone: before this change a REPLICAOF replica completed the PSYNC2 handshake but applied NOTHING — run_handshake_and_stream discarded the FULLRESYNC RDB bulk (logged "received … bytes" only) and stream_commands did buf.clear() after advancing the offset. A freshly attached replica reported DBSIZE 0. The gap was invisible because every replication_hardening.rs test is #[ignore]d and replication_test.rs only covers handshake/INFO/READONLY. What the replica now does (both runtimes, monoio + tokio): - Loads the full-resync RDB snapshot into its local shard (clear + load). - Parses the live RESP command stream incrementally and applies each write on the shard thread via the thread-local ShardSlice — no SPSC self-hop (the ChannelMesh has no self-slot). Tracks SELECT; skips PING/REPLCONF chatter. Bypasses the connection-layer read-only guard by construction, as a replica must. - MOVE and cross-db COPY ... DB n are applied through the same two-db core helpers the master's handler-level intercept uses (generic dispatch cannot apply them; it errors on MOVE and mis-targets COPY..DB). No eviction gate on apply — the replica follows the master. - A replicated command that fails to apply is logged loudly (silent divergence is the failure mode to fear), and a malformed frame drops the connection for a clean resync instead of guessing at resync points. Wire bug fixed: the replica read the diskless FULLRESYNC RDB bulk as len + 2 bytes, assuming a trailing CRLF that diskless replication does not send — stealing the first two bytes of the command stream and desyncing it. Now reads exactly len (capped at 64 GiB). The replication offset advances by bytes CONSUMED by complete frames, not the raw socket read count, so a partial trailing frame never skews the offset. Guards: a multi-shard replica refuses to start replication loudly (R0 topology is --shards 1) rather than diverging silently. Scope: single-shard, logical db 0. The master's live fanout does not yet stream SELECT (multi-db is task #22); per-shard WAIT/ACK (R1) and multi-shard PSYNC (R2) build on this foundation. Tests: new pure stream router replication::apply with 8 unit tests (framing, partial frames, SELECT tracking, chatter skip, fatal parse); new black-box e2e tests/replication_streaming.rs covering snapshot + live SET/DEL + MOVE/COPY-DB with an in-order stream sentinel. Part of v0.7 replication milestone (R0 → R1 → R2). author: Tin Dang
…ardown
Two independent root causes made the replication_hardening suite unable
to pass, discovered by running it against the R0 binary with stderr
captured and stack sampling (the harness nulls server stderr):
1. --repl-backlog-size did not exist. full_resync_outside_backlog spawns
its master with that flag, so the master exited at spawn with a clap
error and the test failed its first sync assert against a dead server
— a startup failure masquerading as a replication bug. The flag is
Redis repl-backlog-size parity: per-shard backlog capacity in raw
bytes (like --maxmemory), default 1 MiB, clamped to Redis's 16 KiB
floor (the floor also guards ServerConfig's derived Default of 0).
ReplicationState.backlog_capacity is now the single source of truth:
- ensure_backlogs_allocated() reads it (capacity param removed; the
two handshake-path callers previously hardcoded 1 MiB),
- ShardMessage::RegisterReplica carries it so the SPSC lazy
fallback-init cannot silently diverge from the handshake path,
- INFO replication repl_backlog_size reports the real value instead
of a hardcoded 1048576.
2. The harness teardown could never complete: it sent SHUTDOWN NOSAVE
and blocked on Child::wait(), but moon has no working SHUTDOWN on the
production path — the inline parse answers "unknown command", the
dispatch arm at command/mod.rs:767 is an error stub, and no connection
handler intercepts it. Every test therefore hung forever at cleanup,
and a mid-test assert panic leaked live servers that wedged later
tests. Teardown is now a kill-on-drop Guard (SIGKILL + wait), which is
also panic-safe. Implementing a real SHUTDOWN [NOSAVE|SAVE] is tracked
as follow-up work.
With both fixes plus R0, the full hardening suite passes for the first
time: 5/5 (partial resync, full resync outside backlog, network
partition recovery, replica kill-restart parity, replica promotion).
Tests: state.rs unit tests for the capacity default/clamp and for
eviction kicking in exactly at the configured capacity.
Part of v0.7 replication milestone (R0 → R1 → R2).
author: Tin Dang
…ontents) Before this change a replica synchronized only the KV keyspace: FT index definitions never left the master (FT.CREATE/FT.DROPINDEX/FT.CONFIG are connection-layer commands that never reach wal_append_and_fanout), and the replica's apply path ran generic dispatch only, skipping the master's auto-index parity hooks. A replica answered FT._LIST empty and FT.SEARCH with errors even while its hashes matched the master byte-for-byte. Snapshot leg: - The FULLRESYNC RDB now carries the master's vector + text index DEFINITIONS as moon-private RDB AUX fields (opcode 0xFA, keys moon-vector-defs / moon-text-defs), reusing the sidecar codecs (serialize_index_metas_v5 / serialize_text_index_metas). Standard RDB loaders skip unknown AUX keys, so the snapshot stays Redis-compatible and the replication offset math is untouched. - On load the replica drops ALL local indexes (full resync = authoritative replace), installs the master's definitions, and backfills them by rescanning matching HASH keys — the same "restart semantics" rescan restart recovery performs (event_loop.rs phase 2). Live leg: - New ShardMessage::ReplicateVerbatim fans a pre-serialized command into the replication plane only (backlog + offset + replica streams; WAL/AOF deliberately None — defs are durable via the sidecars, an AOF copy would double-apply on recovery). - The monoio connection layer pushes successful FT.CREATE / FT.DROPINDEX / FT.CONFIG SET through it (single-shard; multi-shard FT.* replication rides the R2 broadcast redesign; the tokio-sharded master already rejects PSYNC). - The replica applies FT.* through the same ft_create/ft_dropindex/ ft_config handlers (live FT.CREATE = no backfill, master parity), and runs the master's index-parity hooks after every applied KV write: HSET auto-index, DEL/UNLINK tombstone, HDEL vector-field tombstone, FLUSHDB/FLUSHALL content clear. Tests (red/green TDD): - New black-box e2e replica_syncs_vector_index_defs_and_contents (confirmed RED at step 1 before the fix): snapshot defs + KNN backfill, live HSET indexing, live DEL tombstone, live FT.CREATE streaming, FLUSHALL clears contents keeps defs. - Unit tests for the RDB aux carrier: round-trip, loader-skips-aux, foreign/truncated-buffer rejection (no panic on any prefix cut). - No regression: 41 replication unit tests, 2/2 streaming e2e, 5/5 hardening suite, clippy clean on monoio + tokio feature sets. Refs: task #24 (v0.7 R0.5), stacked on R0 (faed291, e346d0e) author: Tin Dang
Fixes for the C1/M1 findings from the R0/R0.5 adversarial review, plus two of its hygiene items. C1 — registration-bounded catch-up (silent replica gap): the PSYNC task read backlog catch-up bytes and only then registered the replica with the event loop. Any write drained in between (KV Execute or the new ReplicateVerbatim FT.* fanout) reached neither the RDB, nor the catch-up read, nor live fan-out — the master answered +OK while the new replica silently never learned about it. The master now registers FIRST; RegisterReplica carries a reply channel and the event loop answers with the shard offset at which live fan-out begins (read synchronously between drains, so it is exact). Catch-up then sends precisely [snapshot_offset, registration_offset): no gap, and bytes at or above the registration offset arrive only via the replica channel (the read is truncated, so nothing is delivered twice). C1b — atomic snapshot capture: snapshot_offset was read at fn entry, with the +FULLRESYNC write awaited before the RDB capture. A write interleaving there landed inside the RDB AND above the advertised offset, so catch-up re-delivered it — double-applying non-idempotent commands (INCR) on the replica. The offset read and RDB capture now share one synchronous stretch with no .await between them. Also: backlog eviction during catch-up now aborts the sync loudly (send_backlog_range bails; the replica retries a fresh full resync) instead of the old silent `if let Some` skip. M1 — a full resync whose RDB carries no moon index-def aux (pre-R0.5 master, def-serialization failure) silently wiped the replica's local FT indexes. The authoritative replace now logs a warning with the dropped counts when there is no replacement. Hygiene (review m2/m3): read_moon_aux validates the RDB version bytes like load_rdb (a foreign version would misalign the fixed 9-byte header walk); the FT.* fanout hook checks replication_fanout_active (replica registered or backlog allocated) before paying the serialize + SPSC round trip — the backlog is allocated during the PSYNC handshake before any snapshot capture, so a mutation racing a first-ever attach still fans out. The multi-shard RegisterReplica paths pass registered: None (fire-and-forget, superseded by the R2 PrepareReplicaSync redesign). Tests: new race-guard e2e replica_attach_races_live_ft_create — 30 FT.CREATEs from a side thread racing a mid-stream replica attach, exact FT._LIST parity required (3/3 green). No regression: 3/3 streaming e2e, 5/5 hardening, 40 replication + 27 redis_rdb unit tests, clippy clean on monoio + tokio feature sets. Refs: task #24 follow-up; review findings C1/M1/m2/m3 author: Tin Dang
…zers PR #278 CI (Check + Check macOS) failed with E0063: the new `repl_backlog_size` ServerConfig field (--repl-backlog-size, default 1 MiB) was missing from four struct literals that only compile under --features runtime-tokio,jemalloc — invisible to default-feature builds: - tests/mq_integration.rs - tests/txn_kv_wiring.rs - tests/workspace_integration.rs (x2) All four now set `repl_backlog_size: 1024 * 1024` (the CLI default). Verified: `cargo check --tests --no-default-features --features runtime-tokio,jemalloc` and default-feature `cargo check --tests` both clean. author: Tin Dang
TinDang97
force-pushed
the
feat/v0.7-r0-replica-apply
branch
from
b06b438 to
1ea05f9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/replication/state.rs`:
- Around line 10-16: Fresh ReplicationBacklog instances are initialized at
offset zero instead of the current master offset. Update
ReplicationState::ensure_backlogs_allocated() and the lazy backlog creation in
spsc_handler to initialize both start_offset and end_offset from the current
master replication offset, including offsets set by seed_master_offset(), so
newly created backlogs align with PSYNC/FULLRESYNC 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: c94d3362-4e27-409a-97b4-393a4e6d715c
📒 Files selected for processing (22)
CHANGELOG.mdsrc/config.rssrc/main.rssrc/persistence/redis_rdb.rssrc/replication/apply.rssrc/replication/handshake.rssrc/replication/master.rssrc/replication/mod.rssrc/replication/replica.rssrc/replication/state.rssrc/server/conn/handler_monoio/dispatch.rssrc/server/conn/handler_monoio/ft.rssrc/server/embedded.rssrc/server/listener.rssrc/shard/dispatch.rssrc/shard/spsc_handler.rstests/mq_integration.rstests/observability_resident_bytes.rstests/replication_hardening.rstests/replication_streaming.rstests/txn_kv_wiring.rstests/workspace_integration.rs
Comment on lines
+10
to
+16
| /// Default per-shard replication backlog capacity (1 MiB), Redis parity. | ||
| pub const DEFAULT_REPL_BACKLOG_SIZE: usize = 1024 * 1024; | ||
| /// Smallest accepted backlog capacity. Redis clamps `repl-backlog-size` to | ||
| /// 16 KiB; the floor also protects `ServerConfig`'s derived `Default` (0) | ||
| /// from allocating an evict-everything backlog. | ||
| pub const MIN_REPL_BACKLOG_SIZE: usize = 16 * 1024; | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether backlog allocation seeds start/end offset to the current
# global replication offset, and locate all allocation/append call sites.
rg -n 'ensure_backlogs_allocated' -C4
echo "---append call sites---"
rg -n '\.append\(' src/shard/spsc_handler.rs -C6
echo "---issue_lsn call sites near append---"
rg -n 'issue_lsn' src/shard/spsc_handler.rs -C6
echo "---any seed-like method on ReplicationBacklog---"
rg -n 'fn (new|seed|set_start|set_end)' src/replication/backlog.rs -A10
echo "---REPLCONF handling on master side---"
rg -n 'REPLCONF' -i src/replication -C4Repository: pilotspace/moon
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files 'src/replication/*' 'src/shard/*' | sed -n '1,200p'
printf '\n--- backlog outline ---\n'
ast-grep outline src/replication/backlog.rs --view expanded || true
printf '\n--- master outline ---\n'
ast-grep outline src/replication/master.rs --view expanded || true
printf '\n--- state outline ---\n'
ast-grep outline src/replication/state.rs --view expanded || true
printf '\n--- search allocation and seeding ---\n'
rg -n 'ensure_backlogs_allocated|ReplicationBacklog::new|backlog.*seed|seed_master_offset|master_repl_offset|start_offset|end_offset|backlog_off|repl_backlog' src/replication src/shard -C 3 || trueRepository: pilotspace/moon
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf -- '--- files ---\n'
git ls-files 'src/replication/*' 'src/shard/*' | sed -n '1,200p'
printf -- '\n--- backlog outline ---\n'
ast-grep outline src/replication/backlog.rs --view expanded || true
printf -- '\n--- master outline ---\n'
ast-grep outline src/replication/master.rs --view expanded || true
printf -- '\n--- state outline ---\n'
ast-grep outline src/replication/state.rs --view expanded || true
printf -- '\n--- search allocation and seeding ---\n'
rg -n 'ensure_backlogs_allocated|ReplicationBacklog::new|backlog.*seed|seed_master_offset|master_repl_offset|start_offset|end_offset|backlog_off|repl_backlog' src/replication src/shard -C 3 || trueRepository: pilotspace/moon
Length of output: 50371
Seed ReplicationBacklog from the current master offset when allocating it
ReplicationState::ensure_backlogs_allocated() and the lazy fallback in src/shard/spsc_handler.rs both create a fresh backlog with start_offset = end_offset = 0. If the master has already advanced (for example after prior writes or seed_master_offset()), the first replica can get a backlog window that no longer matches master_repl_offset, breaking PSYNC/FULLRESYNC catch-up. Seed the backlog at creation time.
🤖 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/state.rs` around lines 10 - 16, Fresh ReplicationBacklog
instances are initialized at offset zero instead of the current master offset.
Update ReplicationState::ensure_backlogs_allocated() and the lazy backlog
creation in spsc_handler to initialize both start_offset and end_offset from the
current master replication offset, including offsets set by
seed_master_offset(), so newly created backlogs align with PSYNC/FULLRESYNC
state.
This was referenced Jul 11, 2026
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
2 participants
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
Foundation for v0.7 replication GA: makes a
REPLICAOFreplica actually apply the stream end-to-end, then synchronizes the vector + text index plane (definitions and contents), and closes two PSYNC attach races found by adversarial review. Single-shard master scope (multi-shard PSYNC is R2).Four commits, each self-contained:
1.
faed291b— replica applies the replication stream (R0)Before this, a
REPLICAOFreplica completed the PSYNC2 handshake but applied nothing — the FULLRESYNC RDB was discarded and the live stream wasbuf.clear()ed after advancing the offset. A freshly attached replica reportedDBSIZE 0. Hidden because everyreplication_hardeningtest was#[ignore]d.SELECT), synchronously on the shard thread via the thread-localShardSlice.len + 2(assuming a trailing\r\ndiskless never sends), stealing the first two stream bytes.MOVE/ cross-dbCOPY … DB napply via the same two-db cores the master intercept uses. New pure stream router (replication::apply, 8 unit tests) + black-box e2e.2.
e346d0e0—--repl-backlog-size+ kill-based hardening teardown--repl-backlog-size(Redis parity: raw bytes, default 1 MiB, clamped to the 16 KiB floor). Single source of truth inReplicationState.backlog_capacity; carried inRegisterReplicaso the SPSC lazy fallback can't diverge from the handshake allocation; reported by INFO.replication_hardeningteardown as a kill-on-dropGuard(moon has no working SHUTDOWN yet — tracked separately). Full hardening suite passes for the first time: 5/5.3.
a7aeeb61— vector/text index-plane sync (R0.5)Before this, a replica synced only the KV keyspace: FT index defs never left the master, and the replica's apply skipped the master's auto-index parity hooks —
FT._LISTempty,FT.SEARCHerroring, even while hashes matched.moon-vector-defs/moon-text-defs, reusing the sidecar codecs). Standard RDB loaders skip AUX, so snapshots stay Redis-tool-compatible and the offset math is untouched. On load the replica drops all local indexes (full resync = authoritative replace), installs the master's defs, and backfills by rescanning matching HASH keys (restart semantics).FT.CREATE/FT.DROPINDEX/FT.CONFIG SETfan out verbatim via a newShardMessage::ReplicateVerbatim(replication legs only — durability stays with the sidecar). The replica applies them through the same handlers and runs the master's parity hooks (HSET auto-index, DEL/UNLINK + HDEL tombstone, FLUSH* content clear) after every applied KV write.4.
f648c367— close PSYNC attach races (adversarial-review C1/M1/m2/m3)RegisterReplicacarries a reply channel and the event loop answers with the exact offset where live fan-out begins. Catch-up sends precisely[snapshot_offset, registration_offset).read_moon_auxvalidates RDB version bytes; the FT.* fanout hook skips the serialize+SPSC round trip until a replica/backlog exists.replica_attach_races_live_ft_create: 30FT.CREATEs racing a mid-stream attach, exactFT._LISTparity required.Verification
replication_streaming), 5/5 hardening (replication_hardening), race guard 3/3.redis_rdbunit tests.cargo fmt --check,clippy -D warningsclean on default (monoio) +runtime-tokio,jemalloc.--no-fail-fastsuite: only pre-existing macOS-only failures (memory_prometheus_kinds, control-confirmed at merge base) — no regressions from this branch.Scope / follow-ups
Single-shard master (R0/R0.5). Multi-shard PSYNC (R2, #20), real WAIT/ACK (R1, #19), full graph-plane sync (#25), multi-db
SELECTstreaming (#22), and read-only-replicaSELECT(#23) are tracked separately.Summary by CodeRabbit