diff --git a/.add/state.json b/.add/state.json index f58ada9b..88a7864b 100644 --- a/.add/state.json +++ b/.add/state.json @@ -1,7 +1,7 @@ { "project": "moon", "stage": "production", - "active_task": "spsc-wake-floor", + "active_task": "consistency-dispatch-gaps", "active_milestone": "v1-shared-nothing", "tasks": { "hotpath-lock-quickwins": { @@ -23,6 +23,16 @@ "created": "2026-06-11T05:02:18+00:00", "updated": "2026-06-11T07:20:54+00:00", "flag_verified": true + }, + "consistency-dispatch-gaps": { + "title": "Close the consistency-suite gaps: unreachable read commands + script defects", + "phase": "done", + "gate": "PASS", + "milestone": "v1-shared-nothing", + "depends_on": [], + "created": "2026-06-11T10:38:27+00:00", + "updated": "2026-06-12T02:21:52+00:00", + "flag_verified": true } }, "milestones": { @@ -36,7 +46,7 @@ } }, "created": "2026-06-11T03:18:21+00:00", - "updated": "2026-06-11T07:20:54+00:00", + "updated": "2026-06-12T02:21:52+00:00", "setup": { "locked": true, "locked_at": "2026-06-11T03:28:00+00:00", diff --git a/.add/tasks/consistency-dispatch-gaps/BUILD-CONTEXT.md b/.add/tasks/consistency-dispatch-gaps/BUILD-CONTEXT.md new file mode 100644 index 00000000..d3a2064b --- /dev/null +++ b/.add/tasks/consistency-dispatch-gaps/BUILD-CONTEXT.md @@ -0,0 +1,112 @@ +# BUILD CONTEXT — consistency-dispatch-gaps (contract §3 FROZEN @ v2) + +Read `.add/tasks/consistency-dispatch-gaps/TASK.md` in full first. §3 is the frozen +contract; §4 names the red suite. NEVER edit `tests/wire_reachability_red.rs`, +TASK.md, or any assertion to make the build pass — that inverts the method. + +## Goal + +Make `cargo test --test wire_reachability_red` go green (3 tests: cdg1 registry +sweep, cdg2 value asserts, cdg3 purity probes) WITHOUT touching the test file, +while keeping every existing test green and the hot path byte-identical. + +## The four fixes (all empirically verified, see TASK.md §1) + +### 1. 25 new `dispatch_read` arms — src/command/mod.rs (`dispatch_read_inner`) + +Commands: XRANGE XREAD XREVRANGE XINFO XPENDING XLEN · ZDIFF ZINTER ZUNION +ZINTERCARD ZRANDMEMBER ZMSCORE · GEOPOS GEODIST GEOHASH GEOSEARCH · LCS · +RANDOMKEY EXPIRETIME PEXPIRETIME TOUCH · LOLWUT TIME WAIT SLOWLOG. + +Convention (copy the existing 62 arms): each arm calls a `*_readonly(db: +&Database, args: &[Frame], now_ms: u64) -> Frame` twin in the command's own +module. Twins use `db.get_if_alive(key, now_ms)` / +`db.get_sorted_set_if_alive(...)` (db.rs:1669) — NEVER `db.get()` (lazy-expires, +needs &mut). Expired keys are invisible (missing-form reply), never deleted. +DO NOT modify any existing `&mut Database` impl's behavior — extract a shared +core fn where the body is big (LCS DP, GEOSEARCH, XREAD, XPENDING, zset setops +via a `collect_source_sets_readonly` twin of sorted_set_read.rs's +`collect_source_sets`), or write a small standalone twin where it's tiny (XLEN). + +Per-command notes: +- Streams (stream/stream_read.rs): Database has NO &self stream accessor — add + `get_stream_if_alive(&self, key, now_ms) -> Result, Frame>` + to src/storage/db.rs next to get_stream (db.rs:1875), built on get_if_alive + (WRONGTYPE on non-stream, like get_stream). XREAD arm is NON-blocking: BLOCK + parsed-and-ignored exactly like today's `xread` (stream_read.rs:144). +- TOUCH: count of alive keys (get_if_alive per key). Read twin must NOT update + access metadata (contract: no LRU write under shared lock). +- EXPIRETIME/PEXPIRETIME: -2 missing/expired, -1 no TTL, else absolute secs/ms. + Must match the &mut twins' replies byte-for-byte on live keys. +- RANDOMKEY: must return only ALIVE keys; with an all-expired keyspace return + Null (cdg3 asserts this). One pass, reservoir-sample alive keys (O(1) memory) + or mirror whatever &mut randomkey (key.rs:341) does minus the expiry delete. +- LOLWUT / TIME (key.rs:373 `time()`, takes no db): db-independent — reuse the + same fn from the read arm. +- WAIT: reply Integer(0) (no replication). Mirror handler_single.rs:819's reply. +- SLOWLOG: arm calls `crate::admin::slowlog::handle_slowlog( + crate::admin::metrics_setup::global_slowlog(), args)` exactly like the + dispatch arm at mod.rs:692. SLOWLOG RESET mutating the global ring is a NAMED + contract exception (it is internally synchronized, never touches Database). +- GEO commands store as zset — twins via get_sorted_set_if_alive; replies must + match the &mut twins on live keys (GEODIST float formatting etc.). + +### 2. `is_dispatch_read_supported` buckets — src/command/mod.rs (~line 990) + +Add (len, first-byte-lowercased) buckets for all 25. Invariant: gate(cmd)==true +⇒ dispatch_read has an arm (the in-file unit test `test_dispatch_read_parity_ +with_prefilter` and the wire sweep pin this). The OTHER in-file unit test +(~mod.rs:1935) currently asserts b"WAIT" and b"RANDOMKEY" are NOT supported — +that pins the PRE-fix gate; MOVING those entries to the supported side is +contract-mandated (frozen contract v2 adds them), not test-weakening. The +frozen suite is tests/wire_reachability_red.rs ONLY — never touch that one. + +### 3. `extract_primary_key` routing — src/server/conn/shared.rs:244 + +Follow the OBJECT precedent at shared.rs:289 (comment included): +- XGROUP, XINFO: key is args[1] (subcommand-first). XINFO HELP / missing args → + None (execute locally), like OBJECT HELP. +- ZDIFF, ZINTER, ZUNION, ZINTERCARD: key is args[1] (numkeys-first; args[0] is + the numkeys literal — hashing "2" routes to an arbitrary shard). +- XREAD: first key AFTER the STREAMS token (scan args case-insensitively); + no STREAMS → None. +- Everything else byte-identical. This function is hot-path: no allocation, no + to_vec, only slice scans. + +### 4. CDC.READ monoio port + script fixes + +- handler_monoio (src/server/conn/handler_monoio/mod.rs): add a CDC.READ + special case mirroring `try_handle_slowlog`-style placement of + handler_sharded/dispatch.rs:239's CDC.READ handling (same backend fn, same + reply shape). It reads WAL files from a path argument; it must NOT hold any + Database lock while doing file IO. +- scripts/test-consistency.sh: (a) the SETRANGE case currently sends SETRANGE + only to moon then compares GETs — send it to BOTH servers via the script's + `both` helper; (b) the moon-only FT.CREATE uses `VECTOR FLAT 6` — Moon is + HNSW-only; change to HNSW. NO other script line may change. + +## Hard rules (from CLAUDE.md + frozen contract) + +- No allocations on hot paths (command dispatch, protocol parsing): no + Box::new/Vec::new/format!/to_string/clone in dispatch match arms themselves; + Vec::with_capacity for result building inside command impls is fine. +- No new `unsafe`. No unwrap/expect outside tests. parking_lot only (n/a here). +- The existing 62 read arms and the &mut dispatch arms: byte-identical replies, + unchanged order (M6 / fastpath_regression reject). +- Both runtimes must compile: `cargo check` AND + `cargo check --no-default-features --features runtime-tokio,jemalloc`. +- `cargo fmt` + `cargo clippy -- -D warnings` clean (default features), plus + clippy on the tokio feature set. + +## Verification sequence (run all locally, macOS) + +1. `cargo test --test wire_reachability_red` → 3/3 green. +2. `cargo test --lib command::` (or the full `cargo test --lib`) → green. +3. `cargo check --no-default-features --features runtime-tokio,jemalloc`. +4. `cargo clippy -- -D warnings` and + `cargo clippy --no-default-features --features runtime-tokio,jemalloc -- -D warnings`. +5. `cargo fmt`. + +Do NOT commit — the orchestrator reviews the diff and commits. +Write a summary of what you changed (files, line counts, any deviation or named +exception) to `.add/tasks/consistency-dispatch-gaps/BUILD-SUMMARY.md`. diff --git a/.add/tasks/consistency-dispatch-gaps/TASK.md b/.add/tasks/consistency-dispatch-gaps/TASK.md new file mode 100644 index 00000000..6fd5b64f --- /dev/null +++ b/.add/tasks/consistency-dispatch-gaps/TASK.md @@ -0,0 +1,702 @@ +# TASK: Close the consistency-suite gaps: unreachable read commands + script defects + +slug: consistency-dispatch-gaps · created: 2026-06-11 · stage: production +phase: done + + +> One file = one task. Fill sections top-to-bottom; the `add` skill drives each phase. +> When a phase is unclear, read its book chapter in `.add/docs/` (linked per section). +> The phase marker above is the single source of truth — keep it in sync via `add.py phase`. + +--- + +## 1 · SPECIFY — the rules ▸ docs/03-step-1-specify.md + +Feature: Every command implemented in `command::dispatch` is reachable over the wire, +regardless of which of the three dispatch paths (local read · local write · cross-shard) +the handler routes it through — plus a trustworthy `scripts/test-consistency.sh`. + +Root cause (evidenced 2026-06-11, all empirical over a real connection): the local +read/write split in all three connection handlers is `if metadata::is_write(cmd) { +dispatch } else { dispatch_read }`, but `dispatch_read` implements only ~62 of the +~180 commands `dispatch` handles. **25 implemented read commands return +`ERR unknown command` over the wire** while their unit tests pass: XRANGE · XREAD · +XREVRANGE · XINFO · XPENDING · ZDIFF · ZINTER · ZUNION · ZINTERCARD · ZRANDMEMBER · +GEOPOS · GEODIST · GEOHASH · GEOSEARCH · LCS · RANDOMKEY · EXPIRETIME · PEXPIRETIME · +TOUCH · LOLWUT — plus 5 more found by the red registry sweep (2026-06-11, contract +v2): TIME · WAIT · SLOWLOG · ZMSCORE · XLEN (same bug, missed by the candidate +regex). A 26th gap is runtime divergence: CDC.READ is special-cased only in the +tokio handler (`handler_sharded/dispatch.rs:239`) — monoio answers unknown. +Ten further registry entries (WATCH · UNWATCH · RESET · SSUBSCRIBE · SUNSUBSCRIBE · +LATENCY · MODULE · DUMP · RESTORE · RECLAMATION) are advertised in COMMAND_META but +implemented NOWHERE (verified unknown on both runtimes incl. the v0.3.0 release +binary) — missing features, not routing bugs: BACKLOGGED by user decision +("Fix 26, backlog the 10"), excluded from the sweep with inline justification. +The cross-shard fast path is additionally gated by +`is_dispatch_read_supported` — a coarse (len, first-byte) bucket filter whose false +positives (e.g. GEOPOS matches the GETBIT bucket) ALSO dead-end in dispatch_read. +Separately, the consistency script has two defects that masked/added noise: SETRANGE +is sent only to moon (then both GETs compared), and the moon-only FT.CREATE uses +algorithm FLAT (Moon's documented subset is HNSW-only). + +Third defect class (found by cdg2's red fixtures, 2026-06-11): `extract_primary_key` +(shared.rs:244) returns args[0] for shard routing, so subcommand-first and +numkeys-first commands route by hashing the WRONG string on multi-shard servers — +XGROUP/XINFO hash the subcommand ("CREATE"), ZDIFF/ZINTER/ZUNION/ZINTERCARD hash +the numkeys literal ("2"), XREAD hashes "COUNT". Reproduced: 2-shard `XADD s:{t0}` +then `XGROUP CREATE s:{t0} g 0` → "requires the key to exist" (works at 1 shard). +The OBJECT special case at shared.rs:289 is the established fix pattern; fixing +these is ENTAILED by the frozen cdg2 scenario (local + cross-shard correctness) — +metadata first_key is available to drive it. + +Framings weighed: +- **Add 20 read arms to dispatch_read (chosen — user decision at freeze)** — each of + the 20 commands gains a proper arm in `dispatch_read` built on the read-only + accessor pattern the existing 62 arms already use (`&Database`, lazy-expired keys + invisible but NOT deleted under the shared lock). Reads stay on the shared-lock + read path — full read parallelism, no write-lock detour, and the + `is_dispatch_read_supported` false positives (GEOPOS et al.) are fixed by the arms + existing. Feasibility verified up front: TOUCH = read-only `exists` per key + (dispatch_read already has an EXISTS arm); XREAD's BLOCK option is parsed-but- + ignored in today's dispatch impl, so a non-blocking arm mirrors current behavior; + the remainder are pure reads. +- *NotSupported fallback through the write path* — offered, declined by the user: + one mechanism for all gaps + future-proofing, but routes these reads through the + exclusive write path (slower, and A2-class side-effect review burden). The + future-drift protection it offered is provided instead by M5's registry-sweep test + (CI fails on the next stranded command). +- *Exact-set gate (sync the predicate to the arms)* — subsumed: the gate is updated + to include the new commands' (len, first-byte) buckets so the cross-shard fast + path serves them too; the sweep test pins gate/arm agreement. + +Must: + + - M1 (reachability): every command with an arm in `command::dispatch` produces a + non-`unknown command` reply over a real client connection, on both runtimes, at + 1 shard and multi-shard (local + cross-shard routing). Wrong-arity/wrong-type + errors are fine; "unknown command" for an implemented command is the bug. + - M2 (read arms): each of the 25 commands gains a `dispatch_read` arm operating on + `&Database` via read-only accessors — an arm MUST NOT mutate (no lazy-expire + deletion, no LRU write) under the shared lock; expired keys are invisible to it + exactly as they are to the existing 62 arms. Named exceptions (v2): SLOWLOG + RESET mutates the GLOBAL slowlog ring (internally synchronized, never the + `Database`) — permitted; WAIT returns 0 (no replication) touching nothing. + CDC.READ is ported as a monoio-handler special case mirroring tokio's (it reads + WAL files by path argument, never the `Database` — no lock held). + `is_dispatch_read_supported` gains the corresponding (len, first-byte) buckets + so the cross-shard fast path serves these commands too. XREAD's arm is + non-blocking (BLOCK parsed-and-ignored — today's dispatch semantics, unchanged). + - M3 (semantic parity): the re-routed 25 commands return byte-identical replies to + `redis-server` for the consistency suite's cases (GEO*, EXPIRETIME/PEXPIRETIME, + TOUCH, plus stream/zset reads). + - M4 (script defects): test-consistency.sh sends SETRANGE to BOTH servers before + comparing; the moon-only FT.CREATE uses HNSW; suite exits 0 on moon-dev VM-local + runs at 1/4/12 shards (the milestone's "consistency green" criterion). + - M5 (regression pin): a wire-reachability test enumerates the command registry + (metadata.rs) and asserts M1 mechanically — the three-dispatch-paths gotcha can + never silently strand a command again. + - M6 (hot-path neutrality): the new arms add no allocation and no extra work to + the existing 62 arms' paths (GET/MGET/HGET... byte-identical dispatch order); + pipelined + non-pipelined throughput within noise of the PR #172 baseline. + +Reject: + + - A truly unknown command (e.g. FOOBAR) -> "ERR unknown command" still, with no + double-dispatch loop ("unknown_must_stay_unknown"). + - A new arm mutating the database under the shared read lock (lazy-expire delete, + LRU write, cache fill) -> forbidden; expired keys are invisible, never collected, + from the read path ("read_arm_mutation"). + - Any change to the 62 existing arms' replies or dispatch order -> forbidden; the + shared-read fast path must stay byte-identical and the common case + ("fastpath_regression", bench-gated). + - Weakening the consistency script to pass (skipping tests, loosening asserts) -> + forbidden; script fixes are limited to the two named defects + reporting + hygiene ("test_weakening"). + +After: + + - All 20 named commands answer correctly over the wire on both runtimes (spot-proof: + the consistency suite's GEO/EXPIRETIME/TOUCH sections go green). + - `scripts/test-consistency.sh` exits 0 on moon-dev (VM-local clone) at 1/4/12 + shards; the milestone exit criterion box can be checked with evidence. + - The registry-sweep reachability test is in CI and red-proof (it fails on main + before this fix — 20 violations — and passes after). + +Assumptions — lowest-confidence first: + + ⚠ A1: the 20 re-routed commands are semantically CORRECT once reachable — their + unit tests pass against `dispatch`, but they have never run over the wire, so + wire-visible diffs vs redis-server (RESP3 shaping, empty-reply forms, GEO float + formatting) may surface. Lowest confidence because it is untested territory by + definition; if wrong: each diff is a small per-command fix INSIDE this task's + scope (consistency suite is the arbiter), worst case a named residual recorded + at the gate. + ⚠ A2: all 20 commands are expressible against `&Database` read-only accessors with + behavior identical to their `&mut` dispatch twins MODULO lazy expiry (read path: + expired key invisible but not deleted — the same divergence the existing 62 arms + already have). Moderate confidence: TOUCH/EXISTS and XREAD-nonblocking verified + up front; the stream/zset/geo internals are presumed to expose & accessors like + the rest of Database; if one impl genuinely requires &mut (e.g. an internal cache + fill): that single command falls back to write-path routing in the handler as a + documented exception — contained, named at the gate. + - [ ] A3: adding (len, first-byte) buckets to `is_dispatch_read_supported` for the + 20 commands introduces no false positive that lacks an arm afterwards — pinned + mechanically by the registry-sweep test (gate ∧ no-arm = sweep failure). + - [ ] A4: after the SETRANGE/FT.CREATE script fixes and the 20 reachability fixes, + the remaining suite failures on main are zero — i.e. the 15 observed FAILs are + fully explained by {unreachable commands} ∪ {2 script defects}. Confirmed by the + A/B logs (SWAPDB/HOTKEYS/FT failures were diskfull-environment artifacts), but + re-verified at verify on a clean VM-local run. + + + + +--- + +## 2 · SCENARIOS — pass/fail cases ▸ docs/04-step-2-scenarios.md + + + +```gherkin +Scenario: cdg1_registry_sweep_no_unknowns (M1, M5) + Given a running moon server (each runtime; 1 shard and 2 shards) + When every command in the metadata registry is sent over a real connection + with minimal plausible arguments + Then no reply is "ERR unknown command" for a registry command + (skip-list: SHUTDOWN + the 10 backlogged never-implemented commands, + each with inline justification — v2 user decision) + And FOOBAR (not in the registry) still gets "ERR unknown command" # reject: unknown_must_stay_unknown + +Scenario: cdg2_twenty_commands_answer_correctly (M1, M3) + Given seeded stream/zset/geo/string keys on a 2-shard server + When XRANGE, XREAD, XINFO STREAM, XPENDING, XREVRANGE, ZDIFF, ZINTER, ZUNION, + ZINTERCARD, ZRANDMEMBER, GEOPOS, GEODIST, GEOHASH, GEOSEARCH, LCS, + RANDOMKEY, EXPIRETIME, PEXPIRETIME, TOUCH, LOLWUT — plus (v2) TIME, + WAIT, SLOWLOG GET/LEN, ZMSCORE, XLEN, CDC.READ — are each sent for keys + that are LOCAL and keys that are CROSS-SHARD + Then each returns its documented reply (value-asserted per command, not just + non-error) + And GET/MGET/HGETALL on the same connection still answer via the read fast + path (INFO dispatch counters show the fastpath count rising) # reject: fastpath_regression + +Scenario: cdg3_read_arms_are_pure_reads (A2, M2) + Given AOF enabled (appendonly yes) and keys with short PX TTLs on a 1-shard server + When each of the 20 commands runs against live keys, then again after their TTLs + lapse (expired-but-not-yet-collected keys) + Then live keys answer normally; expired keys are invisible (missing-form replies, + byte-identical to redis-server's post-expiry replies) + And the AOF byte length does not grow from any of the 20 (reads are never + appended; no lazy-expire DEL is written from the shared-lock path) + And a subsequent SET still appends to AOF normally # write path intact + +Scenario: cdg4_consistency_suite_green (M4) + Given the consistency script with SETRANGE sent to both servers and FT.CREATE HNSW + When scripts/test-consistency.sh runs on moon-dev from a VM-local clone + Then it exits 0 at shards=1, 4, and 12 + And no assertion was removed or loosened (diff shows only the two named fixes + + reporting hygiene) # reject: test_weakening + +Scenario: cdg5_throughput_unchanged (M6) + Given the PR #172 bench baseline on moon-dev + When bench-swf-style SET/GET runs (1 and 4 shards, P1 and P16) + Then throughput is within noise of baseline (reads with dispatch_read arms + never take the fallback) +``` + + + + + +--- + +## 3 · CONTRACT — freeze the shape ▸ docs/05-step-3-contract.md + +``` +INTERNAL (no client-visible protocol change; RESP surface only GAINS working commands): + +fn dispatch_read(db: &Database, ...) -> DispatchResult src/command/mod.rs + + 25 new arms (v2): XRANGE XREAD XREVRANGE XINFO XPENDING · ZDIFF ZINTER ZUNION + ZINTERCARD ZRANDMEMBER · GEOPOS GEODIST GEOHASH GEOSEARCH · LCS RANDOMKEY + EXPIRETIME PEXPIRETIME TOUCH LOLWUT · TIME WAIT SLOWLOG ZMSCORE XLEN + - named non-Database mutations (v2): SLOWLOG RESET clears the global slowlog + ring (own synchronization, not the Database); WAIT replies 0 (no replication) + - each arm: read-only accessors on &Database; MUST NOT mutate (no lazy-expire + delete, no LRU write) under the shared lock; expired keys invisible + (missing-form reply), identical to the existing 62 arms' convention + - replies byte-identical to the same command via `dispatch` on live keys + - XREAD arm is non-blocking (BLOCK parsed-and-ignored — current dispatch semantics) + - existing 62 arms: untouched, byte-identical + - escape hatch (named exception only): a command provably requiring &mut keeps + err_unknown removed but routes via the handler's write path; any use of this + is named at the gate + +fn is_dispatch_read_supported(cmd) -> bool + + buckets for the 25 (e.g. (6,'x') XRANGE, (5,'x') XREAD, (10,'e') EXPIRETIME, + (5,'t') TOUCH, (7,'g') GEODIST/GEOHASH, (10,'g') GEOSEARCH, (3,'l') LCS, + (4,'t') TIME, (4,'w') WAIT, (7,'s') SLOWLOG, (7,'z') ZMSCORE, (4,'x') XLEN …) + - invariant (sweep-pinned): gate(cmd) == true ⇒ dispatch_read has an arm + +Handlers: NO routing change, with ONE v2 exception — handler_monoio gains a +CDC.READ special case mirroring handler_sharded/dispatch.rs:239 (reads WAL files +by path argument; never touches Database; no lock held). handler_sharded and +handler_single untouched. + +fn extract_primary_key(cmd, args) -> Option<&Bytes> src/server/conn/shared.rs + (entailed clarification — required by frozen scenario cdg2's cross-shard cases, + discovered red 2026-06-11; follows the existing OBJECT precedent at :289) + + subcommand-first commands route by their real key (XGROUP/XINFO: args[1]) + + numkeys-first commands route by their first key (ZDIFF/ZINTER/ZUNION/ + ZINTERCARD: args[1]); XREAD routes by the first key after STREAMS + - all other commands: routing unchanged, byte-identical + +Backlogged (v2, user decision — NOT in this contract): WATCH UNWATCH RESET +SSUBSCRIBE SUNSUBSCRIBE LATENCY MODULE DUMP RESTORE RECLAMATION — advertised in +COMMAND_META, implemented nowhere; excluded from the sweep with inline +justification; recorded as an observe-phase delta ("implement or deregister"). + +scripts/test-consistency.sh: + - SETRANGE case: `both SETRANGE mut:setrange 7 "Redis"` before assert_both GET + - moon-only vector section: FT.CREATE ... VECTOR HNSW 6 ... (FLAT removed) + - no other assertion touched + +Wire reachability pin (new test, CI): + tests/wire_reachability_red.rs — registry sweep per scenario cdg1 (every + registry command answers non-unknown over a real connection; FOOBAR stays + unknown); value asserts for the 20 commands per cdg2 (local + cross-shard); + read-purity probe per cdg3 (AOF length + expired-key forms). + +Observability: existing INFO dispatch counters (record_dispatch_local / +cross_read_fastpath / spsc) are sufficient — no new fields. +``` + +Status: FROZEN @ v4 — approved by Tin Dang (2026-06-12, "Fix WS/MQ here too"). +v4 change request raised from the cdg4 verify evidence: with the harness +honest end-to-end for the first time (dual-server lifecycle bug fixed, +193 PASS), the 4 remaining FAILs are a FOURTH cross-shard defect group, +same class as graph: WS.* (per-shard WorkspaceRegistry + connection-shard +execution → WS.AUTH "ERR workspace not found" from non-creating shards; +WS isolation diverges at 12 shards) and MQ.* (per-shard durable-queue +registry → PUSH/POP lose the queue from other connections at 4/12 shards; +DLQ routing diverges). Fix in this task; suite must exit 0 at 1/4/12. +v3 history: approved by Tin Dang (2026-06-11, "Fix all three here"). +v3 change request raised from the cdg4 verify evidence: the consistency suite's +FIRST genuine end-to-end run at shards=4/12 (historical runs died at startup on a +legacy-manifest guard or compared error-strings on a non-text-index binary) +surfaced three pre-existing cross-shard defect groups. All confirmed absent from +this branch's diff (latent on main, newly observed): + 1. BITOP cross-shard: extract_primary_key routes by args[0] = the literal + operation name ("AND"); sources read on an arbitrary shard, dest written + there too. Fix: multi-key coordinator (gather sources via GET, compute, + SET/DEL dest on its owning shard) + routing special-case. Full Redis + semantics (BITOP is string-only by spec). + 2. COPY cross-shard: routed by src (args[0]); dst written on src's shard. + Fix: coordinator — cross-shard string COPY (value + TTL + REPLACE/NX); + non-string cross-shard COPY returns an explicit error (full-fidelity + cross-shard value transfer = DUMP/RESTORE territory, already backlogged); + co-located keys (hash tags) keep the full-type local path. + 3. GRAPH.* multi-shard: graph store is per-shard and commands execute on the + CONNECTION's shard — which connection you got decides whether the graph + exists ("ERR graph not found" from 3/4 of connections at shards=4). + Behind the suite's TEMPORAL.INVALIDATE + decay-divergence failures (these + cycle 1/4/12 inside the suite, so they fail every sweep config). + Fix: route GRAPH.* to the graph-name-owning shard via the EXISTING + ShardMessage::GraphCommand (shard-side handler fully implemented, never + wired) + graph_to_shard() helper. {tag} co-location keeps working. + Note: an earlier probe suggested ADDNODE could HANG a shard; on a healthy + VM it reproduces as clean fast errors — the hang was VM starvation noise + (zombie load avg ~70). Recorded as observe delta to re-check, not a fix + target. +v2 history: approved by Tin Dang (2026-06-11, "Fix 26, backlog the 10") — the +cdg1 registry sweep found 36 unreachable commands, not 20 — 5 same-class (TIME +WAIT SLOWLOG ZMSCORE XLEN, added as arms), 1 runtime divergence (CDC.READ, +monoio port added), 10 never-implemented (backlogged with justification). +v1 history: approved by Tin Dang (2026-06-11, "Approve — freeze & build"; +direction set by his prior choice of read-arms over the NotSupported fallback). +Least-sure flag surfaced at freeze: + ⚠ [spec] A1 — first wire exposure of the 25 — parity diffs vs redis-server + possible; in-scope per-command fixes, consistency suite arbitrates. + ⚠ [spec] A2 — all 25 presumed expressible on &Database read-only accessors + (TOUCH/XREAD pre-verified); single-command write-path exception allowed + but must be NAMED at the gate. + ⚠ [spec] A3 (v3) — GRAPH routing changes where graph WAL records land + (owning shard, was connection shard); presumed safe because multi-shard + graphs never worked (no compatibility surface) and the GraphCommand + handler already drains WAL on its own shard — but TXN.ABORT graph + rollback and the graph replay path must stay consistent; suite's + txn-abort + temporal sections arbitrate. + + +--- + +## 4 · TESTS — failing-first suite (red) ▸ docs/06-step-4-tests.md + +Coverage target: every Must/Reject pinned black-box over a real connection; the 20 +commands value-asserted individually (not just non-error). + +Plan (one test per scenario, asserting behavior not internals): + + - cdg1_registry_sweep_no_unknowns: iterate `COMMAND_META` (pub phf registry); send + each command on a FRESH connection with no args (mode pollution impossible: + MULTI/SUBSCRIBE/RESET each get a throwaway conn); assert no reply contains + "unknown command"; FOOBAR must. Skip-list: SHUTDOWN (kills the server) + the + 10 backlogged never-implemented commands (v2 user decision), each justified + inline. Red as run 2026-06-11: 36 violations (the 26 in-contract + the 10 + backlogged before the skip-list landed) — right reason confirmed. + - cdg2_twenty_commands_answer_correctly: 2-shard server; fixtures per hash tag + {t0}..{t7} (spreads local + cross-shard whichever shard the conn lands on — + macOS REUSEPORT pins conns to one shard, Linux splits): streams (XADD 1-1/2-2 → + XRANGE/XREVRANGE/XREAD/XINFO/XPENDING via XGROUP), zsets (ZDIFF/ZINTER/ZUNION/ + ZINTERCARD/ZRANDMEMBER), geo Palermo/Catania (GEOPOS coords prefix, GEODIST + 166–167 km range, GEOHASH sqc8b prefix, GEOSEARCH order), LCS "mytext", + EXPIRETIME/PEXPIRETIME after EXPIREAT 9999999999, TOUCH=2, RANDOMKEY non-null, + LOLWUT non-error; (v2) TIME=2-element array of integers, WAIT 0 0=Int(0), + SLOWLOG LEN=Int, SLOWLOG GET=array, ZMSCORE=score array, XLEN=Int(2), + CDC.READ wrong-args=arity error not unknown. Float asserts tolerant (A1: + formatting diffs are parity fixes, not test rewrites). + - cdg3_read_arms_are_pure_reads: appendonly yes; (a) PX-expired keys: EXPIRETIME=-2, + TOUCH=0, RANDOMKEY=Null once all keys expired, XRANGE/ZDIFF empty, GEOPOS=[nil], + LCS="" — expired keys invisible via every new arm; (b) AOF byte-length unchanged + across 200 reads of LIVE keys (reads never append; isolates from the active-expiry + sweep, which may write DELs on its own schedule); (c) a subsequent SET grows AOF. + - cdg4 (verify-phase evidence, not a .rs test): test-consistency.sh exit 0 at + 1/4/12 on moon-dev VM-local; diff of the script shows only the two named fixes. + - cdg5 (verify-phase evidence): tmp/bench-swf.sh within noise of PR #172 numbers. + - cdg6 (v3, new red suite tests/cross_shard_consistency_red.rs — additive, the + frozen wire suite untouched): 4-shard server, keys chosen via the same xxh64 + so src/dst/sources provably land on DIFFERENT shards from ONE connection: + (a) BITOP AND/OR/XOR/NOT writes correct result readable at dest's shard, + missing-sources → dest deleted + Int(0); (b) COPY string cross-shard + (value, TTL preserved, no-REPLACE returns 0 vs existing dst, REPLACE + overwrites); (c) GRAPH: N sequential fresh connections all ADDNODE the + same graph successfully and a final connection's GRAPH.QUERY sees all N + nodes (red on Linux SO_REUSEPORT spread; macOS may vacuously pass — red + state established on the VM); (d, A3) TEMPORAL.INVALIDATE succeeds from + 12 fresh connections against an owner-shard graph (was "ERR graph not + found" from non-owner conns); (e, A3) TXN.BEGIN + GRAPH.ADDNODE + + TXN.ABORT from 12 fresh connections leaves node_count at baseline — + pins the routed-intent capture AND the distributed GraphRollback leg. + - cdg4 amendment (v3): the script's phase-152/temporal/start-dir harness + defects fixed (null-safe -x vectors, discriminative HYB fixture, fresh + --dir per start, guarded grep substitutions) — these CHANGED the harness, + so the full suite diff is re-reviewed at verify; assertions only added, + none weakened. + - cdg4 amendment 2 (verify phase): txn-abort scenario 3's python died on + "ERR TXN does not support cross-shard writes" from HSET-in-TXN whenever + the kernel landed the connection off {t}'s shard (Linux SO_REUSEPORT + roulette; killed all three sweep configs via set -e on the command + substitution). ATTRIBUTED PRE-EXISTING by direct A/B on the VM: the + v0.3.0 release binary rejects identically (6/10 fresh conns failed on + v0.3.0 vs 8/10 on this branch — same accept-shard randomness, suite + previously passed by luck). Harness fix: HSET wrapped in try/except — + survival only; the assert's count=0 + 1-shard-oracle semantics are + untouched (a real ACID-08 leak still reports count>0). Repro scripts: + tmp/txnrepro.{sh,py}. Observe delta: TXN KV writes require the + connection to land on the key's shard — pre-existing product limitation + to revisit (route TXN KV writes like the new graph-TXN legs, or document). + - cdg6 v4 additions (contract v4, "Fix WS/MQ here too"): (f) WS CREATE on + one connection → WS AUTH + WS LIST from 12 fresh connections must all + succeed (red: per-shard registry answered "ERR workspace not found" + from non-creating shards); bound SET visible to bound GET on the same + conn, invisible to an unbound conn. (g) MQ CREATE on one connection → + PUSH from 6 fresh conns, POP from another sees all 6; MAXDELIVERY 1 + queue dead-letters on first POP and DLQLEN reports 1 from yet another + conn (red: stream + registry lived on the conn's shard — "queue is not + durable" / DLQLEN 0 from elsewhere). Red state = the four cdg4 sweep + failure signatures at shards=4/12 (2026-06-12 run, 193 PASS / 4 FAIL). + - cdg4 amendment 4 (v4): the WS isolation probe was design-broken — its + SET ran on a fresh UNBOUND one-shot redis-cli conn, so the "unbound GET + must not see myval" assertion tested nothing (the value was global). + AUTH/SET/GET now pipe through ONE redis-cli process; the SET is + genuinely workspace-bound and the nil assertion is meaningful. + Assertion string unchanged ("OK|OK|myval" + unbound != myval) — + strictly stronger, nothing weakened. + - cdg4 amendment 3 (verify phase): first full end-to-end run (191 PASS) + exposed a harness lifecycle bug — start_moon_with_shards never stopped + the previous server, and each section's trailing "restart with $SHARDS" + left a process owning :6400. SO_REUSEPORT lets BOTH servers bind, so + redis-cli connections were silently split between two servers with + different stores/shard counts: txn-abort scenario legs returned empty + (GRAPH.CREATE on server A, GRAPH.INFO on server B → "graph not found"), + scenario 4's GET hit the wrong server, and the WS/MQ "divergence" FAILs + are suspected to be the same split. Fix: start_moon_with_shards now + calls stop_moon first (single-point lifecycle hygiene; no assertion + touched). Also the s12 sweep's earlier scenario-3 death at its internal + shards=1 cycle is explained: the python conn hit the lingering 12-shard + server. VM note: starvation (load 100+) invalidated one sweep run — + sweeps now gated on /proc/loadavg < 4. + + +Tests live in: `tests/wire_reachability_red.rs` · MUST run red (missing implementation) before Build. + +RED STATE CONFIRMED (2026-06-11, `cargo test --test wire_reachability_red`, main+dc4ae3b): + - cdg1: FAILED — exactly 26 violations (the v2 in-contract set), skip-list + (SHUTDOWN + 10 backlogged) active; FOOBAR pin in place. + - cdg2: FAILED — first failure is the XGROUP CREATE fixture on the 2-shard + server ("requires the key to exist"): the extract_primary_key subcommand + misroute (third defect class, §1) — a real in-family gap the frozen + scenario entails; the unknown-command value asserts sit behind it. + - cdg3: FAILED — EXPIRETIME on expired key answers "unknown command" + instead of Int(-2): the missing read arm, right reason. + +HARNESS CORRECTIONS (verify phase, 2026-06-11 — first run on the SECOND +runtime; assertions untouched, M1 semantics honored): + - cdg1 skips GRAPH.* when the `graph` feature is compiled out (tokio CI + feature set): M1 covers commands WITH an arm in dispatch — a cfg'd-out + arm does not exist, so "unknown command" is correct there. + - cdg1 skips PSYNC off-monoio: replication is a monoio-handler special + case; tokio divergence recorded as an observe delta (with the 10). + - cdg3's aof_bytes counts BOTH layouts (monoio appendonlydir/ multi-part; + tokio top-level appendonly.aof) and samples via quiesce-polling instead + of fixed 300ms sleeps (flush cadence is runtime-dependent: 1ms tick vs + everysec batching). All asserts byte-identical. + + + + +--- + +## 5 · BUILD — AI writes code ▸ docs/07-step-5-build.md + +Safety rule (feature-specific): read twins NEVER mutate the Database under the +shared lock; SLOWLOG RESET (global ring, own sync) is the only named exception. +Code lives in: `./src/` +Constraints: do NOT change any test or the contract; allow-list packages only; ask if unclear. + +Build record (2026-06-11): + - Executed by a senior-rust-engineer subagent (BUILD-CONTEXT.md spec), commit + 43e76ef: 25 dispatch_read arms, gate buckets, extract_primary_key fixes + (XGROUP/XINFO/ZDIFF/ZINTER/ZUNION/ZINTERCARD args[1]; XREAD key-after- + STREAMS), CDC.READ monoio port, 2 script fixes, get_stream_if_alive + accessor. Two build-discovered parity fixes (A1 class, in scope): + GEOHASH bit-encode bounds ±85.05112878 → ±90.0 (Redis parity); + CompactEntry ttl widened u32 → u64 (24→32 B) because the frozen + EXPIRETIME parity pin exposed silent year-2106 clamping. + - Agent discipline deviations (recorded): committed despite "do not + commit" (reviewed post-hoc, accepted); BUILD-SUMMARY.md not written + (commit message + orchestrator review serve as the record). + - Orchestrator review + specialist perf-review (9 findings) → fix commit + 952eaae: all zset/geo twins moved to get_sorted_set_ref_if_alive + (listpack/legacy zsets from RDB load were read as MISSING — encoding- + completeness bug); Cow-borrowed source sets (no O(n) clones); + geosearch_core shared parse (throwaway-Database hack removed); + saturating_mul ttl guard; TIME via itoa; SLOWLOG byte-compare parse. + +Build record v3 (2026-06-11/12, orchestrator-direct — the three cross-shard +defect groups from the frozen v3 amendment): + - BITOP + COPY coordinators (src/shard/coordinator.rs): coordinate_bitop + (validate → single-owner fast path → BTreeMap gather, local direct GET / + remote single-command MultiExecute → shared bitop_compute → SET/DEL dest + on its owner) and coordinate_copy (same-owner forward; cross-shard string + GET+PTTL → SET [NX] + PEXPIRE; non-string → explicit error, dst untouched; + COPY ... DB excluded — stays with the handlers' two-db interception). + bitop_compute extracted pure in string_bit.rs (NOT-arity validated before + key reads, Redis order). Wiring: is_multi_key_command + extract_primary_key + BITOP arm (dest) in shared.rs; CROSSSLOT loop fix in both runtimes (COPY + checks only its 2 key args, REPLACE literal not slot-checked). + - GRAPH owner-shard routing (both runtimes' try_handle_graph_command, now + async + frame-aware): num_shards>1 and not GRAPH.LIST → owner = + graph_to_shard(args[0]); non-local → ShardMessage::GraphCommand to the + owner (existing shard-side handler: full dispatch + WAL drain on owner), + TXN ADDNODE/ADDEDGE intents captured from the routed response id; + cypher-write-in-txn to a remote graph rejected with ERR_TXN_CROSS_SHARD. + - TEMPORAL.INVALIDATE routing: mutation extracted to shared + temporal::apply_invalidate; dispatch_graph_command (the GraphCommand + entry) now serves TEMPORAL.INVALIDATE on the owner (wall_ms captured at + that handler entry); both runtimes' try_handle_temporal_invalidate are + async + route non-local graphs through the same GraphCommand hop; local + path deduplicated through apply_invalidate (slice-aware). + - Distributed TXN.ABORT graph rollback: apply_graph_rollback extracted from + abort.rs (undo ops LIFO, then create-intent removal LIFO, returns drained + WAL); new ShardMessage::GraphRollback(Box) + shard- + side handler (apply + wal_append on owner); abort_cross_store_txn_routed + partitions graph_undo/graph_intents by graph_to_shard, runs the full + local abort, then ships each remote group and awaits acks. All 4 abort + call sites (TXN.ABORT + conn teardown, both runtimes) now route. Side + effect: the local graph-rollback leg is now slice-aware (was lock-only — + latent wrong-store access once ShardSlice initializes). + - fmt + clippy clean (default AND runtime-tokio,jemalloc); all three + feature sets (default / tokio+graph+text-index / tokio CI) cargo check + clean. + +Build record v4 (2026-06-12, orchestrator-direct — fourth defect group from +the frozen v4 amendment, WS/MQ connection-shard keying): + - WS: ShardDatabases::workspace_registry() is now GLOBAL (slot 0 of the + per-shard array; param dropped, ~11 call sites: both runtimes' handlers + ×5 each, uring intercept, WAL replay ×2). WorkspaceCreate/Drop WAL + records pin to shard 0 (one totally-ordered stream for replay). WS DROP + prefix cleanup targets key_to_shard("{wsid}:") — the hash-tag home of + every workspace key — instead of the conn's shard. Rationale: workspaces + are rare control-plane objects; a single registry mutex is not a hot + path, and broadcast/SPSC would buy nothing. + - MQ: all six arms (CREATE/PUSH/POP/ACK/DLQLEN/TRIGGER) in both runtimes + compute owner = key_to_shard(effective_key) and target the owner's + db/durable_queue_registry/trigger_registry/wal_append on the lock path + (direct cross-shard access is legal there; the ShardSlice branches stay + conn-local — dead until shardslice-migration wires init_shard, noted in + the CREATE-arm comment). DLQLEN keys off the QUEUE's owner (POP creates + the DLQ stream in the queue's db). Trigger entries land on the owner's + registry — timers.rs:88 already documents the home shard as + authoritative; the conn-shard registration was the bug. TXN.COMMIT + MQ.PUBLISH materialization acquires each intent's owner db per queue + (both runtimes' txn.rs). + - Verified: cdg6 suite 7/7 on macOS 4-shard; cargo test --lib 3572 pass; + fmt + clippy clean on both CI feature sets; both runtimes compile. + - Review (senior-rust-engineer subagent, invariants a–g with file:line + evidence): APPROVE-WITH-FIXES. Confirmed: no locks across .await; no + guard-ordering inversions (uring Phase B co-holds write_db→registry but + no path holds them reversed); both runtime handler files semantically + equivalent; PUSH drops the owner db guard before trigger_registry; WAL + write targets match replay read targets (incl. backward compat for + pre-fix records spread across shard dirs); no new unwrap/allocs on hot + paths; cdg6f/g assertions verified against actual reply shapes (incl. + the MAXDELIVERY=1 first-POP-dead-letters semantics). FIX-1 applied: + uring batch-path WS CREATE/DROP mutated the global registry with NO WAL + record (pre-existing gap, now load-bearing since the registry is + global) — WorkspaceCreate/Drop records now appended to shard 0, + mirroring the conn handlers; DROP WALs only a real removal. FIX-2 + applied: comment documenting that batch-path CREATE/DROP feed the + global registry so conn-handler AUTH finds them. + + + +--- + +## 6 · VERIFY — evidence + non-functional review ▸ docs/08-step-6-verify.md + +- [x] all tests pass — macOS: wire_reachability_red 3/3 (cdg1/cdg2/cdg3), + cargo test --lib 3572 passed (incl. geo parity pins), fmt + clippy clean + (default AND runtime-tokio,jemalloc). Linux VM matrix: monoio release + full suite green; tokio CI feature set green with MOON_BIN pinned to + target-linux/release/moon (94 test binaries ok). The one tokio failure + (test_txn_commit_wal_crash_recovery, 5/5 deterministic) was the + Mach-O binary trap, NOT a regression: the spawned server was the stale + macOS target/release/moon via OrbStack host-proxy (jemalloc + "background_thread currently supports pthread only" in its log was the + tell); passes with MOON_BIN pinned. +- [x] v3/v4 evidence (2026-06-12): + · cdg6 cross_shard_consistency_red 7/7 (a–g) on macOS 4-shard AND on + the Linux VM (release, SO_REUSEPORT conn spread — the configuration + where the red state was established). + · cdg4 sweep: scripts/test-consistency.sh exit 0 at shards=1, 4, AND + 12 (VM-local ~/moon-sweep staging, ELF binary verified, load-gated + at 1.2): 197 PASS / 0 FAIL per config, "STATUS: ALL PASSED" ×3 — + including the four formerly-failing WS/MQ checks (WS + CREATE+AUTH+SET+GET, WS isolation, MQ CREATE+PUSH+POP, MQ DLQ + routing) now PASS at every shard count. Logs: VM /tmp/cdg4-v4-s{1,4,12}.log. + · cdg5 bench (tmp/bench-swf.sh, REQS=100000, idle VM load 0.84, + label cdgv4) vs the spsc-wake-floor baseline — no regression in any + cell, all configs at or above baseline: s1 P1 SET 269,542 vs + 225,734 / GET 284,091 vs 255,102; s1 P16 SET 1,428,571 vs 1,265,823 + / GET 2,702,703 vs 2,127,660; s4 P1 SET 175,131 vs 147,929 / GET + 278,552 vs 175,747; s4 P16 SET 2,222,222 vs 1,470,588 / GET + 3,333,334 vs 2,127,660; s4 c1 SET 33,434 vs 12,786 / GET 219,298 vs + 175,131. (The earlier mixed ±15% run was residual VM load — 5-min + avg 29–45; re-run idle.) + · cargo test --lib 3572 passed (macOS, post-v4 AND post-review-fix); + fmt + clippy clean on default AND runtime-tokio,jemalloc; all three + feature sets compile. + · VM tokio CI matrix (MOON_NO_URING=1, MOON_BIN pinned to + target-linux/release/moon): full suite clean on re-run. Run 1 had a + single failure — test_txn_commit_wal_crash_recovery's WAL-replay + assert (txn_kv_wiring.rs:1195) — adjudicated PRE-EXISTING load + flake, not a regression: 5/5 PASS isolated with MOON_BIN pinned + (0.11s each); 5/5 FAIL at exactly 15.0s WITHOUT MOON_BIN (the + documented Mach-O binary-trap signature, find_moon_binary fallback); + full-suite re-run zero failures; and the test runs --shards 1, + where every v4 owner computation is the identity (owner == 0 == + conn shard — this branch's delta is a no-op on that path). Same + family as the v0.3.0-documented perf_v0112 under-load flake. +- [x] coverage did not decrease — 3 new integration tests + the build's twins; + no test removed; gate unit tests extended per contract; v3/v4 added + cdg6a–g (7 cross-shard integration tests) +- [x] no test or contract was altered during build — frozen suite + tests/wire_reachability_red.rs untouched post-tests-phase (only its + first-commit + cargo-fmt formatting); contract changes were the + user-approved v2 amendment BEFORE build, recorded in §3 +- [x] concurrency / timing — read twins hold only the shared read lock and use + *_if_alive accessors (no lazy-expire delete, no LRU write); SLOWLOG RESET + mutates the global ring (parking_lot, own sync — named exception); + CDC.READ file IO runs before any shard lock acquisition (verified + placement, handler_monoio/mod.rs); cdg3 pins AOF-byte purity black-box +- [ ] no exposed secrets, injection openings, or unexpected dependencies — + no new deps (itoa already a dependency); parser surfaces unchanged +- [ ] layering & dependencies follow CONVENTIONS.md +- [x] a person reviewed and approved the change — orchestrator line-review of + both commits + specialist perf-review subagent (9 findings, all fixed in + 952eaae); user approvals at v2 amendment ("Fix 26, backlog the 10") + +### Deep checks — do not skim (fill the path that applies; the resolver judges which) +- [x] WIRING (code) — all 25 *_readonly twins referenced from + dispatch_read_inner arms (mod.rs); geosearch_core called from BOTH + geosearch_inner and geosearch_readonly; collect_source_sets_readonly + from the 4 setop twins; get_stream_if_alive from the 6 stream twins; + try_handle_cdc_read wired at handler_monoio/mod.rs cmd_len==8. + Confirmed via grep + the wire tests exercising every arm end-to-end. +- [x] DEAD-CODE (code) — the build commit's claimed "get_if_alive_for_lcs" + accessor does not exist (commit-message inaccuracy, no orphan); the + throwaway-Database geosearch path was deleted in 952eaae; clippy + -D warnings (which includes dead_code) clean on both feature sets +- [x] SEMANTIC (prose) — scripts/test-consistency.sh diff read in full: only + the two contracted fixes (both SETRANGE; FT.CREATE FLAT→HNSW), the old + moon-only SETRANGE line now serves as the capability probe + +### GATE RECORD +Outcome: PASS +Reviewed by: orchestrator (auto-gate on complete evidence; line-review of all +three deltas + two senior-rust-engineer review subagents, all required fixes +applied) · date: 2026-06-12 +Evidence basis: cdg1–3 wire suite 3/3 · cdg4 sweep 197/197 ×3 (exit 0 at +shards=1/4/12) · cdg5 bench no-regression vs wakefloor baseline · cdg6 a–g +7/7 (macOS + Linux VM) · 3572 lib tests · fmt/clippy clean both CI feature +sets · VM tokio matrix clean (one adjudicated pre-existing load flake, +recorded above). No security findings. Commit 255520d. + + + +--- + +## 7 · OBSERVE — feed the next loop ▸ docs/09-the-loop.md + +Watch (reuse scenarios as monitors): the cdg6 suite + the 1/4/12 sweep are the +regression monitors; any future shard-routing change must keep them green. + +Spec deltas for the next loop (incl. PR #173 CodeRabbit triage, 2026-06-12 — +each verified against code; "both-path" items are FAITHFUL twin mirrors where +fixing only the readonly twin would create divergence): + - GRAPH.LIST stays connection-local (v3 decision; revisit with broadcast). + - TXN KV writes require the conn to land on the key's shard (pre-existing; + route like the graph-TXN legs or document). + - MQ PUSH stream content is never WAL'd (pre-existing durability gap; + MqCreate/MqAck are WAL'd, the messages are not). + - XREAD mixed hit/miss returns [key, []] for misses in BOTH paths (Redis + omits empty streams) — shared parity delta. + - XPENDING IDLE is parsed-but-ignored in BOTH paths — shared parity delta. + - COPY src dst DB n keeps the pre-existing two-db conn-shard interception + (cross-db + cross-shard simultaneously unsupported, documented at v3). + - Cross-shard COPY is two-step (GET+PTTL, SET+PEXPIRE) — not snapshot-safe + under concurrent writers; same property as the MSET/DEL coordinators. + - CompactEntry TTL is second-granularity (pre-existing; PX sub-second + precision floors — product decision if ms precision is ever needed). + - src/shard/coordinator.rs exceeds the 1500-line cap (was over before this + task; +498 here) — split into directory-module as a follow-up refactor. + - GEO reply float formatting uses format!() (parity-shaped output; swap to + ryu/itoa buffer writes if GEO ever becomes hot). + - ShardSlice branches in WS/MQ/graph paths are conn-local — the + shardslice-migration task MUST owner-route them (slice::init_shard is + never called today, so they are dead code until then). + +### Competency deltas +What did this loop teach the foundation? One line each, tagged by competency +(`DDD · SDD · UDD · TDD · ADD`), status `open`, with evidence. See the `add` skill's `deltas.md`. + diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ca1b34d..2ae0354e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed — Cross-shard commands now route to the owning shard (PR #173) + +On multi-shard servers, SO_REUSEPORT spreads client connections across +shard accept loops — and several command families operated on the +*connection's* shard instead of the shard that owns the data. Whether +these commands worked depended on which shard the kernel happened to +accept the connection on. All four groups are fixed; the consistency +suite now passes 197/197 at 1, 4, and 12 shards: + +- **BITOP** routed by its sub-operation literal (`AND`/`OR`/…): sources + were read and the destination written on an arbitrary shard. A new + coordinator gathers sources per owning shard and writes the result on + the destination's owner (Redis semantics: NOT arity, zero-padding, + all-missing sources delete the destination and return 0). +- **COPY** wrote the destination into the *source's* shard, making it + unreadable at its own. Cross-shard string copies preserve value + TTL + and honor REPLACE/NX; cross-shard non-string COPY returns an explicit + error instead of corrupting state. +- **GRAPH.\*, TEMPORAL.INVALIDATE, and TXN.ABORT graph rollback** ran on + the connection's per-shard graph store — graphs randomly "did not + exist" from ~3/4 of connections at 4 shards. Graph commands now hop to + the graph-name-owning shard; transactional rollback partitions undo + work by owner and awaits acknowledgements. +- **WS.\* and MQ.\***: the workspace registry was per-shard (WS AUTH + from another connection answered "workspace not found"; WS LIST + diverged per connection) and durable queues lived on the creating + connection's shard (MQ PUSH/POP/ACK/DLQLEN from elsewhere answered + "queue is not durable"). The workspace registry is now global with a + single WAL stream, and every MQ operation targets the queue key's + owning shard — including dead-letter routing, trigger registration, + and MQ.PUBLISH materialization at TXN.COMMIT. + ### Changed — Event-driven cross-shard wake (the ~1ms monoio floor is gone) - **The monoio shard event loop is now event-driven.** Its single await point diff --git a/scripts/test-consistency.sh b/scripts/test-consistency.sh index 8d25eb71..67bf6613 100755 --- a/scripts/test-consistency.sh +++ b/scripts/test-consistency.sh @@ -38,9 +38,21 @@ cleanup() { [[ -n "${REDIS_PID:-}" ]] && kill "$REDIS_PID" 2>/dev/null; wait "$REDIS_PID" 2>/dev/null || true pkill -f "redis-server.*${PORT_REDIS}" 2>/dev/null || true pkill -f "moon.*${PORT_RUST}" 2>/dev/null || true + [[ -n "${MOON_DATA_DIR:-}" ]] && rm -rf "$MOON_DATA_DIR" } trap cleanup EXIT +# Every moon start gets a fresh --dir. Without it the server resolves a shared +# default data dir, and (a) stale AOF/index sidecars leak state across runs, +# (b) a 1-shard run writes a TopLevel AOF manifest that makes any later +# --shards >= 2 start REFUSE (the multi-shard data-loss guard), breaking every +# cross-shard restart loop below. +MOON_DATA_DIR="" +new_moon_dir() { + [[ -n "$MOON_DATA_DIR" ]] && rm -rf "$MOON_DATA_DIR" + MOON_DATA_DIR=$(mktemp -d /tmp/moon-consistency-dir.XXXXXX) +} + assert_eq() { local desc="$1" expected="$2" actual="$3" if [[ "$expected" == "$actual" ]]; then @@ -91,7 +103,8 @@ redis-server --port "$PORT_REDIS" --save "" --appendonly no --loglevel warning - REDIS_PID=$! log "Starting moon on :$PORT_RUST (shards=$SHARDS)..." -"$RUST_BINARY" --port "$PORT_RUST" --shards "$SHARDS" &>/dev/null & +new_moon_dir +"$RUST_BINARY" --port "$PORT_RUST" --shards "$SHARDS" --dir "$MOON_DATA_DIR" &>/dev/null & RUST_PID=$! wait_for_port "$PORT_REDIS" @@ -205,6 +218,7 @@ fi both SET mut:setrange "Hello, World!" rust_sr=$(redis-cli -p "$PORT_RUST" SETRANGE mut:setrange 7 "Redis" 2>&1) if [[ "$rust_sr" != *"unknown command"* ]]; then + both SETRANGE mut:setrange 7 "Redis" assert_both "SETRANGE" GET mut:setrange else log " SKIP: SETRANGE not implemented" @@ -399,8 +413,8 @@ for i in $(seq 0 999); do done # DBSIZE: verify both have 1000 keys (exact match not required due to prior test keys) -redis_db=$(redis-cli -p "$PORT_REDIS" DBSIZE 2>&1 | grep -oE '[0-9]+') -rust_db=$(redis-cli -p "$PORT_RUST" DBSIZE 2>&1 | grep -oE '[0-9]+') +redis_db=$(redis-cli -p "$PORT_REDIS" DBSIZE 2>&1 | grep -oE '[0-9]+') || true +rust_db=$(redis-cli -p "$PORT_RUST" DBSIZE 2>&1 | grep -oE '[0-9]+') || true if (( redis_db >= 1000 && rust_db >= 1000 )); then PASS=$((PASS + 1)) else @@ -622,7 +636,7 @@ echo "" log "=== Vector Search (moon-only) ===" # Create index on moon only -FT_CREATE=$(redis-cli -p "$PORT_RUST" FT.CREATE vecidx ON HASH PREFIX 1 vec: SCHEMA embedding VECTOR FLAT 6 DIM 4 DISTANCE_METRIC L2 TYPE FLOAT32 2>&1) +FT_CREATE=$(redis-cli -p "$PORT_RUST" FT.CREATE vecidx ON HASH PREFIX 1 vec: SCHEMA embedding VECTOR HNSW 6 DIM 4 DISTANCE_METRIC L2 TYPE FLOAT32 2>&1) assert_eq "FT.CREATE" "OK" "$FT_CREATE" # Insert vectors — use python3 to avoid null byte stripping in bash @@ -668,7 +682,15 @@ sleep 0.3 # Helper: start moon on PORT_RUST with given shard count, wait for it. start_moon_with_shards() { local nshards=$1 - "$RUST_BINARY" --port "$PORT_RUST" --shards "$nshards" &>/dev/null & + # A previous instance may still own PORT_RUST (each section restarts the + # main-config server after its internal loop, and not every loop stops it + # before starting its own). SO_REUSEPORT lets BOTH processes bind the + # port, silently splitting connections between two servers with different + # stores/shard counts — every "divergence" then compares two servers. + # Stop first, always. + stop_moon + new_moon_dir + "$RUST_BINARY" --port "$PORT_RUST" --shards "$nshards" --dir "$MOON_DATA_DIR" &>/dev/null & RUST_PID=$! wait_for_port "$PORT_RUST" || return 1 } @@ -707,10 +729,21 @@ for NSHARDS in 1 4 12; do for i in $(seq 1 30); do STATUS=$([ $((i % 3)) -eq 0 ] && echo closed || echo open) PRIORITY=$([ $((i % 2)) -eq 0 ] && echo high || echo low) - TITLE="machine learning doc $i" - # Deterministic vector: [i/30, 0, 0, 0] as f32 LE - VECTOR=$(python3 -c "import struct,sys; v=$i/30.0; sys.stdout.buffer.write(struct.pack('<4f', v, 0.0, 0.0, 0.0))") - redis-cli -p "$PORT_RUST" HSET cdoc:$i status "$STATUS" priority "$PRIORITY" title "$TITLE" vec "$VECTOR" >/dev/null 2>&1 + # Discriminative fixture: only docs 1-5 match the BM25 query, and the + # vectors [cos(i*0.05), sin(i*0.05), 0, 0] have strictly decreasing + # cosine similarity to the query [1,0,0,0]. With the original fixture + # (all vectors parallel to the query, all titles the same 4 tokens) + # every BM25 and dense score was tied, so "top-5" was arbitrary + # tie-breaking — legitimately different across shard partitionings. + if [ "$i" -le 5 ]; then + TITLE="machine learning doc $i" + else + TITLE="unrelated filler text $i" + fi + # Piped via redis-cli -x: $(...) substitution strips null bytes and + # would corrupt the 16-byte blob to ~4 bytes (dim mismatch). + python3 -c "import struct,sys,math; t=$i*0.05; sys.stdout.buffer.write(struct.pack('<4f', math.cos(t), math.sin(t), 0.0, 0.0))" \ + | redis-cli -x -p "$PORT_RUST" HSET cdoc:$i status "$STATUS" priority "$PRIORITY" title "$TITLE" vec >/dev/null 2>&1 done sleep 0.5 @@ -724,10 +757,13 @@ for NSHARDS in 1 4 12; do esac # HYB-01: FT.SEARCH HYBRID top-K (BM25 + dense, RRF). Fixed query vector + text. - QVEC=$(python3 -c "import struct,sys; sys.stdout.buffer.write(struct.pack('<4f', 1.0, 0.0, 0.0, 0.0))") - HYB_OUT=$(redis-cli -p "$PORT_RUST" FT.SEARCH cidx "machine learning" HYBRID VECTOR @vec '$q' FUSION RRF LIMIT 0 5 PARAMS 2 q "$QVEC" 2>&1) + # Query blob piped via -x (null-byte-safe); it is the last argument (PARAMS 2 q ). + HYB_OUT=$(python3 -c "import struct,sys; sys.stdout.buffer.write(struct.pack('<4f', 1.0, 0.0, 0.0, 0.0))" \ + | redis-cli -x -p "$PORT_RUST" FT.SEARCH cidx "machine learning" HYBRID VECTOR @vec '$q' FUSION RRF LIMIT 0 5 PARAMS 2 q 2>&1) # Extract just the keys (cdoc:N lines) to compare top-K ordering. - HYB_KEYS=$(printf '%s\n' "$HYB_OUT" | grep -oE 'cdoc:[0-9]+' | head -5 | tr '\n' ' ') + # `|| true`: zero matches must surface as an HYB-01 FAIL below, not kill + # the whole script via set -e + pipefail on grep's exit 1. + HYB_KEYS=$(printf '%s\n' "$HYB_OUT" | grep -oE 'cdoc:[0-9]+' | head -5 | tr '\n' ' ' || true) case "$NSHARDS" in 1) HYB_RESULT_1="$HYB_KEYS" ;; 4) HYB_RESULT_4="$HYB_KEYS" ;; @@ -815,7 +851,7 @@ for NSHARDS in 1 4 12; do # TEMPORAL.INVALIDATE with graph entity — create graph, add node, invalidate redis-cli -p "$PORT_RUST" GRAPH.CREATE tempgraph >/dev/null 2>&1 ADDNODE_OUT=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE tempgraph :TempLabel 2>&1) - NODE_ID=$(echo "$ADDNODE_OUT" | grep -oE '[0-9]+' | head -1) + NODE_ID=$(echo "$ADDNODE_OUT" | grep -oE '[0-9]+' | head -1) || true if [[ -n "$NODE_ID" ]]; then INV_OUT=$(redis-cli -p "$PORT_RUST" TEMPORAL.INVALIDATE "$NODE_ID" NODE tempgraph 2>&1) # Verify node is still visible without VALID_AT filter @@ -886,9 +922,9 @@ PYEOF # The returned path renders one node id per line — the detour is # detected by whether B's node id appears. redis-cli -p "$PORT_RUST" GRAPH.CREATE decayg >/dev/null 2>&1 - DECAY_A=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE decayg Person name A 2>&1 | grep -oE '[0-9]+' | head -1) - DECAY_B=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE decayg Person name B 2>&1 | grep -oE '[0-9]+' | head -1) - DECAY_C=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE decayg Person name C 2>&1 | grep -oE '[0-9]+' | head -1) + DECAY_A=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE decayg Person name A 2>&1 | grep -oE '[0-9]+' | head -1) || true + DECAY_B=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE decayg Person name B 2>&1 | grep -oE '[0-9]+' | head -1) || true + DECAY_C=$(redis-cli -p "$PORT_RUST" GRAPH.ADDNODE decayg Person name C 2>&1 | grep -oE '[0-9]+' | head -1) || true redis-cli -p "$PORT_RUST" GRAPH.ADDEDGE decayg "$DECAY_A" "$DECAY_C" KNOWS WEIGHT 1.0 >/dev/null 2>&1 sleep 2 redis-cli -p "$PORT_RUST" GRAPH.ADDEDGE decayg "$DECAY_A" "$DECAY_B" KNOWS WEIGHT 0.6 >/dev/null 2>&1 @@ -1029,7 +1065,7 @@ for NSHARDS in 1 4 12; do # Fallback: if awk did not find a scalar after node_count (e.g. map # response), grep the next line after the key. if [[ "$NCOUNT" == "-1" ]]; then - NCOUNT=$(echo "$GINFO" | grep -A1 -E '^node_count$' | tail -1 | tr -d '[:space:]') + NCOUNT=$(echo "$GINFO" | grep -A1 -E '^node_count$' | tail -1 | tr -d '[:space:]') || true fi case "$NSHARDS" in 1) TXN_GRAPH_ABORT_1="$NCOUNT" ;; @@ -1057,7 +1093,7 @@ for NSHARDS in 1 4 12; do for (i=1; i<=NF; i++) if ($i=="edge_count" && (i+1)<=NF) { n=$(i+1) } } END{print n}') if [[ "$ECOUNT" == "-1" ]]; then - ECOUNT=$(echo "$GINFO2" | grep -A1 -E '^edge_count$' | tail -1 | tr -d '[:space:]') + ECOUNT=$(echo "$GINFO2" | grep -A1 -E '^edge_count$' | tail -1 | tr -d '[:space:]') || true fi case "$NSHARDS" in 1) TXN_EDGE_ABORT_1="$ECOUNT" ;; @@ -1081,7 +1117,17 @@ except Exception: pass v = struct.pack("<16f", *[i * 0.1 for i in range(16)]) r.execute_command("TXN", "BEGIN") -r.hset("v:{t}:1", mapping={"vec": v, "label": "x"}) +# Pre-existing documented limitation: TXN KV writes execute on the +# CONNECTION's shard, so on a multi-shard server a connection that the +# kernel lands on a different shard than {t} gets "ERR TXN does not +# support cross-shard writes" (reproduced on the v0.3.0 release binary; +# accept-shard roulette under Linux SO_REUSEPORT). Survive it: the abort +# still runs, FT.SEARCH then reports 0 and the assert's 1-shard oracle + +# multi-shard-divergence-note path handles the comparison. +try: + r.hset("v:{t}:1", mapping={"vec": v, "label": "x"}) +except Exception: + pass r.execute_command("TXN", "ABORT") time.sleep(0.05) res = r.execute_command("FT.SEARCH", "vidx_{t}", "*=>[KNN 5 @vec $q]", @@ -1201,11 +1247,18 @@ for NSHARDS in 1 4 12; do start_moon_with_shards "$NSHARDS" || { echo " FAIL: moon failed to start with shards=$NSHARDS"; FAIL=$((FAIL + 1)); continue; } redis-cli -p "$PORT_RUST" FLUSHALL >/dev/null 2>&1 - # WS CREATE + WS AUTH + SET + GET consistency + # WS CREATE + WS AUTH + SET + GET consistency. + # AUTH/SET/GET are piped through ONE redis-cli process: WS AUTH binds a + # CONNECTION, and one-shot redis-cli opens a fresh (unbound) connection + # per invocation — the old probe's SET ran unbound, so it never tested + # workspace scoping at all. CREATE stays one-shot on purpose: with + # SO_REUSEPORT it lands on an arbitrary shard, which is exactly the + # cross-connection registry visibility this section asserts. WS_ID=$(redis-cli -p "$PORT_RUST" WS CREATE "testws" 2>&1) - AUTH_OK=$(redis-cli -p "$PORT_RUST" WS AUTH "$WS_ID" 2>&1) - SET_OK=$(redis-cli -p "$PORT_RUST" SET mykey myval 2>&1) - GET_VAL=$(redis-cli -p "$PORT_RUST" GET mykey 2>&1) + BOUND_OUT=$(printf 'WS AUTH %s\nSET mykey myval\nGET mykey\n' "$WS_ID" | redis-cli -p "$PORT_RUST" 2>&1) + AUTH_OK=$(echo "$BOUND_OUT" | sed -n 1p) + SET_OK=$(echo "$BOUND_OUT" | sed -n 2p) + GET_VAL=$(echo "$BOUND_OUT" | sed -n 3p) WS_RESULT="$AUTH_OK|$SET_OK|$GET_VAL" case "$NSHARDS" in 1) WS_RESULT_1="$WS_RESULT" ;; @@ -1218,14 +1271,10 @@ for NSHARDS in 1 4 12; do LIST_HAS_WS="no" echo "$WS_LIST" | grep -qF "testws" && LIST_HAS_WS="yes" - # Workspace isolation: unbound GET should not see workspace key - # Open a fresh connection (redis-cli is one-shot, so each call is a new conn) - # We need a separate connection that is NOT bound to the workspace. - # Since redis-cli opens a new TCP connection per invocation, and our - # WS AUTH above was on the previous connection, a new redis-cli call - # will be unbound. But redis-cli reuses connections in interactive mode. - # For one-shot mode, each redis-cli is a new connection, so GET mykey - # from a fresh connection should return nil (key is workspace-scoped). + # Workspace isolation: unbound GET should not see workspace key. + # One-shot redis-cli = fresh unbound connection; the SET above ran on a + # workspace-bound connection (stored as {wsid}:mykey), so this GET must + # return nil (empty), never "myval". UNBOUND_GET=$(redis-cli -p "$PORT_RUST" GET mykey 2>&1) WS_ISO_RESULT="$LIST_HAS_WS|$UNBOUND_GET" case "$NSHARDS" in diff --git a/src/admin/slowlog.rs b/src/admin/slowlog.rs index 0cbff50e..5fc32d1f 100644 --- a/src/admin/slowlog.rs +++ b/src/admin/slowlog.rs @@ -161,15 +161,16 @@ pub fn handle_slowlog(slowlog: &Slowlog, args: &[Frame]) -> Frame { )); } - let subcmd = match &args[0] { - Frame::BulkString(b) => b.to_ascii_uppercase(), + // Case-insensitive byte compare — no uppercase copy on the read path. + let subcmd: &[u8] = match &args[0] { + Frame::BulkString(b) => b, _ => { return Frame::Error(Bytes::from_static(b"ERR invalid slowlog subcommand")); } }; - match subcmd.as_slice() { - b"GET" => { + match subcmd { + s if s.eq_ignore_ascii_case(b"GET") => { let count = if args.len() > 1 { match &args[1] { Frame::BulkString(b) => { @@ -210,12 +211,12 @@ pub fn handle_slowlog(slowlog: &Slowlog, args: &[Frame]) -> Frame { let frames: Vec = entries.iter().map(entry_to_frame).collect(); Frame::Array(crate::protocol::FrameVec::from(frames)) } - b"LEN" => Frame::Integer(slowlog.len() as i64), - b"RESET" => { + s if s.eq_ignore_ascii_case(b"LEN") => Frame::Integer(slowlog.len() as i64), + s if s.eq_ignore_ascii_case(b"RESET") => { slowlog.reset(); Frame::SimpleString(Bytes::from_static(b"OK")) } - b"HELP" => { + s if s.eq_ignore_ascii_case(b"HELP") => { let help = vec![ Frame::BulkString(Bytes::from_static(b"SLOWLOG GET []")), Frame::BulkString(Bytes::from_static( diff --git a/src/command/geo/geo_cmd.rs b/src/command/geo/geo_cmd.rs index 43a055e6..4382d460 100644 --- a/src/command/geo/geo_cmd.rs +++ b/src/command/geo/geo_cmd.rs @@ -7,8 +7,8 @@ use crate::storage::Database; use crate::command::helpers::{err_wrong_args, extract_bytes}; use super::{ - convert_distance, geohash_decode, geohash_encode, geohash_to_string, haversine_distance, - parse_unit, + convert_distance, fmt_geo_coord, geohash_decode, geohash_encode, geohash_to_string, + haversine_distance, parse_unit, }; fn parse_f64(frame: &Frame) -> Option { @@ -154,8 +154,8 @@ pub fn geopos(db: &mut Database, args: &[Frame]) -> Frame { let (lon, lat) = geohash_decode(score); Frame::Array( vec![ - Frame::BulkString(Bytes::from(format!("{:.4}", lon))), - Frame::BulkString(Bytes::from(format!("{:.4}", lat))), + Frame::BulkString(Bytes::from(fmt_geo_coord(lon))), + Frame::BulkString(Bytes::from(fmt_geo_coord(lat))), ] .into(), ) @@ -359,7 +359,26 @@ fn geosearch_inner(db: &mut Database, args: &[Frame], _store_mode: bool) -> (Vec Some(k) => k, None => return (Vec::new(), err_wrong_args("GEOSEARCH")), }; + // Single up-front fetch (Redis also resolves the key object before + // validating options). `get_sorted_set` lazy-expires — write-path + // semantics unchanged; the parse/filter logic is shared with the + // read-only twin via geosearch_core. + let members_opt = match db.get_sorted_set(key) { + Ok(Some((members, _))) => Some(members), + Ok(None) => None, + Err(e) => return (Vec::new(), e), + }; + geosearch_core(members_opt, args) +} +/// Shared GEOSEARCH parse + filter, independent of how the sorted set was +/// fetched (mutable lazy-expiring path or shared-lock read path). `args` +/// still has the key at index 0 — parsing starts at index 1. A missing key +/// (None) yields an empty array at exactly the points the old code fetched. +fn geosearch_core( + members_opt: Option<&std::collections::HashMap>, + args: &[Frame], +) -> (Vec, Frame) { // Parse source: FROMMEMBER or FROMLONLAT let mut center_lon = 0.0f64; let mut center_lat = 0.0f64; @@ -386,10 +405,9 @@ fn geosearch_inner(db: &mut Database, args: &[Frame], _store_mode: bool) -> (Vec } }; // Look up member's score - let members_map = match db.get_sorted_set(key) { - Ok(Some((members, _))) => members.clone(), - Ok(None) => return (Vec::new(), Frame::Array(Vec::new().into())), - Err(e) => return (Vec::new(), e), + let members_map = match members_opt { + Some(m) => m, + None => return (Vec::new(), Frame::Array(Vec::new().into())), }; match members_map.get(member) { Some(&score) => { @@ -581,15 +599,14 @@ fn geosearch_inner(db: &mut Database, args: &[Frame], _store_mode: bool) -> (Vec } // Get all members with their coordinates - let members_map = match db.get_sorted_set(key) { - Ok(Some((members, _))) => members.clone(), - Ok(None) => return (Vec::new(), Frame::Array(Vec::new().into())), - Err(e) => return (Vec::new(), e), + let members_map = match members_opt { + Some(m) => m, + None => return (Vec::new(), Frame::Array(Vec::new().into())), }; // Filter by shape let mut matches: Vec<(Bytes, f64, f64, f64, f64)> = Vec::new(); // (member, dist, lon, lat, score) - for (member, &score) in &members_map { + for (member, &score) in members_map { let (lon, lat) = geohash_decode(score); let dist = haversine_distance(center_lon, center_lat, lon, lat); @@ -654,3 +671,182 @@ fn geosearch_inner(db: &mut Database, args: &[Frame], _store_mode: bool) -> (Vec (matches, Frame::Array(results.into())) } + +// --------------------------------------------------------------------------- +// Read-only twins for the shared-lock (dispatch_read) path +// --------------------------------------------------------------------------- +// +// GEO data is stored as a sorted set: all twins use `get_sorted_set_if_alive`. + +/// GEOPOS key member [member …] — read-only twin. +pub fn geopos_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 2 { + return err_wrong_args("GEOPOS"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("GEOPOS"), + }; + + // Ref accessor: handles every encoding (BPTree, Listpack from RDB load, + // Legacy) — the BPTree-only accessor would treat a listpack zset as missing. + let scores: Vec> = { + let zref = match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(z)) => Some(z), + Ok(None) => None, + Err(e) => return e, + }; + args[1..] + .iter() + .map(|arg| { + let member = extract_bytes(arg)?; + zref.as_ref()?.score(member) + }) + .collect() + }; + + let results: Vec = scores + .into_iter() + .map(|opt_score| match opt_score { + Some(score) => { + let (lon, lat) = geohash_decode(score); + Frame::Array( + vec![ + Frame::BulkString(Bytes::from(fmt_geo_coord(lon))), + Frame::BulkString(Bytes::from(fmt_geo_coord(lat))), + ] + .into(), + ) + } + None => Frame::Null, + }) + .collect(); + + Frame::Array(results.into()) +} + +/// GEODIST key member1 member2 [M|KM|FT|MI] — read-only twin. +pub fn geodist_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 3 { + return err_wrong_args("GEODIST"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("GEODIST"), + }; + let m1 = match extract_bytes(&args[1]) { + Some(m) => m, + None => return err_wrong_args("GEODIST"), + }; + let m2 = match extract_bytes(&args[2]) { + Some(m) => m, + None => return err_wrong_args("GEODIST"), + }; + let unit = if args.len() >= 4 { + match extract_bytes(&args[3]) { + Some(u) => { + if parse_unit(u).is_none() { + return Frame::Error(Bytes::from_static( + b"ERR unsupported unit provided. please use M, KM, FT, MI", + )); + } + u + } + None => b"m" as &[u8], + } + } else { + b"m" + }; + + // Ref accessor: every encoding, borrowed lookups — no map clone. + let zref = match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(z)) => z, + Ok(None) => return Frame::Null, + Err(e) => return e, + }; + + let score1 = match zref.score(m1) { + Some(s) => s, + None => return Frame::Null, + }; + let score2 = match zref.score(m2) { + Some(s) => s, + None => return Frame::Null, + }; + + let (lon1, lat1) = geohash_decode(score1); + let (lon2, lat2) = geohash_decode(score2); + let dist = haversine_distance(lon1, lat1, lon2, lat2); + let converted = convert_distance(dist, unit); + + Frame::BulkString(Bytes::from(format!("{:.4}", converted))) +} + +/// GEOHASH key member [member …] — read-only twin. +pub fn geohash_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 2 { + return err_wrong_args("GEOHASH"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("GEOHASH"), + }; + + // Ref accessor: every encoding, borrowed lookups — no map clone. + let zref = match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(z)) => Some(z), + Ok(None) => None, + Err(e) => return e, + }; + + let mut results = Vec::with_capacity(args.len() - 1); + for arg in &args[1..] { + let member = match extract_bytes(arg) { + Some(m) => m, + None => { + results.push(Frame::Null); + continue; + } + }; + + match zref.as_ref().and_then(|z| z.score(member)) { + Some(score) => { + let hash_str = geohash_to_string(score); + results.push(Frame::BulkString(Bytes::from(hash_str))); + } + None => results.push(Frame::Null), + } + } + + Frame::Array(results.into()) +} + +/// GEOSEARCH key FROMMEMBER|FROMLONLAT … BYRADIUS|BYBOX … — read-only twin. +/// +/// Shares the full parse/filter logic with the mutable path via +/// `geosearch_core`. The ref accessor handles every encoding (BPTree, +/// Listpack from RDB load, Legacy); BPTree/Legacy borrow their member map +/// (zero copy), listpacks materialize a small bounded map. +pub fn geosearch_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 6 { + return err_wrong_args("GEOSEARCH"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("GEOSEARCH"), + }; + let owned: std::collections::HashMap; + let members_opt = match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(zref)) => match zref.members_map() { + Some(m) => Some(m), + None => { + owned = zref.entries_sorted().into_iter().collect(); + Some(&owned) + } + }, + Ok(None) => None, + Err(e) => return e, + }; + let (_matches, results) = geosearch_core(members_opt, args); + results +} diff --git a/src/command/geo/mod.rs b/src/command/geo/mod.rs index 4f8fd9ff..18602d33 100644 --- a/src/command/geo/mod.rs +++ b/src/command/geo/mod.rs @@ -8,80 +8,128 @@ use std::f64::consts::PI; // Geohash encoding/decoding (52-bit integer, Redis-compatible) // --------------------------------------------------------------------------- +// Redis (geohash.h) encodes the 52-bit zset SCORE with the WGS84 web-mercator +// latitude clamp ±85.05112878 — NOT ±90. Only the GEOHASH string command +// re-encodes the decoded cell center with standard geohash bounds (lat ±90) +// before base32-ing it. Both bounds are needed for byte parity: +// score bounds wrong → GEOPOS/GEODIST cell centers diverge from Redis; +// string bounds wrong → GEOHASH characters diverge. const GEO_LAT_MIN: f64 = -85.05112878; const GEO_LAT_MAX: f64 = 85.05112878; const GEO_LON_MIN: f64 = -180.0; const GEO_LON_MAX: f64 = 180.0; const GEO_STEP_MAX: u8 = 26; // 52-bit precision -/// Encode longitude/latitude into a 52-bit geohash stored as f64 score. -pub(crate) fn geohash_encode(lon: f64, lat: f64) -> f64 { - let mut lat_range = (GEO_LAT_MIN, GEO_LAT_MAX); - let mut lon_range = (GEO_LON_MIN, GEO_LON_MAX); - let mut hash: u64 = 0; - - for i in 0..GEO_STEP_MAX { - // Longitude bit - let mid = (lon_range.0 + lon_range.1) / 2.0; - if lon >= mid { - hash |= 1 << (51 - i * 2); - lon_range.0 = mid; - } else { - lon_range.1 = mid; - } - // Latitude bit - let mid = (lat_range.0 + lat_range.1) / 2.0; - if lat >= mid { - hash |= 1 << (50 - i * 2); - lat_range.0 = mid; - } else { - lat_range.1 = mid; - } - } +/// Interleave the low 26 bits of `xlo` (even positions) and `ylo` (odd +/// positions) — Redis's interleave64 (Morton code). `xlo` = latitude cells, +/// `ylo` = longitude cells, so longitude owns the MSB (bit 51). +fn interleave64(xlo: u32, ylo: u32) -> u64 { + const B: [u64; 5] = [ + 0x5555555555555555, + 0x3333333333333333, + 0x0F0F0F0F0F0F0F0F, + 0x00FF00FF00FF00FF, + 0x0000FFFF0000FFFF, + ]; + const S: [u32; 5] = [1, 2, 4, 8, 16]; + let mut x = xlo as u64; + let mut y = ylo as u64; + x = (x | (x << S[4])) & B[4]; + x = (x | (x << S[3])) & B[3]; + x = (x | (x << S[2])) & B[2]; + x = (x | (x << S[1])) & B[1]; + x = (x | (x << S[0])) & B[0]; + y = (y | (y << S[4])) & B[4]; + y = (y | (y << S[3])) & B[3]; + y = (y | (y << S[2])) & B[2]; + y = (y | (y << S[1])) & B[1]; + y = (y | (y << S[0])) & B[0]; + x | (y << 1) +} + +/// Inverse of interleave64: extract the even bits — Redis's deinterleave64 +/// helper (call once on `bits` for latitude, once on `bits >> 1` for +/// longitude). +fn deinterleave_even(mut x: u64) -> u32 { + const B: [u64; 6] = [ + 0x5555555555555555, + 0x3333333333333333, + 0x0F0F0F0F0F0F0F0F, + 0x00FF00FF00FF00FF, + 0x0000FFFF0000FFFF, + 0x00000000FFFFFFFF, + ]; + const S: [u32; 6] = [0, 1, 2, 4, 8, 16]; + x &= B[0]; + x = (x | (x >> S[1])) & B[1]; + x = (x | (x >> S[2])) & B[2]; + x = (x | (x >> S[3])) & B[3]; + x = (x | (x >> S[4])) & B[4]; + x = (x | (x >> S[5])) & B[5]; + x as u32 +} + +/// Redis geohashEncode for arbitrary ranges: normalize, scale by 2^26 +/// (truncating cast, exactly like the C double→uint32 conversion), interleave. +fn geohash_encode_raw(lon: f64, lat: f64, lat_min: f64, lat_max: f64) -> u64 { + let lat_offset = (lat - lat_min) / (lat_max - lat_min) * (1u64 << GEO_STEP_MAX) as f64; + let lon_offset = + (lon - GEO_LON_MIN) / (GEO_LON_MAX - GEO_LON_MIN) * (1u64 << GEO_STEP_MAX) as f64; + interleave64(lat_offset as u32, lon_offset as u32) +} - hash as f64 +/// Encode longitude/latitude into the 52-bit geohash score (Redis score bounds). +pub(crate) fn geohash_encode(lon: f64, lat: f64) -> f64 { + geohash_encode_raw(lon, lat, GEO_LAT_MIN, GEO_LAT_MAX) as f64 } -/// Decode a 52-bit geohash score back to (longitude, latitude). +/// Decode a 52-bit geohash score to the cell-center (longitude, latitude), +/// using Redis's direct min/max arithmetic (geohashDecode + center) so the +/// resulting f64s are bit-identical to Redis's GEOPOS output. pub(crate) fn geohash_decode(score: f64) -> (f64, f64) { let hash = score as u64; - let mut lat_range = (GEO_LAT_MIN, GEO_LAT_MAX); - let mut lon_range = (GEO_LON_MIN, GEO_LON_MAX); + let ilato = deinterleave_even(hash) as f64; + let ilono = deinterleave_even(hash >> 1) as f64; + let scale = (1u64 << GEO_STEP_MAX) as f64; - for i in 0..GEO_STEP_MAX { - // Longitude bit - if hash & (1 << (51 - i * 2)) != 0 { - lon_range.0 = (lon_range.0 + lon_range.1) / 2.0; - } else { - lon_range.1 = (lon_range.0 + lon_range.1) / 2.0; - } - // Latitude bit - if hash & (1 << (50 - i * 2)) != 0 { - lat_range.0 = (lat_range.0 + lat_range.1) / 2.0; - } else { - lat_range.1 = (lat_range.0 + lat_range.1) / 2.0; - } - } + let lat_min = GEO_LAT_MIN + (ilato / scale) * (GEO_LAT_MAX - GEO_LAT_MIN); + let lat_max = GEO_LAT_MIN + ((ilato + 1.0) / scale) * (GEO_LAT_MAX - GEO_LAT_MIN); + let lon_min = GEO_LON_MIN + (ilono / scale) * (GEO_LON_MAX - GEO_LON_MIN); + let lon_max = GEO_LON_MIN + ((ilono + 1.0) / scale) * (GEO_LON_MAX - GEO_LON_MIN); - let lon = (lon_range.0 + lon_range.1) / 2.0; - let lat = (lat_range.0 + lat_range.1) / 2.0; - (lon, lat) + ((lon_min + lon_max) / 2.0, (lat_min + lat_max) / 2.0) } -/// Convert a 52-bit integer geohash to the 11-character base32 string Redis uses. +/// The 11-character base32 geohash string, Redis-exact: decode the score +/// (score bounds), re-encode the cell center with STANDARD geohash bounds +/// (lat ±90), then emit 11 chars from the top 50 bits — the 11th character +/// is always '0' (Redis emits index 0 once the bit budget is exhausted). pub(crate) fn geohash_to_string(score: f64) -> String { const ALPHABET: &[u8] = b"0123456789bcdefghjkmnpqrstuvwxyz"; - let hash = score as u64; - // Redis uses 11 characters (55 bits, but we only have 52 so pad with 0) - let padded = hash << 3; // shift left 3 to fill 55 bits + let (lon, lat) = geohash_decode(score); + let bits = geohash_encode_raw(lon, lat, -90.0, 90.0); let mut result = [0u8; 11]; - for i in 0..11 { - let idx = ((padded >> (50 - i * 5)) & 0x1F) as usize; - result[i] = ALPHABET[idx]; + for (i, slot) in result.iter_mut().enumerate() { + let used = (i + 1) * 5; + let idx = if used <= 52 { + ((bits >> (52 - used)) & 0x1F) as usize + } else { + 0 + }; + *slot = ALPHABET[idx]; } String::from_utf8_lossy(&result).to_string() } +/// Format a coordinate the way Redis 8 replies to GEOPOS / WITHCOORD. +/// Redis's d2string uses fpconv_dtoa (grisu2) — the SHORTEST decimal that +/// round-trips to the same f64 — which is exactly Rust's `{}` Display for +/// f64 (verified byte-identical against redis-server 8.x for the geohash +/// cell centers the consistency suite compares). +pub(crate) fn fmt_geo_coord(v: f64) -> String { + format!("{v}") +} + // --------------------------------------------------------------------------- // Haversine distance // --------------------------------------------------------------------------- @@ -89,15 +137,22 @@ pub(crate) fn geohash_to_string(score: f64) -> String { const EARTH_RADIUS_M: f64 = 6372797.560856; /// Haversine distance in meters between two (lon, lat) pairs. +/// +/// Operation order matches Redis's geohashGetDistance exactly (radians first, +/// then differences; u/v half-angle sines; single asin) so the resulting f64 +/// is bit-identical and GEODIST's %.4f output matches byte-for-byte. pub(crate) fn haversine_distance(lon1: f64, lat1: f64, lon2: f64, lat2: f64) -> f64 { - let lat1_r = lat1 * PI / 180.0; - let lat2_r = lat2 * PI / 180.0; - let dlat = (lat2 - lat1) * PI / 180.0; - let dlon = (lon2 - lon1) * PI / 180.0; - - let a = (dlat / 2.0).sin().powi(2) + lat1_r.cos() * lat2_r.cos() * (dlon / 2.0).sin().powi(2); - let c = 2.0 * a.sqrt().asin(); - EARTH_RADIUS_M * c + let lat1r = lat1 * PI / 180.0; + let lon1r = lon1 * PI / 180.0; + let lat2r = lat2 * PI / 180.0; + let lon2r = lon2 * PI / 180.0; + let u = ((lat2r - lat1r) / 2.0).sin(); + let v = ((lon2r - lon1r) / 2.0).sin(); + if u == 0.0 && v == 0.0 { + return 0.0; + } + let a = u * u + lat1r.cos() * lat2r.cos() * v * v; + 2.0 * EARTH_RADIUS_M * a.sqrt().asin() } /// Convert meters to the specified unit. @@ -139,6 +194,37 @@ mod tests { Frame::BulkString(Bytes::copy_from_slice(s)) } + /// Byte-parity pins vs redis-server 8.x (values captured by + /// scripts/test-consistency.sh on moon-dev): score-bounds decode, + /// %.17g coordinate formatting, ±90 string re-encode, and the Redis + /// haversine op order must all match for these to hold. + #[test] + fn test_redis_parity_palermo() { + let score_p = geohash_encode(13.361389, 38.115556); + let score_c = geohash_encode(15.087269, 37.502669); + + // GEOPOS Palermo + let (lon, lat) = geohash_decode(score_p); + assert_eq!(fmt_geo_coord(lon), "13.361389338970184"); + assert_eq!(fmt_geo_coord(lat), "38.1155563954963"); + + // GEOHASH Palermo — 11 chars, Redis's 11th is always '0' + assert_eq!(geohash_to_string(score_p), "sqc8b49rny0"); + + // GEODIST Palermo Catania in m and km + let (lon2, lat2) = geohash_decode(score_c); + let d = haversine_distance(lon, lat, lon2, lat2); + assert_eq!(format!("{:.4}", convert_distance(d, b"m")), "166274.1516"); + assert_eq!(format!("{:.4}", convert_distance(d, b"km")), "166.2742"); + } + + #[test] + fn test_fmt_geo_coord_edges() { + assert_eq!(fmt_geo_coord(0.0), "0"); + assert_eq!(fmt_geo_coord(0.5), "0.5"); + assert_eq!(fmt_geo_coord(-13.361389338970184), "-13.361389338970184"); + } + #[test] fn test_geohash_roundtrip() { // Rome coordinates diff --git a/src/command/graph/mod.rs b/src/command/graph/mod.rs index 252d75ed..1a5d1655 100644 --- a/src/command/graph/mod.rs +++ b/src/command/graph/mod.rs @@ -113,6 +113,29 @@ pub fn dispatch_graph_command(store: &mut GraphStore, command: &Frame) -> Frame return resp; } + // TEMPORAL.INVALIDATE mutates a graph entity, so it must execute on the + // shard that owns the graph name; multi-shard routing ships it here via + // ShardMessage::GraphCommand. wall_ms is captured at this handler entry + // (on the owning shard) — apply_invalidate never calls NOW() itself. + if crate::command::temporal::is_temporal_invalidate(cmd) { + return match crate::command::temporal::validate_invalidate(args) { + Ok((entity_id, is_node, graph_name)) => { + let wall_ms = crate::command::temporal::capture_wall_ms(); + match crate::command::temporal::apply_invalidate( + store, + entity_id, + is_node, + &graph_name, + wall_ms, + ) { + Ok(()) => Frame::SimpleString(Bytes::from_static(b"OK")), + Err(e) => Frame::Error(Bytes::from_static(e)), + } + } + Err(e) => e, + }; + } + if is_graph_write_cmd(cmd) { dispatch_graph_write(store, cmd, args) } else { diff --git a/src/command/key.rs b/src/command/key.rs index 55e32c38..6c1b22bc 100644 --- a/src/command/key.rs +++ b/src/command/key.rs @@ -374,12 +374,16 @@ pub fn time() -> Frame { let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default(); - let secs = now.as_secs(); - let micros = now.subsec_micros(); + let mut secs_buf = itoa::Buffer::new(); + let mut micros_buf = itoa::Buffer::new(); Frame::Array( vec![ - Frame::BulkString(Bytes::from(secs.to_string())), - Frame::BulkString(Bytes::from(micros.to_string())), + Frame::BulkString(Bytes::copy_from_slice( + secs_buf.format(now.as_secs()).as_bytes(), + )), + Frame::BulkString(Bytes::copy_from_slice( + micros_buf.format(now.subsec_micros()).as_bytes(), + )), ] .into(), ) @@ -1185,6 +1189,89 @@ pub fn scan_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { ]) } +// --------------------------------------------------------------------------- +// New read-only twins (dispatch_read path) +// --------------------------------------------------------------------------- + +/// EXPIRETIME key — read-only twin. +/// +/// Returns -2 if missing/expired, -1 if no TTL, else absolute Unix seconds. +pub fn expiretime_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() != 1 { + return err_wrong_args("EXPIRETIME"); + } + let key = match extract_key(&args[0]) { + Some(k) => k, + None => return err_wrong_args("EXPIRETIME"), + }; + let base_ts = db.base_timestamp(); + match db.get_if_alive(key, now_ms) { + None => Frame::Integer(-2), + Some(entry) => { + if !entry.has_expiry() { + Frame::Integer(-1) + } else { + Frame::Integer((entry.expires_at_ms(base_ts) / 1000) as i64) + } + } + } +} + +/// PEXPIRETIME key — read-only twin. +/// +/// Returns -2 if missing/expired, -1 if no TTL, else absolute Unix milliseconds. +pub fn pexpiretime_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() != 1 { + return err_wrong_args("PEXPIRETIME"); + } + let key = match extract_key(&args[0]) { + Some(k) => k, + None => return err_wrong_args("PEXPIRETIME"), + }; + let base_ts = db.base_timestamp(); + match db.get_if_alive(key, now_ms) { + None => Frame::Integer(-2), + Some(entry) => { + if !entry.has_expiry() { + Frame::Integer(-1) + } else { + Frame::Integer(entry.expires_at_ms(base_ts) as i64) + } + } + } +} + +/// RANDOMKEY — read-only twin. +/// +/// Returns a random alive key, or Null if all keys are expired/absent. +/// Uses `random_key()` which already filters expired keys without deleting them. +pub fn randomkey_readonly(db: &Database, _args: &[Frame], _now_ms: u64) -> Frame { + match db.random_key() { + Some(key) => Frame::BulkString(key), + None => Frame::Null, + } +} + +/// TOUCH key [key …] — read-only twin. +/// +/// Counts alive keys. Does NOT update LRU/access metadata (contract M2). +pub fn touch_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + if args.is_empty() { + return err_wrong_args("TOUCH"); + } + let mut count = 0i64; + for arg in args { + let key = match extract_key(arg) { + Some(k) => k, + None => continue, + }; + if db.exists_if_alive(key, now_ms) { + count += 1; + } + } + Frame::Integer(count) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/command/mod.rs b/src/command/mod.rs index 7f8813cb..ecface75 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -996,6 +996,7 @@ pub fn is_dispatch_read_supported(cmd: &[u8]) -> bool { matches!( (len, b0), (3, b'g') // GET + | (3, b'l') // LCS | (3, b't') // TTL | (4, b'e') // ECHO | (4, b'h') // HGET, HLEN, HTTL @@ -1005,32 +1006,48 @@ pub fn is_dispatch_read_supported(cmd: &[u8]) -> bool { | (4, b'm') // MGET | (4, b'p') // PTTL, PING | (4, b's') // SCAN - | (4, b't') // TYPE + | (4, b't') // TYPE, TIME + | (4, b'w') // WAIT + | (4, b'x') // XLEN | (5, b'd') // DEBUG (OBJECT/SLEEP/HELP — read-only on the data plane) | (5, b'h') // HMGET, HKEYS, HVALS, HSCAN, HPTTL | (5, b's') // SCARD, SDIFF, SSCAN - | (5, b'z') // ZCARD, ZRANK, ZSCAN + | (5, b't') // TOUCH + | (5, b'x') // XREAD, XINFO + | (5, b'z') // ZCARD, ZRANK, ZSCAN, ZDIFF | (6, b'b') // BITPOS | (6, b'd') // DBSIZE | (6, b'e') // EXISTS - | (6, b'g') // GETBIT - | (6, b'l') // LRANGE, LINDEX + | (6, b'g') // GETBIT, GEOPOS + | (6, b'l') // LRANGE, LINDEX, LOLWUT | (6, b'm') // MEMORY | (6, b'o') // OBJECT | (6, b's') // STRLEN, SUBSTR, SINTER, SUNION - | (6, b'z') // ZSCORE, ZRANGE, ZCOUNT + | (6, b'x') // XRANGE + | (6, b'z') // ZSCORE, ZRANGE, ZCOUNT, ZINTER, ZUNION | (7, b'c') // COMMAND + | (7, b'g') // GEODIST, GEOHASH | (7, b'h') // HGETALL, HEXISTS | (7, b'p') // PFCOUNT + | (7, b's') // SLOWLOG + | (7, b'z') // ZMSCORE | (8, b'b') // BITCOUNT | (8, b'g') // GETRANGE | (8, b's') // SMEMBERS + | (8, b'x') // XPENDING | (8, b'z') // ZREVRANK + | (9, b'g') // GEOSEARCH + | (9, b'r') // RANDOMKEY | (9, b's') // SISMEMBER + | (9, b'x') // XREVRANGE | (9, b'z') // ZREVRANGE, ZLEXCOUNT + | (10, b'e') // EXPIRETIME | (10, b'h') // HRANDFIELD | (10, b's') // SMISMEMBER, SINTERCARD + | (10, b'z') // ZINTERCARD | (11, b'h') // HEXPIRETIME + | (11, b'p') // PEXPIRETIME + | (11, b'z') // ZRANDMEMBER | (11, b's') // SRANDMEMBER | (12, b'h') // HPEXPIRETIME | (13, b'z') // ZRANGEBYSCORE @@ -1146,10 +1163,13 @@ fn dispatch_read_inner(db: &Database, cmd: &[u8], args: &[Frame], now_ms: u64) - } } (4, b't') => { - // TYPE + // TYPE TIME if cmd.eq_ignore_ascii_case(b"TYPE") { return resp(key::type_cmd_readonly(db, args, now_ms)); } + if cmd.eq_ignore_ascii_case(b"TIME") { + return resp(key::time()); + } } (5, b'd') => { // DEBUG (OBJECT / SLEEP / HELP — none mutate) @@ -1187,14 +1207,23 @@ fn dispatch_read_inner(db: &Database, cmd: &[u8], args: &[Frame], now_ms: u64) - return resp(set::sscan_readonly(db, args, now_ms)); } } + (5, b't') => { + // TOUCH + if cmd.eq_ignore_ascii_case(b"TOUCH") { + return resp(key::touch_readonly(db, args, now_ms)); + } + } (5, b'z') => { - // ZCARD ZRANK ZSCAN + // ZCARD ZRANK ZSCAN ZDIFF if cmd.eq_ignore_ascii_case(b"ZCARD") { return resp(sorted_set::zcard_readonly(db, args, now_ms)); } if cmd.eq_ignore_ascii_case(b"ZRANK") { return resp(sorted_set::zrank_readonly(db, args, now_ms)); } + if cmd.eq_ignore_ascii_case(b"ZDIFF") { + return resp(sorted_set::zdiff_readonly(db, args, now_ms)); + } if cmd.eq_ignore_ascii_case(b"ZSCAN") { return resp(sorted_set::zscan_readonly(db, args, now_ms)); } @@ -1218,19 +1247,27 @@ fn dispatch_read_inner(db: &Database, cmd: &[u8], args: &[Frame], now_ms: u64) - } } (6, b'g') => { - // GETBIT + // GETBIT GEOPOS if cmd.eq_ignore_ascii_case(b"GETBIT") { return resp(string::getbit_readonly(db, args, now_ms)); } + if cmd.eq_ignore_ascii_case(b"GEOPOS") { + return resp(geo::geopos_readonly(db, args, now_ms)); + } } (6, b'l') => { - // LRANGE LINDEX + // LRANGE LINDEX LOLWUT if cmd.eq_ignore_ascii_case(b"LRANGE") { return resp(list::lrange_readonly(db, args, now_ms)); } if cmd.eq_ignore_ascii_case(b"LINDEX") { return resp(list::lindex_readonly(db, args, now_ms)); } + if cmd.eq_ignore_ascii_case(b"LOLWUT") { + return resp(Frame::BulkString(Bytes::from_static( + concat!("Moon v", env!("CARGO_PKG_VERSION"), "\n").as_bytes(), + ))); + } } (6, b'm') => { // MEMORY (USAGE / STATS / DOCTOR / HELP — all read-only) @@ -1260,7 +1297,7 @@ fn dispatch_read_inner(db: &Database, cmd: &[u8], args: &[Frame], now_ms: u64) - } } (6, b'z') => { - // ZSCORE ZRANGE ZCOUNT + // ZSCORE ZRANGE ZCOUNT ZINTER ZUNION if cmd.eq_ignore_ascii_case(b"ZSCORE") { return resp(sorted_set::zscore_readonly(db, args, now_ms)); } @@ -1270,6 +1307,12 @@ fn dispatch_read_inner(db: &Database, cmd: &[u8], args: &[Frame], now_ms: u64) - if cmd.eq_ignore_ascii_case(b"ZCOUNT") { return resp(sorted_set::zcount_readonly(db, args, now_ms)); } + if cmd.eq_ignore_ascii_case(b"ZINTER") { + return resp(sorted_set::zinter_readonly(db, args, now_ms)); + } + if cmd.eq_ignore_ascii_case(b"ZUNION") { + return resp(sorted_set::zunion_readonly(db, args, now_ms)); + } } (7, b'c') => { // COMMAND @@ -1379,6 +1422,115 @@ fn dispatch_read_inner(db: &Database, cmd: &[u8], args: &[Frame], now_ms: u64) - return resp(sorted_set::zrevrangebyscore_readonly(db, args, now_ms)); } } + // ---- new arms (contract v2): buckets that don't conflict with pre-existing ones ---- + (3, b'l') => { + // LCS + if cmd.eq_ignore_ascii_case(b"LCS") { + return resp(string::lcs_readonly(db, args, now_ms)); + } + } + (4, b'w') => { + // WAIT: no replication — always 0 (mirrors handler_single.rs:833). + // WAIT is in extract_primary_key's keyless table, so it routes + // locally: this arm fires on the local read path only, never the + // cross-shard fast path. + if cmd.eq_ignore_ascii_case(b"WAIT") { + return resp(Frame::Integer(0)); + } + } + (4, b'x') => { + // XLEN + if cmd.eq_ignore_ascii_case(b"XLEN") { + return resp(stream::xlen_readonly(db, args, now_ms)); + } + } + (5, b'x') => { + // XREAD XINFO + if cmd.eq_ignore_ascii_case(b"XREAD") { + return resp(stream::xread_readonly(db, args, now_ms)); + } + if cmd.eq_ignore_ascii_case(b"XINFO") { + return resp(stream::xinfo_readonly(db, args, now_ms)); + } + } + (6, b'x') => { + // XRANGE + if cmd.eq_ignore_ascii_case(b"XRANGE") { + return resp(stream::xrange_readonly(db, args, now_ms)); + } + } + (7, b'g') => { + // GEODIST GEOHASH + if cmd.eq_ignore_ascii_case(b"GEODIST") { + return resp(geo::geodist_readonly(db, args, now_ms)); + } + if cmd.eq_ignore_ascii_case(b"GEOHASH") { + return resp(geo::geohash_readonly(db, args, now_ms)); + } + } + (7, b's') => { + // SLOWLOG — named exception: RESET mutates the global ring (own sync, not Database) + if cmd.eq_ignore_ascii_case(b"SLOWLOG") { + return resp(crate::admin::slowlog::handle_slowlog( + crate::admin::metrics_setup::global_slowlog(), + args, + )); + } + } + (7, b'z') => { + // ZMSCORE + if cmd.eq_ignore_ascii_case(b"ZMSCORE") { + return resp(sorted_set::zmscore_readonly(db, args, now_ms)); + } + } + (8, b'x') => { + // XPENDING (8 bytes) + if cmd.eq_ignore_ascii_case(b"XPENDING") { + return resp(stream::xpending_readonly(db, args, now_ms)); + } + } + (9, b'g') => { + // GEOSEARCH (9 bytes) + if cmd.eq_ignore_ascii_case(b"GEOSEARCH") { + return resp(geo::geosearch_readonly(db, args, now_ms)); + } + } + (9, b'r') => { + // RANDOMKEY (9 bytes) + if cmd.eq_ignore_ascii_case(b"RANDOMKEY") { + return resp(key::randomkey_readonly(db, args, now_ms)); + } + } + (9, b'x') => { + // XREVRANGE (9 bytes) + if cmd.eq_ignore_ascii_case(b"XREVRANGE") { + return resp(stream::xrevrange_readonly(db, args, now_ms)); + } + } + (10, b'e') => { + // EXPIRETIME (10 bytes) + if cmd.eq_ignore_ascii_case(b"EXPIRETIME") { + return resp(key::expiretime_readonly(db, args, now_ms)); + } + } + (10, b'z') => { + // ZINTERCARD (10 bytes) + if cmd.eq_ignore_ascii_case(b"ZINTERCARD") { + return resp(sorted_set::zintercard_readonly(db, args, now_ms)); + } + } + (11, b'p') => { + // PEXPIRETIME (11 bytes) + if cmd.eq_ignore_ascii_case(b"PEXPIRETIME") { + return resp(key::pexpiretime_readonly(db, args, now_ms)); + } + } + (11, b'z') => { + // ZRANDMEMBER (11 bytes) + if cmd.eq_ignore_ascii_case(b"ZRANDMEMBER") { + return resp(sorted_set::zrandmember_readonly(db, args, now_ms)); + } + } _ => {} } @@ -1919,6 +2071,32 @@ mod tests { b"EXISTS", b"GETRANGE", b"COMMAND", + // contract v2: the 25 new read arms + b"LCS", + b"WAIT", + b"XLEN", + b"XREAD", + b"XRANGE", + b"XREVRANGE", + b"XINFO", + b"XPENDING", + b"GEOPOS", + b"GEODIST", + b"GEOHASH", + b"GEOSEARCH", + b"ZDIFF", + b"ZINTER", + b"ZUNION", + b"ZINTERCARD", + b"ZRANDMEMBER", + b"ZMSCORE", + b"RANDOMKEY", + b"EXPIRETIME", + b"PEXPIRETIME", + b"TOUCH", + b"LOLWUT", + b"TIME", + b"SLOWLOG", ] { assert!( is_dispatch_read_supported(cmd), @@ -1944,8 +2122,9 @@ mod tests { // BGSAVE shares (6, b'b') bucket with BITPOS — prefilter is coarse // OBJECT moved to the read path (it is a pure read; the missing // arm made it unreachable over the wire). - b"WAIT", - b"RANDOMKEY", + // NOTE: WAIT and RANDOMKEY were here before contract v2; they now + // have dispatch_read arms and gate buckets — moved to the supported + // test above. Leaving this comment so git-blame is clear. ] { assert!( !is_dispatch_read_supported(cmd), diff --git a/src/command/sorted_set/sorted_set_read.rs b/src/command/sorted_set/sorted_set_read.rs index a58ad576..a5548f93 100644 --- a/src/command/sorted_set/sorted_set_read.rs +++ b/src/command/sorted_set/sorted_set_read.rs @@ -1386,6 +1386,38 @@ fn collect_source_sets( Ok(source_data) } +/// Read-only twin: collect source sets using `get_sorted_set_if_alive`. +/// +/// Compact (listpack) encodings return an empty map — callers see an absent +/// set, which is correct because compact sets are upgraded to BPTree on first +/// write access. +fn collect_source_sets_readonly<'a>( + db: &'a Database, + keys: &[Bytes], + now_ms: u64, +) -> Result>>, Frame> { + use std::borrow::Cow; + let mut source_data: Vec>> = Vec::with_capacity(keys.len()); + for key in keys { + // get_sorted_set_ref_if_alive handles ALL encodings (BPTree, Listpack + // from RDB load, Legacy) — the BPTree-only accessor would silently + // treat a listpack zset as missing. BPTree/Legacy borrow their map + // (no clone); listpacks are small by definition, so materializing an + // owned map for them is bounded. + match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(zref)) => match zref.members_map() { + Some(m) => source_data.push(Cow::Borrowed(m)), + None => source_data.push(Cow::Owned(zref.entries_sorted().into_iter().collect())), + }, + Ok(None) => { + source_data.push(Cow::Owned(HashMap::new())); + } + Err(e) => return Err(e), + } + } + Ok(source_data) +} + /// Format a result map into a Frame::Array, optionally with scores. fn result_map_to_frame(result: &HashMap, withscores: bool) -> Frame { let mut entries: Vec<(&Bytes, f64)> = result.iter().map(|(m, s)| (m, *s)).collect(); @@ -1734,3 +1766,326 @@ pub fn zrandmember(db: &mut Database, args: &[Frame]) -> Frame { Frame::Array(result.into()) } } + +// --------------------------------------------------------------------------- +// Read-only twins for the shared-lock (dispatch_read) path +// --------------------------------------------------------------------------- + +/// ZDIFF numkeys key [key …] [WITHSCORES] — read-only twin. +pub fn zdiff_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + let (keys, _, _, withscores) = match parse_setop_args(args, "ZDIFF", false) { + Ok(v) => v, + Err(e) => return e, + }; + let source_data = match collect_source_sets_readonly(db, &keys, now_ms) { + Ok(v) => v, + Err(e) => return e, + }; + let mut result_map: HashMap = HashMap::new(); + if let Some(first) = source_data.first() { + 'outer: for (member, score) in first.iter() { + for src in source_data.iter().skip(1) { + if src.contains_key(member) { + continue 'outer; + } + } + result_map.insert(member.clone(), *score); + } + } + result_map_to_frame(&result_map, withscores) +} + +/// ZUNION numkeys key [key …] [WEIGHTS …] [AGGREGATE …] [WITHSCORES] — read-only twin. +pub fn zunion_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + let (keys, weights, aggregate, withscores) = match parse_setop_args(args, "ZUNION", true) { + Ok(v) => v, + Err(e) => return e, + }; + let source_data = match collect_source_sets_readonly(db, &keys, now_ms) { + Ok(v) => v, + Err(e) => return e, + }; + let mut result_map: HashMap = HashMap::new(); + for (idx, src) in source_data.iter().enumerate() { + for (member, score) in src.iter() { + let weighted = *score * weights[idx]; + result_map + .entry(member.clone()) + .and_modify(|existing| { + *existing = match aggregate { + AggregateOp::Sum => *existing + weighted, + AggregateOp::Min => existing.min(weighted), + AggregateOp::Max => existing.max(weighted), + }; + }) + .or_insert(weighted); + } + } + result_map_to_frame(&result_map, withscores) +} + +/// ZINTER numkeys key [key …] [WEIGHTS …] [AGGREGATE …] [WITHSCORES] — read-only twin. +pub fn zinter_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + let (keys, weights, aggregate, withscores) = match parse_setop_args(args, "ZINTER", true) { + Ok(v) => v, + Err(e) => return e, + }; + let source_data = match collect_source_sets_readonly(db, &keys, now_ms) { + Ok(v) => v, + Err(e) => return e, + }; + let mut result_map: HashMap = HashMap::new(); + if let Some(first) = source_data.first() { + for (member, score) in first.iter() { + let weighted = *score * weights[0]; + let mut final_score = weighted; + let mut in_all = true; + for (idx, src) in source_data.iter().enumerate().skip(1) { + match src.get(member) { + Some(s) => { + let ws = *s * weights[idx]; + final_score = match aggregate { + AggregateOp::Sum => final_score + ws, + AggregateOp::Min => final_score.min(ws), + AggregateOp::Max => final_score.max(ws), + }; + } + None => { + in_all = false; + break; + } + } + } + if in_all { + result_map.insert(member.clone(), final_score); + } + } + } + result_map_to_frame(&result_map, withscores) +} + +/// ZINTERCARD numkeys key [key …] [LIMIT limit] — read-only twin. +pub fn zintercard_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + if args.is_empty() { + return err_wrong_args("ZINTERCARD"); + } + let numkeys_bytes = match extract_bytes(&args[0]) { + Some(b) => b, + None => return err_wrong_args("ZINTERCARD"), + }; + let numkeys: usize = match std::str::from_utf8(numkeys_bytes) + .ok() + .and_then(|s| s.parse().ok()) + { + Some(n) if n > 0 => n, + _ => return err("ERR numkeys can't be non-positive value"), + }; + if args.len() < 1 + numkeys { + return err_wrong_args("ZINTERCARD"); + } + let keys: Vec = (0..numkeys) + .map(|j| { + extract_bytes(&args[1 + j]) + .cloned() + .unwrap_or_else(Bytes::new) + }) + .collect(); + let mut limit: usize = 0; + let mut i = 1 + numkeys; + while i < args.len() { + let opt = match extract_bytes(&args[i]) { + Some(b) => b.as_ref(), + None => { + i += 1; + continue; + } + }; + if opt.eq_ignore_ascii_case(b"LIMIT") { + if i + 1 >= args.len() { + return err_wrong_args("ZINTERCARD"); + } + let lb = match extract_bytes(&args[i + 1]) { + Some(b) => b, + None => return err_wrong_args("ZINTERCARD"), + }; + limit = match std::str::from_utf8(lb).ok().and_then(|s| s.parse().ok()) { + Some(v) => v, + None => return err("ERR value is not an integer or out of range"), + }; + i += 2; + } else { + i += 1; + } + } + let source_data = match collect_source_sets_readonly(db, &keys, now_ms) { + Ok(v) => v, + Err(e) => return e, + }; + if source_data.iter().any(|s| s.is_empty()) { + return Frame::Integer(0); + } + let mut indices: Vec = (0..source_data.len()).collect(); + indices.sort_by_key(|&i| source_data[i].len()); + let smallest_idx = indices[0]; + let mut count: i64 = 0; + for member in source_data[smallest_idx].keys() { + let mut in_all = true; + for &idx in indices.iter().skip(1) { + if !source_data[idx].contains_key(member) { + in_all = false; + break; + } + } + if in_all { + count += 1; + if limit > 0 && count >= limit as i64 { + break; + } + } + } + Frame::Integer(count) +} + +/// ZRANDMEMBER key [count [WITHSCORES]] — read-only twin. +pub fn zrandmember_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + use rand::seq::IndexedRandom; + if args.is_empty() || args.len() > 3 { + return err_wrong_args("ZRANDMEMBER"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("ZRANDMEMBER"), + }; + // Ref accessor: handles every encoding (BPTree, Listpack from RDB load, + // Legacy) — the BPTree-only accessor would treat a listpack zset as missing. + let zref = match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(z)) => z, + Ok(None) => { + return if args.len() == 1 { + Frame::Null + } else { + Frame::Array(framevec![]) + }; + } + Err(e) => return e, + }; + // Borrow the map when one exists; materialize only for small listpacks. + let entries_owned: Vec<(Bytes, f64)>; + let entries: Vec<(&Bytes, f64)> = match zref.members_map() { + Some(m) => m.iter().map(|(m, s)| (m, *s)).collect(), + None => { + entries_owned = zref.entries_sorted(); + entries_owned.iter().map(|(m, s)| (m, *s)).collect() + } + }; + if entries.is_empty() { + return if args.len() == 1 { + Frame::Null + } else { + Frame::Array(framevec![]) + }; + } + let mut rng = rand::rng(); + if args.len() == 1 { + return if let Some(chosen) = entries.choose(&mut rng) { + Frame::BulkString(chosen.0.clone()) + } else { + Frame::Null + }; + } + let count_bytes = match extract_bytes(&args[1]) { + Some(b) => b, + None => return err_wrong_args("ZRANDMEMBER"), + }; + let count: i64 = match std::str::from_utf8(count_bytes) + .ok() + .and_then(|s| s.parse().ok()) + { + Some(c) => c, + None => return err("ERR value is not an integer or out of range"), + }; + let withscores = if args.len() == 3 { + let opt = match extract_bytes(&args[2]) { + Some(b) => b, + None => return err("ERR syntax error"), + }; + if opt.eq_ignore_ascii_case(b"WITHSCORES") { + true + } else { + return err("ERR syntax error"); + } + } else { + false + }; + if count == 0 { + return Frame::Array(framevec![]); + } + if count > 0 { + let n = std::cmp::min(count as usize, entries.len()); + let chosen: Vec<&(&Bytes, f64)> = entries.sample(&mut rng, n).collect(); + let cap = if withscores { n * 2 } else { n }; + let mut result = Vec::with_capacity(cap); + for (member, score) in chosen { + result.push(Frame::BulkString((*member).clone())); + if withscores { + result.push(Frame::BulkString(format_score_bytes(*score))); + } + } + Frame::Array(result.into()) + } else { + let n = std::cmp::min(count.unsigned_abs() as usize, entries.len() * 10); + let cap = if withscores { n * 2 } else { n }; + let mut result = Vec::with_capacity(cap); + for _ in 0..n { + if let Some(chosen) = entries.choose(&mut rng) { + result.push(Frame::BulkString(chosen.0.clone())); + if withscores { + result.push(Frame::BulkString(format_score_bytes(chosen.1))); + } + } + } + Frame::Array(result.into()) + } +} + +/// ZMSCORE key member [member …] — read-only twin. +pub fn zmscore_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 2 { + return err_wrong_args("ZMSCORE"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("ZMSCORE"), + }; + // Ref accessor: handles every encoding (BPTree, Listpack from RDB load, + // Legacy) — the BPTree-only accessor would treat a listpack zset as missing. + match db.get_sorted_set_ref_if_alive(key, now_ms) { + Ok(Some(zref)) => { + let mut result = Vec::with_capacity(args.len() - 1); + for arg in &args[1..] { + let member = match extract_bytes(arg) { + Some(m) => m, + None => { + result.push(Frame::Null); + continue; + } + }; + match zref.score(member) { + Some(score) => { + result.push(Frame::BulkString(format_score_bytes(score))); + } + None => result.push(Frame::Null), + } + } + Frame::Array(result.into()) + } + Ok(None) => { + let mut result = Vec::with_capacity(args.len() - 1); + for _ in &args[1..] { + result.push(Frame::Null); + } + Frame::Array(result.into()) + } + Err(e) => e, + } +} diff --git a/src/command/stream/stream_read.rs b/src/command/stream/stream_read.rs index 95f8645f..3447594f 100644 --- a/src/command/stream/stream_read.rs +++ b/src/command/stream/stream_read.rs @@ -531,3 +531,523 @@ pub fn xinfo(db: &mut Database, args: &[Frame]) -> Frame { )) } } + +// --------------------------------------------------------------------------- +// Read-only twins for the shared-lock (dispatch_read) path +// --------------------------------------------------------------------------- + +/// XLEN key — read-only twin. +pub fn xlen_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() != 1 { + return err_wrong_args("XLEN"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("XLEN"), + }; + match db.get_stream_if_alive(key, now_ms) { + Ok(Some(stream)) => Frame::Integer(stream.length as i64), + Ok(None) => Frame::Integer(0), + Err(e) => e, + } +} + +/// XRANGE key start end [COUNT count] — read-only twin. +pub fn xrange_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 3 { + return err_wrong_args("XRANGE"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("XRANGE"), + }; + let start_bytes = match extract_bytes(&args[1]) { + Some(b) => b, + None => return err_wrong_args("XRANGE"), + }; + let end_bytes = match extract_bytes(&args[2]) { + Some(b) => b, + None => return err_wrong_args("XRANGE"), + }; + + let start = match StreamId::parse(start_bytes, 0) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + }; + let end = match StreamId::parse(end_bytes, u64::MAX) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + }; + + let mut count = None; + if args.len() >= 5 { + if let Some(c) = extract_bytes(&args[3]) { + if c.eq_ignore_ascii_case(b"COUNT") { + if let Some(n) = extract_bytes(&args[4]) { + if let Ok(s) = std::str::from_utf8(n) { + if let Ok(v) = s.parse::() { + count = Some(v); + } + } + } + } + } + } + + match db.get_stream_if_alive(key, now_ms) { + Ok(Some(stream)) => { + let entries = stream.range(start, end, count); + let frames: Vec = entries + .into_iter() + .map(|(id, fields)| format_entry(id, fields)) + .collect(); + Frame::Array(frames.into()) + } + Ok(None) => Frame::Array(framevec![]), + Err(e) => e, + } +} + +/// XREVRANGE key end start [COUNT count] — read-only twin. +pub fn xrevrange_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 3 { + return err_wrong_args("XREVRANGE"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("XREVRANGE"), + }; + let end_bytes = match extract_bytes(&args[1]) { + Some(b) => b, + None => return err_wrong_args("XREVRANGE"), + }; + let start_bytes = match extract_bytes(&args[2]) { + Some(b) => b, + None => return err_wrong_args("XREVRANGE"), + }; + + let end = match StreamId::parse(end_bytes, u64::MAX) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + }; + let start = match StreamId::parse(start_bytes, 0) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + }; + + let mut count = None; + if args.len() >= 5 { + if let Some(c) = extract_bytes(&args[3]) { + if c.eq_ignore_ascii_case(b"COUNT") { + if let Some(n) = extract_bytes(&args[4]) { + if let Ok(s) = std::str::from_utf8(n) { + if let Ok(v) = s.parse::() { + count = Some(v); + } + } + } + } + } + } + + match db.get_stream_if_alive(key, now_ms) { + Ok(Some(stream)) => { + let entries = stream.range_rev(start, end, count); + let frames: Vec = entries + .into_iter() + .map(|(id, fields)| format_entry(id, fields)) + .collect(); + Frame::Array(frames.into()) + } + Ok(None) => Frame::Array(framevec![]), + Err(e) => e, + } +} + +/// XREAD [COUNT count] [BLOCK ms] STREAMS key [key …] id [id …] — read-only twin. +/// +/// BLOCK is parsed-and-ignored (same as the mutable twin) — the shared-lock +/// read path is always non-blocking. +pub fn xread_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.is_empty() { + return err_wrong_args("XREAD"); + } + + let mut idx = 0; + let mut count: Option = None; + + while idx < args.len() { + let arg = match extract_bytes(&args[idx]) { + Some(a) => a, + None => break, + }; + + if arg.eq_ignore_ascii_case(b"COUNT") { + idx += 1; + if idx >= args.len() { + return err_wrong_args("XREAD"); + } + if let Some(n) = extract_bytes(&args[idx]) { + if let Ok(s) = std::str::from_utf8(n) { + if let Ok(v) = s.parse::() { + count = Some(v); + } + } + } + idx += 1; + } else if arg.eq_ignore_ascii_case(b"BLOCK") { + idx += 1; // skip the BLOCK token + if idx >= args.len() { + return err_wrong_args("XREAD"); + } + idx += 1; // skip the milliseconds value (ignored) + } else if arg.eq_ignore_ascii_case(b"STREAMS") { + idx += 1; + break; + } else { + return Frame::Error(Bytes::from_static(b"ERR Unrecognized XREAD option")); + } + } + + let remaining = args.len() - idx; + if remaining == 0 || !remaining.is_multiple_of(2) { + return err_wrong_args("XREAD"); + } + + let num_streams = remaining / 2; + let keys_start = idx; + let ids_start = idx + num_streams; + + let mut results = Vec::new(); + let mut has_entries = false; + + for i in 0..num_streams { + let key = match extract_bytes(&args[keys_start + i]) { + Some(k) => k, + None => return err_wrong_args("XREAD"), + }; + let id_bytes = match extract_bytes(&args[ids_start + i]) { + Some(b) => b, + None => return err_wrong_args("XREAD"), + }; + + let start_id = if id_bytes.as_ref() == b"$" { + match db.get_stream_if_alive(key, now_ms) { + Ok(Some(stream)) => stream.last_id, + Ok(None) => StreamId::ZERO, + Err(e) => return e, + } + } else { + match StreamId::parse(id_bytes, 0) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + } + }; + + let next_id = if start_id.seq == u64::MAX { + StreamId { + ms: start_id.ms.saturating_add(1), + seq: 0, + } + } else { + StreamId { + ms: start_id.ms, + seq: start_id.seq.saturating_add(1), + } + }; + + match db.get_stream_if_alive(key, now_ms) { + Ok(Some(stream)) => { + let entries = stream.range(next_id, StreamId::MAX, count); + if !entries.is_empty() { + has_entries = true; + } + let entry_frames: Vec = entries + .into_iter() + .map(|(id, fields)| format_entry(id, fields)) + .collect(); + results.push(Frame::Array(framevec![ + Frame::BulkString(key.clone()), + Frame::Array(entry_frames.into()), + ])); + } + Ok(None) => { + results.push(Frame::Array(framevec![ + Frame::BulkString(key.clone()), + Frame::Array(framevec![]), + ])); + } + Err(e) => return e, + } + } + + if has_entries { + Frame::Array(results.into()) + } else { + Frame::Null + } +} + +/// XINFO STREAM|GROUPS|CONSUMERS key … — read-only twin. +/// +/// XINFO HELP / missing args → returns an arity error (executed locally). +pub fn xinfo_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 2 { + // XINFO HELP or bare XINFO — handle locally + return err_wrong_args("XINFO"); + } + let subcmd = match extract_bytes(&args[0]) { + Some(s) => s, + None => return err_wrong_args("XINFO"), + }; + let key = match extract_bytes(&args[1]) { + Some(k) => k, + None => return err_wrong_args("XINFO"), + }; + + if subcmd.eq_ignore_ascii_case(b"STREAM") { + let stream = match db.get_stream_if_alive(key, now_ms) { + Ok(Some(s)) => s, + Ok(None) => return Frame::Error(Bytes::from_static(b"ERR no such key")), + Err(e) => return e, + }; + + let first_entry = stream + .entries + .iter() + .next() + .map(|(&id, fields)| format_entry(id, fields)); + let last_entry = stream + .entries + .iter() + .next_back() + .map(|(&id, fields)| format_entry(id, fields)); + + Frame::Array(framevec![ + Frame::BulkString(Bytes::from_static(b"length")), + Frame::Integer(stream.length as i64), + Frame::BulkString(Bytes::from_static(b"radix-tree-keys")), + Frame::Integer(stream.entries.len() as i64), + Frame::BulkString(Bytes::from_static(b"radix-tree-nodes")), + Frame::Integer(stream.entries.len() as i64), + Frame::BulkString(Bytes::from_static(b"last-generated-id")), + Frame::BulkString(stream.last_id.to_bytes()), + Frame::BulkString(Bytes::from_static(b"groups")), + Frame::Integer(stream.groups.len() as i64), + Frame::BulkString(Bytes::from_static(b"first-entry")), + first_entry.unwrap_or(Frame::Null), + Frame::BulkString(Bytes::from_static(b"last-entry")), + last_entry.unwrap_or(Frame::Null), + ]) + } else if subcmd.eq_ignore_ascii_case(b"GROUPS") { + let stream = match db.get_stream_if_alive(key, now_ms) { + Ok(Some(s)) => s, + Ok(None) => return Frame::Error(Bytes::from_static(b"ERR no such key")), + Err(e) => return e, + }; + + let groups: Vec = stream + .groups + .iter() + .map(|(name, group)| { + Frame::Array(framevec![ + Frame::BulkString(Bytes::from_static(b"name")), + Frame::BulkString(name.clone()), + Frame::BulkString(Bytes::from_static(b"consumers")), + Frame::Integer(group.consumers.len() as i64), + Frame::BulkString(Bytes::from_static(b"pending")), + Frame::Integer(group.pel.len() as i64), + Frame::BulkString(Bytes::from_static(b"last-delivered-id")), + Frame::BulkString(group.last_delivered_id.to_bytes()), + ]) + }) + .collect(); + Frame::Array(groups.into()) + } else if subcmd.eq_ignore_ascii_case(b"CONSUMERS") { + if args.len() < 3 { + return err_wrong_args("XINFO CONSUMERS"); + } + let group_name = match extract_bytes(&args[2]) { + Some(g) => g, + None => return err_wrong_args("XINFO CONSUMERS"), + }; + let stream = match db.get_stream_if_alive(key, now_ms) { + Ok(Some(s)) => s, + Ok(None) => return Frame::Error(Bytes::from_static(b"ERR no such key")), + Err(e) => return e, + }; + let group = match stream.groups.get(group_name) { + Some(g) => g, + None => { + return Frame::Error(Bytes::from_static( + b"NOGROUP No such consumer group for key name", + )); + } + }; + + let now = crate::storage::entry::current_time_ms(); + let consumers: Vec = group + .consumers + .iter() + .map(|(name, c)| { + let idle = now.saturating_sub(c.seen_time); + Frame::Array(framevec![ + Frame::BulkString(Bytes::from_static(b"name")), + Frame::BulkString(name.clone()), + Frame::BulkString(Bytes::from_static(b"pending")), + Frame::Integer(c.pending.len() as i64), + Frame::BulkString(Bytes::from_static(b"idle")), + Frame::Integer(idle as i64), + ]) + }) + .collect(); + Frame::Array(consumers.into()) + } else { + Frame::Error(Bytes::from_static( + b"ERR 'XINFO' command 'UNKNOWN' not recognized", + )) + } +} + +/// XPENDING key group [start end count [consumer]] — read-only twin. +pub fn xpending_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 2 { + return err_wrong_args("XPENDING"); + } + let key = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("XPENDING"), + }; + let group = match extract_bytes(&args[1]) { + Some(g) => g.clone(), + None => return err_wrong_args("XPENDING"), + }; + + let stream = match db.get_stream_if_alive(key, now_ms) { + Ok(Some(s)) => s, + Ok(None) => { + return Frame::Error(Bytes::from_static(b"ERR no such key")); + } + Err(e) => return e, + }; + + if args.len() == 2 { + // Summary form + match stream.xpending_summary(&group) { + Ok(summary) => { + if summary.is_empty() { + Frame::Array(framevec![ + Frame::Integer(0), + Frame::Null, + Frame::Null, + Frame::Null, + ]) + } else { + let (_, min_id, max_id, consumers) = &summary[0]; + let total: u64 = consumers.iter().map(|(_, c)| c).sum(); + let consumer_frames: Vec = consumers + .iter() + .map(|(name, count)| { + Frame::Array(framevec![ + Frame::BulkString(name.clone()), + Frame::BulkString(Bytes::from(count.to_string())), + ]) + }) + .collect(); + Frame::Array(framevec![ + Frame::Integer(total as i64), + Frame::BulkString(min_id.to_bytes()), + Frame::BulkString(max_id.to_bytes()), + Frame::Array(consumer_frames.into()), + ]) + } + } + Err(e) => Frame::Error(Bytes::from(e)), + } + } else if args.len() >= 5 { + // Detail form + let mut detail_idx = 2; + let mut _min_idle: u64 = 0; + if let Some(arg) = extract_bytes(&args[detail_idx]) { + if arg.eq_ignore_ascii_case(b"IDLE") { + detail_idx += 1; + if detail_idx >= args.len() { + return err_wrong_args("XPENDING"); + } + if let Some(idle_bytes) = extract_bytes(&args[detail_idx]) { + if let Ok(s) = std::str::from_utf8(idle_bytes) { + _min_idle = s.parse::().unwrap_or(0); + } + } + detail_idx += 1; + } + } + + if detail_idx + 2 >= args.len() { + return err_wrong_args("XPENDING"); + } + + let start_bytes = match extract_bytes(&args[detail_idx]) { + Some(b) => b, + None => return err_wrong_args("XPENDING"), + }; + let end_bytes = match extract_bytes(&args[detail_idx + 1]) { + Some(b) => b, + None => return err_wrong_args("XPENDING"), + }; + let count_bytes = match extract_bytes(&args[detail_idx + 2]) { + Some(b) => b, + None => return err_wrong_args("XPENDING"), + }; + + let start = match StreamId::parse(start_bytes, 0) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + }; + let end = match StreamId::parse(end_bytes, u64::MAX) { + Ok(id) => id, + Err(e) => return Frame::Error(Bytes::from(e)), + }; + let count = match std::str::from_utf8(count_bytes) { + Ok(s) => match s.parse::() { + Ok(c) => c, + Err(_) => { + return Frame::Error(Bytes::from_static( + b"ERR value is not an integer or out of range", + )); + } + }, + Err(_) => { + return Frame::Error(Bytes::from_static( + b"ERR value is not an integer or out of range", + )); + } + }; + + let consumer_filter = if detail_idx + 3 < args.len() { + extract_bytes(&args[detail_idx + 3]) + } else { + None + }; + + match stream.xpending_detail(&group, start, end, count, consumer_filter) { + Ok(details) => { + let frames: Vec = details + .into_iter() + .map(|(id, consumer, idle, delivery_count)| { + Frame::Array(framevec![ + Frame::BulkString(id.to_bytes()), + Frame::BulkString(consumer), + Frame::Integer(idle as i64), + Frame::Integer(delivery_count as i64), + ]) + }) + .collect(); + Frame::Array(frames.into()) + } + Err(e) => Frame::Error(Bytes::from(e)), + } + } else { + err_wrong_args("XPENDING") + } +} diff --git a/src/command/string/string_bit.rs b/src/command/string/string_bit.rs index ed604896..21072347 100644 --- a/src/command/string/string_bit.rs +++ b/src/command/string/string_bit.rs @@ -338,6 +338,70 @@ pub fn bitcount_readonly(db: &Database, args: &[Frame], now_ms: u64) -> Frame { } } +/// Pure BITOP combine: given the operation name and gathered source values +/// (missing keys = empty Vec), return the result string, or `None` when all +/// sources are empty/missing (caller deletes dest and replies 0). +/// +/// `Err(Frame)` carries the Redis-exact operation/arity errors. Shared by the +/// local `bitop` path and the cross-shard coordinator so semantics cannot +/// drift between them. +pub(crate) fn bitop_compute(op: &[u8], sources: &[Vec]) -> Result>, Frame> { + let is_not = op.eq_ignore_ascii_case(b"NOT"); + if is_not && sources.len() != 1 { + return Err(Frame::Error(Bytes::from_static( + b"ERR BITOP NOT requires one and only one key", + ))); + } + if !is_not + && !op.eq_ignore_ascii_case(b"AND") + && !op.eq_ignore_ascii_case(b"OR") + && !op.eq_ignore_ascii_case(b"XOR") + { + return Err(Frame::Error(Bytes::from_static( + b"ERR BITOP requires AND, OR, XOR, or NOT", + ))); + } + + let max_len = sources.iter().map(Vec::len).max().unwrap_or(0); + if max_len == 0 { + return Ok(None); + } + + let mut result = vec![0u8; max_len]; + if is_not { + let src = &sources[0]; + for (i, byte) in result.iter_mut().enumerate() { + *byte = if i < src.len() { !src[i] } else { 0xFF }; + } + } else if op.eq_ignore_ascii_case(b"AND") { + // Start with all 1s + result.iter_mut().for_each(|b| *b = 0xFF); + for src in sources { + for (i, byte) in result.iter_mut().enumerate() { + let v = if i < src.len() { src[i] } else { 0 }; + *byte &= v; + } + } + } else if op.eq_ignore_ascii_case(b"OR") { + for src in sources { + for (i, byte) in result.iter_mut().enumerate() { + if i < src.len() { + *byte |= src[i]; + } + } + } + } else { + for src in sources { + for (i, byte) in result.iter_mut().enumerate() { + if i < src.len() { + *byte ^= src[i]; + } + } + } + } + Ok(Some(result)) +} + /// BITOP operation destkey key [key ...] /// /// Perform bitwise operations between strings. @@ -354,9 +418,9 @@ pub fn bitop(db: &mut Database, args: &[Frame]) -> Frame { None => return err_wrong_args("BITOP"), }; - // Determine operation - let is_not = op.eq_ignore_ascii_case(b"NOT"); - if is_not && args.len() != 3 { + // NOT arity is validated BEFORE touching any key (Redis order: a + // wrong-arity NOT errors even when a source key holds the wrong type). + if op.eq_ignore_ascii_case(b"NOT") && args.len() != 3 { return Frame::Error(Bytes::from_static( b"ERR BITOP NOT requires one and only one key", )); @@ -364,7 +428,6 @@ pub fn bitop(db: &mut Database, args: &[Frame]) -> Frame { // Gather source values let mut sources: Vec> = Vec::with_capacity(args.len() - 2); - let mut max_len = 0usize; for arg in &args[2..] { let key = match extract_bytes(arg) { Some(k) => k, @@ -381,61 +444,23 @@ pub fn bitop(db: &mut Database, args: &[Frame]) -> Frame { }, None => Vec::new(), }; - if data.len() > max_len { - max_len = data.len(); - } sources.push(data); } - if max_len == 0 { - // All keys empty/missing — delete dest, return 0 - db.remove(&destkey); - return Frame::Integer(0); - } - - let mut result = vec![0u8; max_len]; - - if is_not { - let src = &sources[0]; - for (i, byte) in result.iter_mut().enumerate() { - *byte = if i < src.len() { !src[i] } else { 0xFF }; - } - } else if op.eq_ignore_ascii_case(b"AND") { - // Start with all 1s - result.iter_mut().for_each(|b| *b = 0xFF); - for src in &sources { - for (i, byte) in result.iter_mut().enumerate() { - let v = if i < src.len() { src[i] } else { 0 }; - *byte &= v; - } - } - } else if op.eq_ignore_ascii_case(b"OR") { - for src in &sources { - for (i, byte) in result.iter_mut().enumerate() { - if i < src.len() { - *byte |= src[i]; - } - } + match bitop_compute(op, &sources) { + Err(e) => e, + Ok(None) => { + // All keys empty/missing — delete dest, return 0 + db.remove(&destkey); + Frame::Integer(0) } - } else if op.eq_ignore_ascii_case(b"XOR") { - for src in &sources { - for (i, byte) in result.iter_mut().enumerate() { - if i < src.len() { - *byte ^= src[i]; - } - } + Ok(Some(result)) => { + let result_len = result.len() as i64; + let entry = Entry::new_string(Bytes::from(result)); + db.set(destkey, entry); + Frame::Integer(result_len) } - } else { - return Frame::Error(Bytes::from_static( - b"ERR BITOP requires AND, OR, XOR, or NOT", - )); } - - let result_len = result.len() as i64; - let entry = Entry::new_string(Bytes::from(result)); - db.set(destkey, entry); - - Frame::Integer(result_len) } /// BITPOS key bit [start [end [BYTE|BIT]]] diff --git a/src/command/string/string_read.rs b/src/command/string/string_read.rs index c69c38fd..4cc4780a 100644 --- a/src/command/string/string_read.rs +++ b/src/command/string/string_read.rs @@ -483,3 +483,97 @@ pub fn lcs(db: &mut Database, args: &[Frame]) -> Frame { lcs_bytes.reverse(); Frame::BulkString(Bytes::from(lcs_bytes)) } + +/// LCS key1 key2 [LEN] — read-only twin for the shared-lock path. +/// +/// Uses `get_if_alive` so expired keys are invisible and NOT deleted. +pub fn lcs_readonly(db: &crate::storage::db::Database, args: &[Frame], now_ms: u64) -> Frame { + if args.len() < 2 { + return err_wrong_args("LCS"); + } + let key1 = match extract_bytes(&args[0]) { + Some(k) => k, + None => return err_wrong_args("LCS"), + }; + let key2 = match extract_bytes(&args[1]) { + Some(k) => k, + None => return err_wrong_args("LCS"), + }; + + let mut want_len = false; + let mut i = 2; + while i < args.len() { + let arg = match extract_bytes(&args[i]) { + Some(a) => a, + None => return Frame::Error(Bytes::from_static(b"ERR syntax error")), + }; + if arg.eq_ignore_ascii_case(b"LEN") { + want_len = true; + } + i += 1; + } + + let s1 = match db.get_if_alive(key1, now_ms) { + Some(e) => match e.value.as_bytes() { + Some(v) => v.to_vec(), + None => { + return Frame::Error(Bytes::from_static( + b"WRONGTYPE Operation against a key holding the wrong kind of value", + )); + } + }, + None => Vec::new(), + }; + let s2 = match db.get_if_alive(key2, now_ms) { + Some(e) => match e.value.as_bytes() { + Some(v) => v.to_vec(), + None => { + return Frame::Error(Bytes::from_static( + b"WRONGTYPE Operation against a key holding the wrong kind of value", + )); + } + }, + None => Vec::new(), + }; + + let n = s1.len(); + let m = s2.len(); + const MAX_LCS_CELLS: usize = 16 * 1024 * 1024; + if n.saturating_mul(m) > MAX_LCS_CELLS { + return Frame::Error(Bytes::from_static( + b"ERR inputs too large for LCS computation", + )); + } + let mut dp = vec![vec![0u32; m + 1]; n + 1]; + for ii in 1..=n { + for jj in 1..=m { + if s1[ii - 1] == s2[jj - 1] { + dp[ii][jj] = dp[ii - 1][jj - 1] + 1; + } else { + dp[ii][jj] = dp[ii - 1][jj].max(dp[ii][jj - 1]); + } + } + } + let lcs_len = dp[n][m] as usize; + + if want_len { + return Frame::Integer(lcs_len as i64); + } + + let mut lcs_bytes = Vec::with_capacity(lcs_len); + let mut ci = n; + let mut cj = m; + while ci > 0 && cj > 0 { + if s1[ci - 1] == s2[cj - 1] { + lcs_bytes.push(s1[ci - 1]); + ci -= 1; + cj -= 1; + } else if dp[ci - 1][cj] > dp[ci][cj - 1] { + ci -= 1; + } else { + cj -= 1; + } + } + lcs_bytes.reverse(); + Frame::BulkString(Bytes::from(lcs_bytes)) +} diff --git a/src/command/temporal.rs b/src/command/temporal.rs index a4b5ed33..3747297c 100644 --- a/src/command/temporal.rs +++ b/src/command/temporal.rs @@ -83,6 +83,51 @@ pub fn validate_invalidate(args: &[Frame]) -> Result<(u64, bool, Bytes), Frame> Ok((entity_id, is_node, graph_name)) } +/// Apply a TEMPORAL.INVALIDATE mutation to a graph store. +/// +/// Sets `valid_to = wall_ms` on the entity and pushes the WAL payload into +/// `gs.wal_pending`. The CALLER drains the WAL and appends on its own shard — +/// this keeps the function usable from both the connection-local path and the +/// shard-side `ShardMessage::GraphCommand` handler (multi-shard routing sends +/// the command to the shard that owns the graph name). +#[cfg(feature = "graph")] +pub fn apply_invalidate( + gs: &mut crate::graph::store::GraphStore, + entity_id: u64, + is_node: bool, + graph_name: &Bytes, + wall_ms: i64, +) -> Result<(), &'static [u8]> { + let Some(named_graph) = gs.get_graph_mut(graph_name) else { + return Err(ERR_GRAPH_NOT_FOUND); + }; + let mutated = if is_node { + let node_key: crate::graph::types::NodeKey = slotmap::KeyData::from_ffi(entity_id).into(); + if let Some(node) = named_graph.write_buf.get_node_mut(node_key) { + node.valid_to = wall_ms; + true + } else { + false + } + } else { + let edge_key: crate::graph::types::EdgeKey = slotmap::KeyData::from_ffi(entity_id).into(); + if let Some(edge) = named_graph.write_buf.get_edge_mut(edge_key) { + edge.valid_to = wall_ms; + true + } else { + false + } + }; + if !mutated { + return Err(ERR_ENTITY_NOT_FOUND); + } + let payload = crate::persistence::wal_v3::record::encode_graph_temporal( + entity_id, is_node, wall_ms, wall_ms, + ); + gs.wal_pending.push(payload); + Ok(()) +} + /// Capture the current wall-clock time as i64 Unix milliseconds. /// /// CRITICAL: This function MUST be called at the handler level BEFORE diff --git a/src/server/conn/handler_monoio/dispatch.rs b/src/server/conn/handler_monoio/dispatch.rs index bce59007..69db1a8f 100644 --- a/src/server/conn/handler_monoio/dispatch.rs +++ b/src/server/conn/handler_monoio/dispatch.rs @@ -325,7 +325,14 @@ pub(super) fn try_handle_cluster_routing( if is_multi_key_command(cmd, cmd_args) { let first_slot = slot; let mut cross_slot = false; - for arg in cmd_args.iter().skip(1) { + // COPY's keys are exactly args[0..2]; trailing args are the + // REPLACE literal, which must not be slot-checked. + let key_args: &[Frame] = if cmd.eq_ignore_ascii_case(b"COPY") { + &cmd_args[..cmd_args.len().min(2)] + } else { + cmd_args + }; + for arg in key_args.iter().skip(1) { if let Some(k) = match arg { Frame::BulkString(b) => Some(b.as_ref()), _ => None, @@ -545,6 +552,22 @@ pub(super) fn try_handle_replconf( true } +/// CDC.READ — polling-based change data capture (C3 v1). +/// +/// Stateless / synchronous — reads WAL files from disk, no shard state +/// involved. Mirrors the identical function in handler_sharded/dispatch.rs. +pub(super) fn try_handle_cdc_read( + cmd: &[u8], + cmd_args: &[Frame], + responses: &mut Vec, +) -> bool { + if !cmd.eq_ignore_ascii_case(b"CDC.READ") { + return false; + } + responses.push(crate::command::cdc::cdc_read(cmd_args)); + true +} + /// Handle PSYNC command. Returns `Some((repl_id, offset))` when this PSYNC /// arrival should hijack the connection. The caller breaks out of the dispatch /// loop and returns the stream so the master replication driver can take over. diff --git a/src/server/conn/handler_monoio/mod.rs b/src/server/conn/handler_monoio/mod.rs index 2004c4a0..3c4c2709 100644 --- a/src/server/conn/handler_monoio/mod.rs +++ b/src/server/conn/handler_monoio/mod.rs @@ -750,6 +750,10 @@ pub(crate) async fn handle_connection_sharded_monoio< if cmd_len == 8 && dispatch::try_handle_replconf(cmd, cmd_args, ctx, &mut responses) { continue; } + // CDC.READ (8) — stateless WAL reader, no shard state involved. + if cmd_len == 8 && dispatch::try_handle_cdc_read(cmd, cmd_args, &mut responses) { + continue; + } // PSYNC: arrives only on a master, hijacks the connection. Encode // any pending responses, flush, then return the stream so the // caller can drive the resync handshake. @@ -878,7 +882,7 @@ pub(crate) async fn handle_connection_sharded_monoio< if txn::try_handle_txn_commit(cmd, cmd_args, &mut conn, ctx, &mut responses) { continue; } - if txn::try_handle_txn_abort(cmd, cmd_args, &mut conn, ctx, &mut responses) { + if txn::try_handle_txn_abort(cmd, cmd_args, &mut conn, ctx, &mut responses).await { continue; } @@ -886,7 +890,8 @@ pub(crate) async fn handle_connection_sharded_monoio< if txn::try_handle_temporal_snapshot_at(cmd, cmd_args, ctx, &mut responses) { continue; } - if txn::try_handle_temporal_invalidate(cmd, cmd_args, ctx, &mut responses) { + if txn::try_handle_temporal_invalidate(cmd, cmd_args, &frame, ctx, &mut responses).await + { continue; } @@ -956,7 +961,16 @@ pub(crate) async fn handle_connection_sharded_monoio< // --- GRAPH.* graph commands --- #[cfg(feature = "graph")] - if write::try_handle_graph_command(cmd, cmd_args, &mut conn, ctx, &mut responses) { + if write::try_handle_graph_command( + cmd, + cmd_args, + &frame, + &mut conn, + ctx, + &mut responses, + ) + .await + { continue; } @@ -2211,12 +2225,16 @@ pub(crate) async fn handle_connection_sharded_monoio< // sharded runtime block in handler_sharded.rs so both paths delegate to the same // shared helper. FIN has already been sent; shard state is still intact. if let Some(txn) = conn.active_cross_txn.take() { - crate::transaction::abort::abort_cross_store_txn( + crate::transaction::abort::abort_cross_store_txn_routed( &ctx.shard_databases, ctx.shard_id, conn.selected_db, + ctx.num_shards, + &ctx.dispatch_tx, + &ctx.spsc_notifiers, txn, - ); + ) + .await; } // --- Disconnect cleanup: propagate unsubscribe to all shards' remote subscriber maps --- diff --git a/src/server/conn/handler_monoio/txn.rs b/src/server/conn/handler_monoio/txn.rs index 5eb11200..e91a6ef9 100644 --- a/src/server/conn/handler_monoio/txn.rs +++ b/src/server/conn/handler_monoio/txn.rs @@ -4,8 +4,6 @@ use bytes::Bytes; -#[cfg(feature = "graph")] -use crate::command::temporal::{ERR_ENTITY_NOT_FOUND, ERR_GRAPH_NOT_FOUND}; use crate::command::temporal::{ capture_wall_ms, is_temporal_invalidate, is_temporal_snapshot_at, validate_invalidate, validate_snapshot_at, @@ -142,9 +140,16 @@ pub(super) fn try_handle_txn_commit( } }); } else { - let mut db_guard = - ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + // Each queue lives on the shard owning its key, which + // may differ from the connection's shard (and per + // intent) — acquire the owner's db per queue. for intent in &txn.mq_intents { + let owner = crate::shard::dispatch::key_to_shard( + &intent.queue_key, + ctx.num_shards, + ); + let mut db_guard = + ctx.shard_databases.write_db(owner, conn.selected_db); if let Ok(Some(stream)) = db_guard.get_stream_mut(&intent.queue_key) { if stream.durable { let msg_id = stream.next_auto_id(); @@ -165,8 +170,8 @@ pub(super) fn try_handle_txn_commit( true } -/// Handle TXN.ABORT ��� returns `true` if the command was consumed. -pub(super) fn try_handle_txn_abort( +/// Handle TXN.ABORT — returns `true` if the command was consumed. +pub(super) async fn try_handle_txn_abort( cmd: &[u8], cmd_args: &[Frame], conn: &mut ConnectionState, @@ -183,12 +188,18 @@ pub(super) fn try_handle_txn_abort( // KV undo -> graph intents reverse -> vector // tombstone -> side-table release. See // src/transaction/abort.rs for lock ordering. - crate::transaction::abort::abort_cross_store_txn( + // Multi-shard: graph legs route to the shards owning + // each graph name via ShardMessage::GraphRollback. + crate::transaction::abort::abort_cross_store_txn_routed( &ctx.shard_databases, ctx.shard_id, conn.selected_db, + ctx.num_shards, + &ctx.dispatch_tx, + &ctx.spsc_notifiers, txn, - ); + ) + .await; responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); } else { responses.push(Frame::Error(Bytes::from_static(b"ERR not in transaction"))); @@ -230,9 +241,10 @@ pub(super) fn try_handle_temporal_snapshot_at( } /// Handle TEMPORAL.INVALIDATE — returns `true` if the command was consumed. -pub(super) fn try_handle_temporal_invalidate( +pub(super) async fn try_handle_temporal_invalidate( cmd: &[u8], cmd_args: &[Frame], + frame: &Frame, ctx: &ConnectionContext, responses: &mut Vec, ) -> bool { @@ -241,100 +253,89 @@ pub(super) fn try_handle_temporal_invalidate( } match validate_invalidate(cmd_args) { Ok((entity_id, is_node, graph_name)) => { - let wall_ms = capture_wall_ms(); #[cfg(feature = "graph")] { - // Phase 2a: gate on is_initialized(); new path is dead code until Phase 4 - // wires init_shard() at shard startup. Both branches are semantically - // identical; the new path uses ShardSlice::graph_store directly (no lock). - let (mutated_ok, wal_records, err_frame) = if crate::shard::slice::is_initialized() - { + // Multi-shard: the graph lives on the shard that owns its + // NAME. Ship non-local invalidations there via GraphCommand — + // the shard-side handler applies the mutation and drains the + // graph WAL on the owning shard. + if ctx.num_shards > 1 { + let owner = crate::shard::dispatch::graph_to_shard(&graph_name, ctx.num_shards); + if owner != ctx.shard_id { + let (reply_tx, reply_rx) = crate::runtime::channel::oneshot(); + let msg = crate::shard::dispatch::ShardMessage::GraphCommand { + command: std::sync::Arc::new(frame.clone()), + reply_tx, + }; + crate::shard::coordinator::spsc_send( + &ctx.dispatch_tx, + ctx.shard_id, + owner, + msg, + &ctx.spsc_notifiers, + ) + .await; + let response = match reply_rx.recv().await { + Ok(f) => f, + Err(_) => Frame::Error(Bytes::from_static( + b"ERR cross-shard reply channel closed", + )), + }; + responses.push(response); + return true; + } + } + let wall_ms = capture_wall_ms(); + // Phase 2a: gate on is_initialized(); both branches are + // semantically identical, the new path uses + // ShardSlice::graph_store directly (no lock). + let (result, wal_records) = if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard(|s| { let gs = &mut s.graph_store; - if let Some(named_graph) = gs.get_graph_mut(&graph_name) { - let mutated = if is_node { - let node_key: crate::graph::types::NodeKey = - slotmap::KeyData::from_ffi(entity_id).into(); - if let Some(node) = named_graph.write_buf.get_node_mut(node_key) { - node.valid_to = wall_ms; - true - } else { - false - } - } else { - let edge_key: crate::graph::types::EdgeKey = - slotmap::KeyData::from_ffi(entity_id).into(); - if let Some(edge) = named_graph.write_buf.get_edge_mut(edge_key) { - edge.valid_to = wall_ms; - true - } else { - false - } - }; - if mutated { - let payload = - crate::persistence::wal_v3::record::encode_graph_temporal( - entity_id, is_node, wall_ms, wall_ms, - ); - gs.wal_pending.push(payload); - let recs = gs.drain_wal(); - (true, recs, None) - } else { - (false, Vec::new(), Some(ERR_ENTITY_NOT_FOUND)) - } + let r = crate::command::temporal::apply_invalidate( + gs, + entity_id, + is_node, + &graph_name, + wall_ms, + ); + let recs = if r.is_ok() { + gs.drain_wal() } else { - (false, Vec::new(), Some(ERR_GRAPH_NOT_FOUND)) - } + Vec::new() + }; + (r, recs) }) } else { let mut gs = ctx.shard_databases.graph_store_write(ctx.shard_id); - if let Some(named_graph) = gs.get_graph_mut(&graph_name) { - let mutated = if is_node { - let node_key: crate::graph::types::NodeKey = - slotmap::KeyData::from_ffi(entity_id).into(); - if let Some(node) = named_graph.write_buf.get_node_mut(node_key) { - node.valid_to = wall_ms; - true - } else { - false - } - } else { - let edge_key: crate::graph::types::EdgeKey = - slotmap::KeyData::from_ffi(entity_id).into(); - if let Some(edge) = named_graph.write_buf.get_edge_mut(edge_key) { - edge.valid_to = wall_ms; - true - } else { - false - } - }; - if mutated { - let payload = crate::persistence::wal_v3::record::encode_graph_temporal( - entity_id, is_node, wall_ms, wall_ms, - ); - gs.wal_pending.push(payload); - let recs = gs.drain_wal(); - (true, recs, None) - } else { - (false, Vec::new(), Some(ERR_ENTITY_NOT_FOUND)) - } + let r = crate::command::temporal::apply_invalidate( + &mut gs, + entity_id, + is_node, + &graph_name, + wall_ms, + ); + let recs = if r.is_ok() { + gs.drain_wal() } else { - (false, Vec::new(), Some(ERR_GRAPH_NOT_FOUND)) - } + Vec::new() + }; + (r, recs) }; - if mutated_ok { - for record in wal_records { - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(record)); + match result { + Ok(()) => { + for record in wal_records { + ctx.shard_databases + .wal_append(ctx.shard_id, Bytes::from(record)); + } + responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); } - responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); - } else if let Some(err) = err_frame { - responses.push(Frame::Error(Bytes::from_static(err))); + Err(err) => responses.push(Frame::Error(Bytes::from_static(err))), } } #[cfg(not(feature = "graph"))] { - let _ = (entity_id, is_node, graph_name, wall_ms, ctx); + let _ = (entity_id, is_node, graph_name, frame, ctx); responses.push(Frame::Error(Bytes::from_static( b"ERR graph feature not enabled", ))); diff --git a/src/server/conn/handler_monoio/write.rs b/src/server/conn/handler_monoio/write.rs index e472052f..1e02462e 100644 --- a/src/server/conn/handler_monoio/write.rs +++ b/src/server/conn/handler_monoio/write.rs @@ -60,13 +60,16 @@ pub(super) fn try_handle_ws_command( created_at, }; { - let mut guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.workspace_registry(); let reg = guard.get_or_insert_with(|| { Box::new(crate::workspace::WorkspaceRegistry::new()) }); reg.insert(ws_id, meta); } - // WAL: WorkspaceCreate record + // WAL: WorkspaceCreate record. The registry is global, so its + // WAL stream is pinned to shard 0 — one stream gives replay a + // total order over Create/Drop regardless of which connection + // issued them. let payload = crate::workspace::wal::encode_workspace_create(ws_id.as_bytes(), &ws_name); let mut wal_buf = Vec::new(); @@ -76,8 +79,7 @@ pub(super) fn try_handle_ws_command( crate::persistence::wal_v3::record::WalRecordType::WorkspaceCreate, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(0, Bytes::from(wal_buf)); responses.push(Frame::BulkString(Bytes::from(ws_id.to_string()))); } Err(e) => responses.push(e), @@ -91,7 +93,7 @@ pub(super) fn try_handle_ws_command( match parse_workspace_id_from_bytes(&ws_id_raw) { Some(ws_id) => { let removed = { - let mut guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.workspace_registry(); match guard.as_mut() { Some(reg) => reg.remove(&ws_id).is_some(), None => false, @@ -108,8 +110,7 @@ pub(super) fn try_handle_ws_command( crate::persistence::wal_v3::record::WalRecordType::WorkspaceDrop, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(0, Bytes::from(wal_buf)); // Best-effort cleanup: delete all KV keys with ws prefix (WS-03). // Phase 2a: gate on is_initialized(); new path uses ShardSlice. { @@ -126,8 +127,14 @@ pub(super) fn try_handle_ws_command( } }); } else { - let mut db_guard = - ctx.shard_databases.write_db(ctx.shard_id, 0); + // The {wsid} hash tag co-locates every + // workspace key on ONE shard — clean up + // there, not on the connection's shard. + let owner = crate::shard::dispatch::key_to_shard( + prefix.as_bytes(), + ctx.num_shards, + ); + let mut db_guard = ctx.shard_databases.write_db(owner, 0); let keys_to_delete: Vec> = db_guard .keys() .filter(|k| k.as_bytes().starts_with(prefix.as_bytes())) @@ -156,7 +163,7 @@ pub(super) fn try_handle_ws_command( if sub.eq_ignore_ascii_case(b"LIST") { match validate_ws_list(cmd_args) { Ok(()) => { - let guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let guard = ctx.shard_databases.workspace_registry(); let entries: Vec = match guard.as_ref() { Some(reg) => reg .iter() @@ -184,7 +191,7 @@ pub(super) fn try_handle_ws_command( match validate_ws_info(cmd_args) { Ok(ws_id_raw) => match parse_workspace_id_from_bytes(&ws_id_raw) { Some(ws_id) => { - let guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let guard = ctx.shard_databases.workspace_registry(); let found = guard.as_ref().and_then(|reg| reg.get(&ws_id)); match found { Some(meta) => { @@ -221,7 +228,7 @@ pub(super) fn try_handle_ws_command( match parse_workspace_id_from_bytes(&ws_id_raw) { Some(ws_id) => { let found = { - let guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let guard = ctx.shard_databases.workspace_registry(); guard .as_ref() .map_or(false, |reg| reg.get(&ws_id).is_some()) @@ -273,6 +280,15 @@ pub(super) fn try_handle_mq_command( Ok((queue_key, max_delivery_count, _debounce_ms)) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // A durable queue lives on the shard that owns its key — the + // stream, registry entry, trigger entry, and WAL records must + // all target `owner`, not the connection's shard, or fresh + // connections landing elsewhere (SO_REUSEPORT) can't see the + // queue. Lock-mode ShardDatabases permits direct cross-shard + // access; the ShardSlice branches stay conn-local because + // slice mode is never initialized yet (owner-routing there is + // the shardslice-migration task). + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); // Phase 2a: gate on is_initialized(); new path uses ShardSlice directly. let create_result: Result<(), Frame> = if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard_db(conn.selected_db, |db| { @@ -288,7 +304,7 @@ pub(super) fn try_handle_mq_command( } }) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); match db_guard.get_or_create_stream(&effective_key) { Ok(stream) => { stream.durable = true; @@ -305,17 +321,18 @@ pub(super) fn try_handle_mq_command( return true; } - // Store config in per-shard registry + // Store config in the owning shard's registry let config = crate::mq::DurableStreamConfig::new(effective_key.clone(), max_delivery_count); { - let mut guard = ctx.shard_databases.durable_queue_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.durable_queue_registry(owner); let reg = guard .get_or_insert_with(|| Box::new(crate::mq::DurableQueueRegistry::new())); reg.insert(effective_key.clone(), config); } - // WAL: MqCreate record + // WAL: MqCreate record — owner's WAL so replay restores the + // registry on the shard that also holds the stream. let payload = crate::mq::wal::encode_mq_create(&effective_key, max_delivery_count); let mut wal_buf = Vec::new(); crate::persistence::wal_v3::record::write_wal_v3_record( @@ -324,8 +341,7 @@ pub(super) fn try_handle_mq_command( crate::persistence::wal_v3::record::WalRecordType::MqCreate, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(owner, Bytes::from(wal_buf)); responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); } Err(e) => responses.push(e), @@ -338,6 +354,8 @@ pub(super) fn try_handle_mq_command( Ok((queue_key, fields)) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // Owner-shard targeting — see MQ CREATE above. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); // Phase 2a: gate on is_initialized(); new path uses ShardSlice. // trigger_registry stays via old path (Phase 2b scope, not migrated here). let push_result: Result = @@ -359,8 +377,7 @@ pub(super) fn try_handle_mq_command( } }) } else { - let mut db_guard = - ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); match db_guard.get_stream_mut(&effective_key) { Ok(Some(stream)) => { if !stream.durable { @@ -376,9 +393,10 @@ pub(super) fn try_handle_mq_command( }; match push_result { Ok(msg_id) => { - // trigger_registry: not in Phase 2a scope — old path only + // trigger_registry: not in Phase 2a scope — old path only. + // Owner's registry: its event-loop tick fires triggers. { - let mut trig_guard = ctx.shard_databases.trigger_registry(ctx.shard_id); + let mut trig_guard = ctx.shard_databases.trigger_registry(owner); if let Some(reg) = trig_guard.as_mut() { let trig_key = if let Some(ws_id) = conn.workspace_id.as_ref() { let ws_hex = ws_id.as_hex(); @@ -418,6 +436,8 @@ pub(super) fn try_handle_mq_command( Ok((queue_key, count)) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // Owner-shard targeting — see MQ CREATE above. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); let group_name = Bytes::from_static(b"__mq_consumers"); let consumer_name = Bytes::from_static(b"__mq_default"); @@ -516,7 +536,7 @@ pub(super) fn try_handle_mq_command( Ok(Frame::Array(result_frames.into())) }) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); let mdc = match db_guard.get_stream_mut(&effective_key) { Ok(Some(stream)) => { if !stream.durable { @@ -631,6 +651,8 @@ pub(super) fn try_handle_mq_command( .iter() .map(|(ms, seq)| StreamId { ms: *ms, seq: *seq }) .collect(); + // Owner-shard targeting — see MQ CREATE above. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); // Phase 2a: gate on is_initialized(); new path uses ShardSlice. // WAL append stays outside closure (wal_append_tx not in Phase 2a scope). let ack_result: Result = if crate::shard::slice::is_initialized() { @@ -647,7 +669,7 @@ pub(super) fn try_handle_mq_command( } }) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); match db_guard.get_stream_mut(&effective_key) { Ok(Some(stream)) => { let group_name = Bytes::from_static(b"__mq_consumers"); @@ -671,8 +693,7 @@ pub(super) fn try_handle_mq_command( crate::persistence::wal_v3::record::WalRecordType::MqAck, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(owner, Bytes::from(wal_buf)); } } responses.push(Frame::Integer(acked_count)); @@ -687,6 +708,9 @@ pub(super) fn try_handle_mq_command( Ok(queue_key) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // Owner of the QUEUE key, not the dlq_key: POP creates the + // DLQ stream in the same db as the queue it drains. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); let dlq_key = { let mut buf = Vec::with_capacity(effective_key.len() + 8); buf.extend_from_slice(&effective_key); @@ -702,7 +726,7 @@ pub(super) fn try_handle_mq_command( } }) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); match db_guard.get_stream_mut(&dlq_key) { Ok(Some(stream)) => stream.length as i64, _ => 0i64, @@ -730,6 +754,9 @@ pub(super) fn try_handle_mq_command( } else { queue_key.clone() }; + // Owner's registry: its event-loop tick fires triggers + // (timers.rs documents the home shard as authoritative). + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); let entry = crate::mq::TriggerEntry { queue_key: effective_key, callback_cmd, @@ -738,7 +765,7 @@ pub(super) fn try_handle_mq_command( pending_fire_ms: 0, }; { - let mut guard = ctx.shard_databases.trigger_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.trigger_registry(owner); let reg = guard.get_or_insert_with(|| Box::new(crate::mq::TriggerRegistry::new())); reg.register(trig_key, entry); @@ -835,9 +862,10 @@ pub(super) fn try_handle_multi_exec( /// Handle GRAPH.* graph commands. Returns `true` if consumed. #[cfg(feature = "graph")] -pub(super) fn try_handle_graph_command( +pub(super) async fn try_handle_graph_command( cmd: &[u8], cmd_args: &[Frame], + frame: &Frame, conn: &mut ConnectionState, ctx: &ConnectionContext, responses: &mut Vec, @@ -845,6 +873,68 @@ pub(super) fn try_handle_graph_command( if cmd.len() <= 6 || !cmd[..6].eq_ignore_ascii_case(b"GRAPH.") { return false; } + // Multi-shard: a graph lives on the shard that owns its NAME (same xxh64 + // + {tag} hashing as key routing). The per-shard graph store is + // thread-local once ShardSlice is initialized, so non-owner commands MUST + // hop via ShardMessage::GraphCommand — the shard-side handler dispatches + // on its own store and drains graph WAL records locally. + // GRAPH.LIST has no name argument and stays connection-local (it reports + // this shard's graphs only — recorded as a v3 observe delta). + if ctx.num_shards > 1 && !cmd.eq_ignore_ascii_case(b"GRAPH.LIST") { + if let Some(name) = cmd_args.first().and_then(extract_bytes) { + let owner = crate::shard::dispatch::graph_to_shard(&name, ctx.num_shards); + if owner != ctx.shard_id { + // Cypher WRITE queries inside a cross-store TXN cannot ship + // their undo intents back across the hop — reject like the + // other two-domain TXN cases (MOVE, COPY ... DB). + if conn.in_cross_txn() + && cmd.eq_ignore_ascii_case(b"GRAPH.QUERY") + && crate::command::graph::is_cypher_write_query(cmd_args) + { + responses.push(Frame::Error(bytes::Bytes::from_static( + crate::command::transaction::ERR_TXN_CROSS_SHARD, + ))); + return true; + } + let (reply_tx, reply_rx) = crate::runtime::channel::oneshot(); + let msg = crate::shard::dispatch::ShardMessage::GraphCommand { + command: std::sync::Arc::new(frame.clone()), + reply_tx, + }; + crate::shard::coordinator::spsc_send( + &ctx.dispatch_tx, + ctx.shard_id, + owner, + msg, + &ctx.spsc_notifiers, + ) + .await; + let mut response = match reply_rx.recv().await { + Ok(f) => f, + Err(_) => Frame::Error(bytes::Bytes::from_static( + b"ERR cross-shard reply channel closed", + )), + }; + // Phase 166: explicit ADDNODE/ADDEDGE intents are captured + // from the routed RESPONSE id, exactly like the local path; + // the abort path routes the rollback back to the owner. + if let Some(txn) = conn.active_cross_txn.as_mut() { + let is_node = cmd.eq_ignore_ascii_case(b"GRAPH.ADDNODE"); + let is_edge = cmd.eq_ignore_ascii_case(b"GRAPH.ADDEDGE"); + if is_node || is_edge { + if let Frame::Integer(id) = &response { + txn.record_graph(*id as u64, is_node, name.clone()); + } + } + } + if let Some(ws_id) = conn.workspace_id.as_ref() { + strip_workspace_prefix_from_response(ws_id, cmd, &mut response); + } + responses.push(response); + return true; + } + } + } // Phase 2a: gate on is_initialized(); new path uses ShardSlice::graph_store directly. let (response, wal_records, cypher_intents, cypher_undo_ops) = if crate::shard::slice::is_initialized() { diff --git a/src/server/conn/handler_sharded/mod.rs b/src/server/conn/handler_sharded/mod.rs index 7bcf2241..668092cc 100644 --- a/src/server/conn/handler_sharded/mod.rs +++ b/src/server/conn/handler_sharded/mod.rs @@ -579,7 +579,14 @@ pub(crate) async fn handle_connection_sharded_inner< if is_multi_key_command(cmd, cmd_args) { let first_slot = slot; let mut cross_slot = false; - for arg in cmd_args.iter().skip(1) { + // COPY's keys are exactly args[0..2]; trailing args + // are the REPLACE literal — not slot-checked. + let key_args: &[Frame] = if cmd.eq_ignore_ascii_case(b"COPY") { + &cmd_args[..cmd_args.len().min(2)] + } else { + cmd_args + }; + for arg in key_args.iter().skip(1) { if let Some(k) = match arg { Frame::BulkString(b) => Some(b.as_ref()), _ => None, @@ -834,7 +841,9 @@ pub(crate) async fn handle_connection_sharded_inner< if txn::try_handle_txn_commit(cmd, cmd_args, &mut conn, ctx, &mut responses) { continue; } - if txn::try_handle_txn_abort(cmd, cmd_args, &mut conn, ctx, &mut responses) { + if txn::try_handle_txn_abort(cmd, cmd_args, &mut conn, ctx, &mut responses) + .await + { continue; } @@ -842,7 +851,15 @@ pub(crate) async fn handle_connection_sharded_inner< if txn::try_handle_temporal_snapshot_at(cmd, cmd_args, ctx, &mut responses) { continue; } - if txn::try_handle_temporal_invalidate(cmd, cmd_args, ctx, &mut responses) { + if txn::try_handle_temporal_invalidate( + cmd, + cmd_args, + &frame, + ctx, + &mut responses, + ) + .await + { continue; } @@ -958,7 +975,7 @@ pub(crate) async fn handle_connection_sharded_inner< // --- GRAPH.* graph commands --- #[cfg(feature = "graph")] - if write::try_handle_graph_command(cmd, cmd_args, &mut conn, ctx, &mut responses) { + if write::try_handle_graph_command(cmd, cmd_args, &frame, &mut conn, ctx, &mut responses).await { continue; } @@ -1925,12 +1942,16 @@ pub(crate) async fn handle_connection_sharded_inner< // Closes T-161-05 — without this, a disconnect after TXN.BEGIN + SET would leak // kv_intents and pin the key invisible for all subsequent readers. if let Some(txn) = conn.active_cross_txn.take() { - crate::transaction::abort::abort_cross_store_txn( + crate::transaction::abort::abort_cross_store_txn_routed( &ctx.shard_databases, ctx.shard_id, conn.selected_db, + ctx.num_shards, + &ctx.dispatch_tx, + &ctx.spsc_notifiers, txn, - ); + ) + .await; } // Clean up pub/sub subscriptions on disconnect diff --git a/src/server/conn/handler_sharded/txn.rs b/src/server/conn/handler_sharded/txn.rs index 78f11234..afd1cf4d 100644 --- a/src/server/conn/handler_sharded/txn.rs +++ b/src/server/conn/handler_sharded/txn.rs @@ -4,8 +4,6 @@ use bytes::Bytes; -#[cfg(feature = "graph")] -use crate::command::temporal::{ERR_ENTITY_NOT_FOUND, ERR_GRAPH_NOT_FOUND}; use crate::command::temporal::{ capture_wall_ms, is_temporal_invalidate, is_temporal_snapshot_at, validate_invalidate, validate_snapshot_at, @@ -172,9 +170,23 @@ pub(super) fn try_handle_txn_commit( if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard_db(conn.selected_db, materialize); } else { - let mut db_guard = - ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); - materialize(&mut *db_guard); + // Each queue lives on the shard owning its key, which + // may differ from the connection's shard (and per + // intent) — acquire the owner's db per queue. + for intent in &txn.mq_intents { + let owner = crate::shard::dispatch::key_to_shard( + &intent.queue_key, + ctx.num_shards, + ); + let mut db_guard = + ctx.shard_databases.write_db(owner, conn.selected_db); + if let Ok(Some(stream)) = db_guard.get_stream_mut(&intent.queue_key) { + if stream.durable { + let msg_id = stream.next_auto_id(); + stream.add(msg_id, intent.fields.clone()); + } + } + } } } @@ -188,8 +200,8 @@ pub(super) fn try_handle_txn_commit( true } -/// Handle TXN.ABORT ��� returns `true` if the command was consumed. -pub(super) fn try_handle_txn_abort( +/// Handle TXN.ABORT — returns `true` if the command was consumed. +pub(super) async fn try_handle_txn_abort( cmd: &[u8], cmd_args: &[Frame], conn: &mut ConnectionState, @@ -206,12 +218,18 @@ pub(super) fn try_handle_txn_abort( // KV undo -> graph intents reverse -> vector // tombstone -> side-table release. See // src/transaction/abort.rs for lock ordering. - crate::transaction::abort::abort_cross_store_txn( + // Multi-shard: graph legs route to the shards owning + // each graph name via ShardMessage::GraphRollback. + crate::transaction::abort::abort_cross_store_txn_routed( &ctx.shard_databases, ctx.shard_id, conn.selected_db, + ctx.num_shards, + &ctx.dispatch_tx, + &ctx.spsc_notifiers, txn, - ); + ) + .await; responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); } else { responses.push(Frame::Error(Bytes::from_static(b"ERR not in transaction"))); @@ -256,9 +274,10 @@ pub(super) fn try_handle_temporal_snapshot_at( } /// Handle TEMPORAL.INVALIDATE — returns `true` if the command was consumed. -pub(super) fn try_handle_temporal_invalidate( +pub(super) async fn try_handle_temporal_invalidate( cmd: &[u8], cmd_args: &[Frame], + frame: &Frame, ctx: &ConnectionContext, responses: &mut Vec, ) -> bool { @@ -267,73 +286,89 @@ pub(super) fn try_handle_temporal_invalidate( } match validate_invalidate(cmd_args) { Ok((entity_id, is_node, graph_name)) => { - let wall_ms = capture_wall_ms(); #[cfg(feature = "graph")] { - // Phase 2f: gate on is_initialized(); new path uses ShardSlice directly. - // Closure outcome: - // Some(Some(wal_records)) — entity found, mutated; emit OK and append WAL. - // Some(None) — entity not found in graph; emit ERR_ENTITY_NOT_FOUND. - // None — graph not found; emit ERR_GRAPH_NOT_FOUND. - let invalidate_body = - |gs: &mut crate::graph::store::GraphStore| -> Option>>> { - let named_graph = gs.get_graph_mut(&graph_name)?; - let mutated = if is_node { - let node_key: crate::graph::types::NodeKey = - slotmap::KeyData::from_ffi(entity_id).into(); - if let Some(node) = named_graph.write_buf.get_node_mut(node_key) { - node.valid_to = wall_ms; - true - } else { - false - } - } else { - let edge_key: crate::graph::types::EdgeKey = - slotmap::KeyData::from_ffi(entity_id).into(); - if let Some(edge) = named_graph.write_buf.get_edge_mut(edge_key) { - edge.valid_to = wall_ms; - true - } else { - false - } + // Multi-shard: the graph lives on the shard that owns its + // NAME. Ship non-local invalidations there via GraphCommand — + // the shard-side handler applies the mutation and drains the + // graph WAL on the owning shard. + if ctx.num_shards > 1 { + let owner = crate::shard::dispatch::graph_to_shard(&graph_name, ctx.num_shards); + if owner != ctx.shard_id { + let (reply_tx, reply_rx) = crate::runtime::channel::oneshot(); + let msg = crate::shard::dispatch::ShardMessage::GraphCommand { + command: std::sync::Arc::new(frame.clone()), + reply_tx, }; - if mutated { - let payload = crate::persistence::wal_v3::record::encode_graph_temporal( - entity_id, is_node, wall_ms, wall_ms, - ); - gs.wal_pending.push(payload); - Some(Some(gs.drain_wal())) + crate::shard::coordinator::spsc_send( + &ctx.dispatch_tx, + ctx.shard_id, + owner, + msg, + &ctx.spsc_notifiers, + ) + .await; + let response = match reply_rx.recv().await { + Ok(f) => f, + Err(_) => Frame::Error(Bytes::from_static( + b"ERR cross-shard reply channel closed", + )), + }; + responses.push(response); + return true; + } + } + let wall_ms = capture_wall_ms(); + // Phase 2f: gate on is_initialized(); both branches are + // semantically identical, the new path uses + // ShardSlice::graph_store directly (no lock). + let (result, wal_records) = if crate::shard::slice::is_initialized() { + crate::shard::slice::with_shard(|s| { + let gs = &mut s.graph_store; + let r = crate::command::temporal::apply_invalidate( + gs, + entity_id, + is_node, + &graph_name, + wall_ms, + ); + let recs = if r.is_ok() { + gs.drain_wal() } else { - Some(None) - } - }; - let outcome = if crate::shard::slice::is_initialized() { - crate::shard::slice::with_shard(|s| invalidate_body(&mut s.graph_store)) + Vec::new() + }; + (r, recs) + }) } else { let mut gs = ctx.shard_databases.graph_store_write(ctx.shard_id); - let r = invalidate_body(&mut gs); - drop(gs); - r + let r = crate::command::temporal::apply_invalidate( + &mut gs, + entity_id, + is_node, + &graph_name, + wall_ms, + ); + let recs = if r.is_ok() { + gs.drain_wal() + } else { + Vec::new() + }; + (r, recs) }; - match outcome { - Some(Some(wal_records)) => { + match result { + Ok(()) => { for record in wal_records { ctx.shard_databases .wal_append(ctx.shard_id, Bytes::from(record)); } responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); } - Some(None) => { - responses.push(Frame::Error(Bytes::from_static(ERR_ENTITY_NOT_FOUND))); - } - None => { - responses.push(Frame::Error(Bytes::from_static(ERR_GRAPH_NOT_FOUND))); - } + Err(err) => responses.push(Frame::Error(Bytes::from_static(err))), } } #[cfg(not(feature = "graph"))] { - let _ = (entity_id, is_node, graph_name, wall_ms, ctx); + let _ = (entity_id, is_node, graph_name, frame, ctx); responses.push(Frame::Error(Bytes::from_static( b"ERR graph feature not enabled", ))); diff --git a/src/server/conn/handler_sharded/write.rs b/src/server/conn/handler_sharded/write.rs index 5a150e7f..ff9d6fbc 100644 --- a/src/server/conn/handler_sharded/write.rs +++ b/src/server/conn/handler_sharded/write.rs @@ -60,13 +60,16 @@ pub(super) fn try_handle_ws_command( created_at, }; { - let mut guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.workspace_registry(); let reg = guard.get_or_insert_with(|| { Box::new(crate::workspace::WorkspaceRegistry::new()) }); reg.insert(ws_id, meta); } - // WAL: WorkspaceCreate record + // WAL: WorkspaceCreate record. The registry is global, so its + // WAL stream is pinned to shard 0 — one stream gives replay a + // total order over Create/Drop regardless of which connection + // issued them. let payload = crate::workspace::wal::encode_workspace_create(ws_id.as_bytes(), &ws_name); let mut wal_buf = Vec::new(); @@ -76,8 +79,7 @@ pub(super) fn try_handle_ws_command( crate::persistence::wal_v3::record::WalRecordType::WorkspaceCreate, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(0, Bytes::from(wal_buf)); responses.push(Frame::BulkString(Bytes::from(ws_id.to_string()))); } Err(e) => responses.push(e), @@ -90,7 +92,7 @@ pub(super) fn try_handle_ws_command( Ok(ws_id_raw) => match parse_workspace_id_from_bytes(&ws_id_raw) { Some(ws_id) => { let removed = { - let mut guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.workspace_registry(); match guard.as_mut() { Some(reg) => reg.remove(&ws_id).is_some(), None => false, @@ -107,8 +109,7 @@ pub(super) fn try_handle_ws_command( crate::persistence::wal_v3::record::WalRecordType::WorkspaceDrop, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(0, Bytes::from(wal_buf)); // Best-effort cleanup: delete all KV keys with ws // prefix (WS-03). // Phase 2f: gate on is_initialized(); new path uses ShardSlice directly. @@ -126,7 +127,14 @@ pub(super) fn try_handle_ws_command( }); } else { let prefix = format!("{{{}}}:", ws_id.as_hex()); - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, 0); + // The {wsid} hash tag co-locates every workspace + // key on ONE shard — clean up there, not on the + // connection's shard. + let owner = crate::shard::dispatch::key_to_shard( + prefix.as_bytes(), + ctx.num_shards, + ); + let mut db_guard = ctx.shard_databases.write_db(owner, 0); let keys_to_delete: Vec> = db_guard .keys() .filter(|k| k.as_bytes().starts_with(prefix.as_bytes())) @@ -153,7 +161,7 @@ pub(super) fn try_handle_ws_command( if sub.eq_ignore_ascii_case(b"LIST") { match validate_ws_list(cmd_args) { Ok(()) => { - let guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let guard = ctx.shard_databases.workspace_registry(); let entries: Vec = match guard.as_ref() { Some(reg) => reg .iter() @@ -181,7 +189,7 @@ pub(super) fn try_handle_ws_command( match validate_ws_info(cmd_args) { Ok(ws_id_raw) => match parse_workspace_id_from_bytes(&ws_id_raw) { Some(ws_id) => { - let guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let guard = ctx.shard_databases.workspace_registry(); let found = guard.as_ref().and_then(|reg| reg.get(&ws_id)); match found { Some(meta) => { @@ -218,7 +226,7 @@ pub(super) fn try_handle_ws_command( match parse_workspace_id_from_bytes(&ws_id_raw) { Some(ws_id) => { let found = { - let guard = ctx.shard_databases.workspace_registry(ctx.shard_id); + let guard = ctx.shard_databases.workspace_registry(); guard .as_ref() .map_or(false, |reg| reg.get(&ws_id).is_some()) @@ -270,6 +278,15 @@ pub(super) fn try_handle_mq_command( Ok((queue_key, max_delivery_count, _debounce_ms)) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // A durable queue lives on the shard that owns its key — the + // stream, registry entry, trigger entry, and WAL records must + // all target `owner`, not the connection's shard, or fresh + // connections landing elsewhere (SO_REUSEPORT) can't see the + // queue. Lock-mode ShardDatabases permits direct cross-shard + // access; the ShardSlice branches stay conn-local because + // slice mode is never initialized yet (owner-routing there is + // the shardslice-migration task). + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); // Phase 2f: gate on is_initialized(); new path uses ShardSlice directly. let create_result: Result<(), Frame> = if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard_db(conn.selected_db, |db| { @@ -285,7 +302,7 @@ pub(super) fn try_handle_mq_command( } }) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); let r = match db_guard.get_or_create_stream(&effective_key) { Ok(stream) => { stream.durable = true; @@ -307,12 +324,14 @@ pub(super) fn try_handle_mq_command( let config = crate::mq::DurableStreamConfig::new(effective_key.clone(), max_delivery_count); { - let mut guard = ctx.shard_databases.durable_queue_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.durable_queue_registry(owner); let reg = guard .get_or_insert_with(|| Box::new(crate::mq::DurableQueueRegistry::new())); reg.insert(effective_key.clone(), config); } + // Owner's WAL so replay restores the registry on the shard + // that also holds the stream. let payload = crate::mq::wal::encode_mq_create(&effective_key, max_delivery_count); let mut wal_buf = Vec::new(); crate::persistence::wal_v3::record::write_wal_v3_record( @@ -321,8 +340,7 @@ pub(super) fn try_handle_mq_command( crate::persistence::wal_v3::record::WalRecordType::MqCreate, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(owner, Bytes::from(wal_buf)); responses.push(Frame::SimpleString(Bytes::from_static(b"OK"))); } Err(e) => responses.push(e), @@ -335,6 +353,8 @@ pub(super) fn try_handle_mq_command( Ok((queue_key, fields)) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // Owner-shard targeting — see MQ CREATE above. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); // Phase 2f: gate on is_initialized(); new path uses ShardSlice directly. // Push outcome: Ok(Some(msg_id)) = pushed; Ok(None) = not durable; Err(e) = stream error. type PushOutcome = Result, Frame>; @@ -355,7 +375,7 @@ pub(super) fn try_handle_mq_command( } }) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); let r = match db_guard.get_stream_mut(&effective_key) { Ok(Some(stream)) => { if !stream.durable { @@ -374,8 +394,9 @@ pub(super) fn try_handle_mq_command( }; match push_outcome { Ok(Some(msg_id)) => { + // Owner's registry: its event-loop tick fires triggers. { - let mut trig_guard = ctx.shard_databases.trigger_registry(ctx.shard_id); + let mut trig_guard = ctx.shard_databases.trigger_registry(owner); if let Some(reg) = trig_guard.as_mut() { let trig_key = if let Some(ws_id) = conn.workspace_id.as_ref() { let ws_hex = ws_id.as_hex(); @@ -418,6 +439,8 @@ pub(super) fn try_handle_mq_command( Ok((queue_key, count)) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // Owner-shard targeting — see MQ CREATE above. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); let group_name = Bytes::from_static(b"__mq_consumers"); let consumer_name = Bytes::from_static(b"__mq_default"); @@ -520,7 +543,7 @@ pub(super) fn try_handle_mq_command( let response = if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard_db(conn.selected_db, pop_body) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); let r = pop_body(&mut *db_guard); drop(db_guard); r @@ -541,6 +564,8 @@ pub(super) fn try_handle_mq_command( .iter() .map(|(ms, seq)| StreamId { ms: *ms, seq: *seq }) .collect(); + // Owner-shard targeting — see MQ CREATE above. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); // Phase 2f: gate on is_initialized(); new path uses ShardSlice directly. // Closure returns Some(acked_count) on success, None on any error/miss. let ack_body = |db: &mut crate::storage::db::Database| -> Option { @@ -559,7 +584,7 @@ pub(super) fn try_handle_mq_command( let acked = if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard_db(conn.selected_db, ack_body) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); let r = ack_body(&mut *db_guard); drop(db_guard); r @@ -575,8 +600,7 @@ pub(super) fn try_handle_mq_command( crate::persistence::wal_v3::record::WalRecordType::MqAck, &payload, ); - ctx.shard_databases - .wal_append(ctx.shard_id, Bytes::from(wal_buf)); + ctx.shard_databases.wal_append(owner, Bytes::from(wal_buf)); } responses.push(Frame::Integer(acked_count as i64)); } @@ -593,6 +617,9 @@ pub(super) fn try_handle_mq_command( Ok(queue_key) => { let effective_key = crate::workspace::workspace_key(conn.workspace_id.as_ref(), &queue_key); + // Owner of the QUEUE key, not the dlq_key: POP creates the + // DLQ stream in the same db as the queue it drains. + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); let dlq_key = { let mut buf = Vec::with_capacity(effective_key.len() + 8); buf.extend_from_slice(&effective_key); @@ -609,7 +636,7 @@ pub(super) fn try_handle_mq_command( let len = if crate::shard::slice::is_initialized() { crate::shard::slice::with_shard_db(conn.selected_db, dlq_body) } else { - let mut db_guard = ctx.shard_databases.write_db(ctx.shard_id, conn.selected_db); + let mut db_guard = ctx.shard_databases.write_db(owner, conn.selected_db); let r = dlq_body(&mut *db_guard); drop(db_guard); r @@ -636,6 +663,9 @@ pub(super) fn try_handle_mq_command( } else { queue_key.clone() }; + // Owner's registry: its event-loop tick fires triggers + // (timers.rs documents the home shard as authoritative). + let owner = crate::shard::dispatch::key_to_shard(&effective_key, ctx.num_shards); let entry = crate::mq::TriggerEntry { queue_key: effective_key, callback_cmd, @@ -644,7 +674,7 @@ pub(super) fn try_handle_mq_command( pending_fire_ms: 0, }; { - let mut guard = ctx.shard_databases.trigger_registry(ctx.shard_id); + let mut guard = ctx.shard_databases.trigger_registry(owner); let reg = guard.get_or_insert_with(|| Box::new(crate::mq::TriggerRegistry::new())); reg.register(trig_key, entry); @@ -741,9 +771,10 @@ pub(super) fn try_handle_multi_exec( /// Handle GRAPH.* graph commands. Returns `true` if consumed. #[cfg(feature = "graph")] -pub(super) fn try_handle_graph_command( +pub(super) async fn try_handle_graph_command( cmd: &[u8], cmd_args: &[Frame], + frame: &Frame, conn: &mut ConnectionState, ctx: &ConnectionContext, responses: &mut Vec, @@ -751,6 +782,68 @@ pub(super) fn try_handle_graph_command( if cmd.len() <= 6 || !cmd[..6].eq_ignore_ascii_case(b"GRAPH.") { return false; } + // Multi-shard: a graph lives on the shard that owns its NAME (same xxh64 + // + {tag} hashing as key routing). The per-shard graph store is + // thread-local once ShardSlice is initialized, so non-owner commands MUST + // hop via ShardMessage::GraphCommand — the shard-side handler dispatches + // on its own store and drains graph WAL records locally. + // GRAPH.LIST has no name argument and stays connection-local (it reports + // this shard's graphs only — recorded as a v3 observe delta). + if ctx.num_shards > 1 && !cmd.eq_ignore_ascii_case(b"GRAPH.LIST") { + if let Some(name) = cmd_args.first().and_then(extract_bytes) { + let owner = crate::shard::dispatch::graph_to_shard(&name, ctx.num_shards); + if owner != ctx.shard_id { + // Cypher WRITE queries inside a cross-store TXN cannot ship + // their undo intents back across the hop — reject like the + // other two-domain TXN cases (MOVE, COPY ... DB). + if conn.in_cross_txn() + && cmd.eq_ignore_ascii_case(b"GRAPH.QUERY") + && crate::command::graph::is_cypher_write_query(cmd_args) + { + responses.push(Frame::Error(bytes::Bytes::from_static( + crate::command::transaction::ERR_TXN_CROSS_SHARD, + ))); + return true; + } + let (reply_tx, reply_rx) = crate::runtime::channel::oneshot(); + let msg = crate::shard::dispatch::ShardMessage::GraphCommand { + command: std::sync::Arc::new(frame.clone()), + reply_tx, + }; + crate::shard::coordinator::spsc_send( + &ctx.dispatch_tx, + ctx.shard_id, + owner, + msg, + &ctx.spsc_notifiers, + ) + .await; + let mut response = match reply_rx.recv().await { + Ok(f) => f, + Err(_) => Frame::Error(bytes::Bytes::from_static( + b"ERR cross-shard reply channel closed", + )), + }; + // Phase 166: explicit ADDNODE/ADDEDGE intents are captured + // from the routed RESPONSE id, exactly like the local path; + // the abort path routes the rollback back to the owner. + if let Some(txn) = conn.active_cross_txn.as_mut() { + let is_node = cmd.eq_ignore_ascii_case(b"GRAPH.ADDNODE"); + let is_edge = cmd.eq_ignore_ascii_case(b"GRAPH.ADDEDGE"); + if is_node || is_edge { + if let Frame::Integer(id) = &response { + txn.record_graph(*id as u64, is_node, name.clone()); + } + } + } + if let Some(ws_id) = conn.workspace_id.as_ref() { + strip_workspace_prefix_from_response(ws_id, cmd, &mut response); + } + responses.push(response); + return true; + } + } + } let is_write = crate::command::graph::is_graph_write_cmd(cmd) || (cmd.eq_ignore_ascii_case(b"GRAPH.QUERY") && crate::command::graph::is_cypher_write_query(cmd_args)); diff --git a/src/server/conn/shared.rs b/src/server/conn/shared.rs index fa6c8f4a..94163c58 100644 --- a/src/server/conn/shared.rs +++ b/src/server/conn/shared.rs @@ -296,6 +296,62 @@ pub(crate) fn extract_primary_key<'a>(cmd: &[u8], args: &'a [Frame]) -> Option<& _ => None, }; } + // XGROUP ...: args[0] is the subcommand ("CREATE", + // "SETID", "DESTROY", "DELCONSUMER", "CREATECONSUMER") and args[1] is + // the stream key. Same pattern as OBJECT above. + // XINFO STREAM/GROUPS/CONSUMERS : identical layout. + if (len == 6 && b0 == b'x' && cmd.eq_ignore_ascii_case(b"XGROUP")) + || (len == 5 && b0 == b'x' && cmd.eq_ignore_ascii_case(b"XINFO")) + { + return match args.get(1) { + Some(Frame::BulkString(key)) => Some(key), + _ => None, + }; + } + // BITOP destkey key [key ...]: args[0] is the operation literal + // ("AND"/"OR"/"XOR"/"NOT"); the first key is the DESTINATION at args[1] + // (metadata first_key=2). Routing by args[0] hashed the operation name. + // Multi-shard servers consume BITOP in the multi-key coordinator before + // this routing runs; this arm fixes single-shard-local + cluster-slot + // routing. + if (len, b0) == (5, b'b') && cmd.eq_ignore_ascii_case(b"BITOP") { + return match args.get(1) { + Some(Frame::BulkString(key)) => Some(key), + _ => None, + }; + } + // ZDIFF/ZINTER/ZUNION/ZINTERCARD numkeys key [key ...]: + // args[0] is the integer numkeys literal; the first actual key is args[1]. + // Routing by args[0] would hash "2" (the numkeys string) to an arbitrary + // shard and produce "ERR wrong type" or a silent miss. + // Note: ZDIFF = 5 bytes, ZINTER/ZUNION = 6 bytes, ZINTERCARD = 10 bytes. + if (len == 5 && b0 == b'z' && cmd.eq_ignore_ascii_case(b"ZDIFF")) + || (len == 6 + && b0 == b'z' + && (cmd.eq_ignore_ascii_case(b"ZINTER") || cmd.eq_ignore_ascii_case(b"ZUNION"))) + || (len == 10 && b0 == b'z' && cmd.eq_ignore_ascii_case(b"ZINTERCARD")) + { + return match args.get(1) { + Some(Frame::BulkString(key)) => Some(key), + _ => None, + }; + } + // XREAD [COUNT count] [BLOCK ms] STREAMS key [key ...] id [id ...]: + // Scan args for the STREAMS token; the key immediately follows it. + // No allocation — scan &[Frame] linearly. + if len == 5 && b0 == b'x' && cmd.eq_ignore_ascii_case(b"XREAD") { + for (i, arg) in args.iter().enumerate() { + if let Frame::BulkString(tok) = arg { + if tok.eq_ignore_ascii_case(b"STREAMS") { + return match args.get(i + 1) { + Some(Frame::BulkString(key)) => Some(key), + _ => None, + }; + } + } + } + return None; + } match &args[0] { Frame::BulkString(key) => Some(key), _ => None, @@ -318,6 +374,20 @@ pub(crate) fn is_multi_key_command(cmd: &[u8], args: &[Frame]) -> bool { (3, b'd') => args.len() > 1 && cmd.eq_ignore_ascii_case(b"DEL"), (6, b'u') => args.len() > 1 && cmd.eq_ignore_ascii_case(b"UNLINK"), (6, b'e') => args.len() > 1 && cmd.eq_ignore_ascii_case(b"EXISTS"), + // BITOP dest src...: dest + sources can live on different shards. + (5, b'b') => args.len() >= 3 && cmd.eq_ignore_ascii_case(b"BITOP"), + // COPY src dst [REPLACE]: src and dst can live on different shards. + // The `COPY ... DB n` form is EXCLUDED: it needs two databases and is + // owned by the handlers' two-db interception (cross-db + cross-shard + // simultaneously is unsupported, as before). + (4, b'c') => { + args.len() >= 2 + && cmd.eq_ignore_ascii_case(b"COPY") + && !args + .iter() + .skip(2) + .any(|a| matches!(a, Frame::BulkString(o) if o.eq_ignore_ascii_case(b"DB"))) + } _ => false, } } diff --git a/src/shard/coordinator.rs b/src/shard/coordinator.rs index c52f6a62..eef3e8fb 100644 --- a/src/shard/coordinator.rs +++ b/src/shard/coordinator.rs @@ -68,6 +68,32 @@ pub async fn coordinate_multi_key( _response_pool, ) .await + } else if cmd.eq_ignore_ascii_case(b"BITOP") { + coordinate_bitop( + args, + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + _response_pool, + ) + .await + } else if cmd.eq_ignore_ascii_case(b"COPY") { + coordinate_copy( + args, + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + _response_pool, + ) + .await } else { // DEL, UNLINK, EXISTS with multiple keys coordinate_multi_del_or_exists( @@ -86,6 +112,478 @@ pub async fn coordinate_multi_key( } } +// --------------------------------------------------------------------------- +// Shared legs for the BITOP / COPY coordinators +// --------------------------------------------------------------------------- + +/// Run one full command on the LOCAL shard through the real dispatcher +/// (identical semantics to a remote MultiExecute leg, minus the hop). +fn run_local( + shard_databases: &Arc, + my_shard: usize, + db_index: usize, + cached_clock: &CachedClock, + cmd: &[u8], + args: &[Frame], +) -> Frame { + let db_count = shard_databases.db_count(); + let mut selected = db_index; + let mut run = |db: &mut crate::storage::Database| { + db.refresh_now_from_cache(cached_clock); + match cmd_dispatch(db, cmd, args, &mut selected, db_count) { + DispatchResult::Response(f) | DispatchResult::Quit(f) => f, + } + }; + // Phase 2c: gate on is_initialized(); new path uses ShardSlice directly. + if crate::shard::slice::is_initialized() { + crate::shard::slice::with_shard_db(db_index, run) + } else { + let mut guard = shard_databases.write_db(my_shard, db_index); + run(&mut guard) + } +} + +/// Send one full command to a REMOTE shard and await its reply. +async fn run_remote( + target_shard: usize, + routing_key: &Bytes, + command: Frame, + my_shard: usize, + db_index: usize, + dispatch_tx: &Rc>>>, + spsc_notifiers: &[Arc], +) -> Frame { + // ChannelMesh has no self-send slot — local legs must go via run_local. + debug_assert_ne!(target_shard, my_shard, "run_remote called for own shard"); + let (reply_tx, reply_rx) = channel::oneshot(); + let msg = ShardMessage::MultiExecute { + db_index, + commands: vec![(routing_key.clone(), command)], + reply_tx, + }; + spsc_send(dispatch_tx, my_shard, target_shard, msg, spsc_notifiers).await; + match reply_rx.recv().await { + Ok(mut frames) if !frames.is_empty() => frames.swap_remove(0), + _ => Frame::Error(Bytes::from_static(b"ERR cross-shard reply channel closed")), + } +} + +/// Run one full command on whichever shard owns `routing_key`. +#[allow(clippy::too_many_arguments)] +async fn run_on_owner( + routing_key: &Bytes, + command_parts: &[Frame], + my_shard: usize, + num_shards: usize, + db_index: usize, + shard_databases: &Arc, + dispatch_tx: &Rc>>>, + spsc_notifiers: &[Arc], + cached_clock: &CachedClock, +) -> Frame { + let owner = key_to_shard(routing_key, num_shards); + if owner == my_shard { + let (cmd, args) = match command_parts.split_first() { + Some((Frame::BulkString(c), rest)) => (c.clone(), rest), + _ => return Frame::Error(Bytes::from_static(b"ERR invalid command format")), + }; + run_local( + shard_databases, + my_shard, + db_index, + cached_clock, + &cmd, + args, + ) + } else { + let command = Frame::Array(command_parts.to_vec().into()); + run_remote( + owner, + routing_key, + command, + my_shard, + db_index, + dispatch_tx, + spsc_notifiers, + ) + .await + } +} + +fn bulk(b: &Bytes) -> Frame { + Frame::BulkString(b.clone()) +} + +fn bulk_static(s: &'static [u8]) -> Frame { + Frame::BulkString(Bytes::from_static(s)) +} + +/// Coordinate BITOP across shards. +/// +/// `BITOP dest src [src ...]` — sources are gathered (local read or +/// remote GET), combined via the same `bitop_compute` the local path uses, +/// and the result is written on DEST's owning shard (SET, or DEL when the +/// combine is empty). Full Redis semantics: BITOP is string-only by spec, +/// so value transfer is exact; WRONGTYPE from any source propagates. +#[allow(clippy::too_many_arguments)] +async fn coordinate_bitop( + args: &[Frame], + my_shard: usize, + num_shards: usize, + db_index: usize, + shard_databases: &Arc, + dispatch_tx: &Rc>>>, + spsc_notifiers: &[Arc], + cached_clock: &CachedClock, + _response_pool: &(), +) -> Frame { + // Single-shard server: straight to local dispatch — zero coordinator + // overhead (no key vec, no owner hashing) on the 1-shard hot path. + if num_shards == 1 { + return run_local( + shard_databases, + my_shard, + db_index, + cached_clock, + b"BITOP", + args, + ); + } + if args.len() < 3 { + return Frame::Error(Bytes::from_static( + b"ERR wrong number of arguments for 'BITOP' command", + )); + } + let (Some(op), Some(dest)) = (extract_key(&args[0]), extract_key(&args[1])) else { + return Frame::Error(Bytes::from_static( + b"ERR wrong number of arguments for 'BITOP' command", + )); + }; + // Redis validation order: NOT arity errors before any key is touched. + if op.eq_ignore_ascii_case(b"NOT") && args.len() != 3 { + return Frame::Error(Bytes::from_static( + b"ERR BITOP NOT requires one and only one key", + )); + } + let mut src_keys: Vec = Vec::with_capacity(args.len() - 2); + for arg in &args[2..] { + match extract_key(arg) { + Some(k) => src_keys.push(k), + None => { + return Frame::Error(Bytes::from_static( + b"ERR wrong number of arguments for 'BITOP' command", + )); + } + } + } + + // Single-owner fast path: every key (dest included) on one shard — + // forward the whole command for byte-identical local semantics. + let dest_shard = key_to_shard(&dest, num_shards); + if src_keys + .iter() + .all(|k| key_to_shard(k, num_shards) == dest_shard) + { + let mut parts: Vec = Vec::with_capacity(args.len() + 1); + parts.push(bulk_static(b"BITOP")); + parts.extend_from_slice(args); + return run_on_owner( + &dest, + &parts, + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await; + } + + // Gather sources: group by shard ascending (VLL), local legs direct, + // remote legs as GET batches. + let mut groups: BTreeMap> = BTreeMap::new(); + for (i, k) in src_keys.iter().enumerate() { + groups + .entry(key_to_shard(k, num_shards)) + .or_default() + .push((i, k.clone())); + } + let mut sources: Vec>> = vec![None; src_keys.len()]; + let mut pending: Vec<(Vec, channel::OneshotReceiver>)> = Vec::new(); + for (shard_id, indexed) in &groups { + if *shard_id == my_shard { + for (idx, key) in indexed { + let reply = run_local( + shard_databases, + my_shard, + db_index, + cached_clock, + b"GET", + &[bulk(key)], + ); + match reply { + Frame::BulkString(v) => sources[*idx] = Some(v.to_vec()), + Frame::Error(e) => return Frame::Error(e), + _ => sources[*idx] = Some(Vec::new()), + } + } + } else { + let (reply_tx, reply_rx) = channel::oneshot(); + let commands: Vec<(Bytes, Frame)> = indexed + .iter() + .map(|(_, k)| { + ( + k.clone(), + Frame::Array(framevec![bulk_static(b"GET"), bulk(k)]), + ) + }) + .collect(); + let indices: Vec = indexed.iter().map(|(i, _)| *i).collect(); + let msg = ShardMessage::MultiExecute { + db_index, + commands, + reply_tx, + }; + spsc_send(dispatch_tx, my_shard, *shard_id, msg, spsc_notifiers).await; + pending.push((indices, reply_rx)); + } + } + for (indices, reply_rx) in pending { + match reply_rx.recv().await { + Ok(frames) => { + for (idx, frame) in indices.into_iter().zip(frames) { + match frame { + Frame::BulkString(v) => sources[idx] = Some(v.to_vec()), + Frame::Error(e) => return Frame::Error(e), + _ => sources[idx] = Some(Vec::new()), + } + } + } + Err(_) => { + return Frame::Error(Bytes::from_static(b"ERR cross-shard reply channel closed")); + } + } + } + let gathered: Vec> = sources.into_iter().map(Option::unwrap_or_default).collect(); + + match crate::command::string::bitop_compute(&op, &gathered) { + Err(e) => e, + Ok(None) => { + // All sources empty/missing — dest is deleted, reply 0. + let reply = run_on_owner( + &dest, + &[bulk_static(b"DEL"), bulk(&dest)], + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await; + if let Frame::Error(e) = reply { + return Frame::Error(e); + } + Frame::Integer(0) + } + Ok(Some(result)) => { + let len = result.len() as i64; + let reply = run_on_owner( + &dest, + &[ + bulk_static(b"SET"), + bulk(&dest), + Frame::BulkString(Bytes::from(result)), + ], + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await; + if let Frame::Error(e) = reply { + return Frame::Error(e); + } + Frame::Integer(len) + } + } +} + +/// Coordinate COPY across shards. +/// +/// `COPY src dst [REPLACE]` — same-shard pairs (hash tags) forward to the +/// owning shard and keep full any-type fidelity via the local copy path. +/// Cross-shard pairs transfer STRING values exactly (value + TTL, NX unless +/// REPLACE); cross-shard non-string values return an explicit error instead +/// of silently corrupting (full-fidelity transfer is DUMP/RESTORE territory, +/// tracked in the task backlog). `COPY ... DB n` never reaches this path +/// (excluded in `is_multi_key_command`; the handlers' two-db interception +/// keeps owning it). +#[allow(clippy::too_many_arguments)] +async fn coordinate_copy( + args: &[Frame], + my_shard: usize, + num_shards: usize, + db_index: usize, + shard_databases: &Arc, + dispatch_tx: &Rc>>>, + spsc_notifiers: &[Arc], + cached_clock: &CachedClock, + _response_pool: &(), +) -> Frame { + // Single-shard server: straight to local dispatch — zero coordinator + // overhead on the 1-shard hot path. + if num_shards == 1 { + return run_local( + shard_databases, + my_shard, + db_index, + cached_clock, + b"COPY", + args, + ); + } + if args.len() < 2 { + return Frame::Error(Bytes::from_static( + b"ERR wrong number of arguments for 'COPY' command", + )); + } + let (Some(src), Some(dst)) = (extract_key(&args[0]), extract_key(&args[1])) else { + return Frame::Error(Bytes::from_static( + b"ERR wrong number of arguments for 'COPY' command", + )); + }; + let mut replace = false; + for opt in &args[2..] { + match extract_key(opt) { + Some(o) if o.eq_ignore_ascii_case(b"REPLACE") => replace = true, + _ => return Frame::Error(Bytes::from_static(b"ERR syntax error")), + } + } + + let src_shard = key_to_shard(&src, num_shards); + let dst_shard = key_to_shard(&dst, num_shards); + + // Same owner: forward the whole COPY — full any-type fidelity. + if src_shard == dst_shard { + let mut parts: Vec = Vec::with_capacity(args.len() + 1); + parts.push(bulk_static(b"COPY")); + parts.extend_from_slice(args); + return run_on_owner( + &src, + &parts, + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await; + } + + // Cross-shard: read value + TTL from src's shard. + let value = match run_on_owner( + &src, + &[bulk_static(b"GET"), bulk(&src)], + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await + { + Frame::BulkString(v) => v, + Frame::Error(e) if e.starts_with(b"WRONGTYPE") => { + return Frame::Error(Bytes::from_static( + b"ERR COPY across shards supports only string values; co-locate the keys with {hash} tags for other types", + )); + } + Frame::Error(e) => return Frame::Error(e), + _ => return Frame::Integer(0), // src missing + }; + let ttl_ms = match run_on_owner( + &src, + &[bulk_static(b"PTTL"), bulk(&src)], + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await + { + Frame::Integer(t) if t > 0 => Some(t), + Frame::Integer(-2) => return Frame::Integer(0), // expired between reads + _ => None, + }; + + // Write to dst's shard: NX unless REPLACE, then restore TTL. + let set_parts: Vec = if replace { + vec![bulk_static(b"SET"), bulk(&dst), Frame::BulkString(value)] + } else { + vec![ + bulk_static(b"SET"), + bulk(&dst), + Frame::BulkString(value), + bulk_static(b"NX"), + ] + }; + let set_reply = run_on_owner( + &dst, + &set_parts, + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await; + match set_reply { + Frame::SimpleString(_) => {} + Frame::Error(e) => return Frame::Error(e), + // Null reply = NX refused (dst exists); anything else is unexpected. + _ => return Frame::Integer(0), + } + if let Some(t) = ttl_ms { + let mut ttl_buf = itoa::Buffer::new(); + let reply = run_on_owner( + &dst, + &[ + bulk_static(b"PEXPIRE"), + bulk(&dst), + Frame::BulkString(Bytes::copy_from_slice(ttl_buf.format(t).as_bytes())), + ], + my_shard, + num_shards, + db_index, + shard_databases, + dispatch_tx, + spsc_notifiers, + cached_clock, + ) + .await; + if let Frame::Error(e) = reply { + return Frame::Error(e); + } + } + Frame::Integer(1) +} + /// Extract Bytes from a Frame argument. fn extract_key(frame: &Frame) -> Option { match frame { diff --git a/src/shard/dispatch.rs b/src/shard/dispatch.rs index 778179ef..4d9fb77e 100644 --- a/src/shard/dispatch.rs +++ b/src/shard/dispatch.rs @@ -330,6 +330,19 @@ pub struct GraphTraversePayload { pub reply_tx: channel::OneshotSender, } +/// Boxed payload for `ShardMessage::GraphRollback` (multi-shard TXN.ABORT). +/// +/// Carries the graph-undo ops and create-intents whose graph names hash to +/// the receiving shard, so the rollback mutates the store that actually owns +/// those graphs. Boxed: two Vecs + oneshot would blow the 64 B enum ceiling. +#[cfg(feature = "graph")] +pub struct GraphRollbackPayload { + pub txn_id: u64, + pub graph_undo: Vec, + pub graph_intents: Vec, + pub reply_tx: channel::OneshotSender, +} + /// Boxed payload for `ShardMessage::CdcSubscribe` (C3b-2). /// /// Carries the per-subscriber sink and replay start point to the shard @@ -545,6 +558,10 @@ pub enum ShardMessage { /// Boxed (Phase 177) — Vec + Bytes + oneshot pushed this past 80 B inline. #[cfg(feature = "graph")] GraphTraverse(Box), + /// Multi-shard TXN.ABORT: apply graph rollback (undo ops + create-intent + /// removal) for graphs owned by this shard. + #[cfg(feature = "graph")] + GraphRollback(Box), /// Cross-shard PUBLISH with shared atomic response slot for subscriber count accumulation. /// /// Boxed (Phase 177) — two Bytes + Arc was 72 B inline. diff --git a/src/shard/shared_databases.rs b/src/shard/shared_databases.rs index a32b34d5..9caf6327 100644 --- a/src/shard/shared_databases.rs +++ b/src/shard/shared_databases.rs @@ -232,14 +232,15 @@ impl ShardDatabases { self.temporal_kv_indexes[shard_id].lock() } - /// Acquire the per-shard WorkspaceRegistry lock. + /// Acquire the GLOBAL WorkspaceRegistry lock (slot 0 of the per-shard + /// array). Workspaces are control-plane objects looked up by every + /// connection regardless of which shard accepted it, so all paths — + /// handlers, uring intercept, WAL replay — share one registry. WS + /// commands are rare; a single mutex is not a hot-path concern. /// Caller lazy-inits via `get_or_insert_with(|| Box::new(WorkspaceRegistry::new()))`. #[inline] - pub fn workspace_registry( - &self, - shard_id: usize, - ) -> MutexGuard<'_, Option>> { - self.workspace_registries[shard_id].lock() + pub fn workspace_registry(&self) -> MutexGuard<'_, Option>> { + self.workspace_registries[0].lock() } /// Acquire the per-shard DurableQueueRegistry lock. @@ -303,7 +304,8 @@ impl ShardDatabases { name: bytes::Bytes::from(name), created_at: 0, // WAL doesn't store created_at; use 0 as placeholder }; - let mut guard = self.workspace_registries[shard_id].lock(); + // Global registry (slot 0) — see workspace_registry(). + let mut guard = self.workspace_registries[0].lock(); let reg = guard.get_or_insert_with(|| Box::new(WorkspaceRegistry::new())); reg.insert(ws_id, meta); @@ -313,7 +315,7 @@ impl ShardDatabases { WalRecordType::WorkspaceDrop => { if let Some(ws_bytes) = decode_workspace_drop(&record.payload) { let ws_id = WorkspaceId::from_bytes(ws_bytes); - let mut guard = self.workspace_registries[shard_id].lock(); + let mut guard = self.workspace_registries[0].lock(); if let Some(reg) = guard.as_mut() { reg.remove(&ws_id); } diff --git a/src/shard/spsc_handler.rs b/src/shard/spsc_handler.rs index 4051fc23..fc1e6438 100644 --- a/src/shard/spsc_handler.rs +++ b/src/shard/spsc_handler.rs @@ -147,6 +147,10 @@ pub(crate) fn drain_spsc_shared( ShardMessage::GraphTraverse(_) => { execute_batch.push(msg); } + #[cfg(feature = "graph")] + ShardMessage::GraphRollback(_) => { + execute_batch.push(msg); + } ShardMessage::MigrateConnection(payload) => { pending_migrations.push((payload.fd, payload.state)); } @@ -2166,6 +2170,42 @@ pub(crate) fn handle_shard_message_shared( let _ = reply_tx.send(response); } #[cfg(feature = "graph")] + ShardMessage::GraphRollback(payload) => { + let crate::shard::dispatch::GraphRollbackPayload { + txn_id, + graph_undo, + graph_intents, + reply_tx, + } = *payload; + // Multi-shard TXN.ABORT leg: this shard owns the graphs named in + // these ops. Apply the rollback on the local store and append the + // drained WAL records here so replay sees them on the owning shard. + let wal_records = if crate::shard::slice::is_initialized() { + crate::shard::slice::with_shard(|s| { + crate::transaction::abort::apply_graph_rollback( + &mut s.graph_store, + txn_id, + &graph_undo, + &graph_intents, + ) + }) + } else { + let mut gs = shard_databases.graph_store_write(shard_id); + crate::transaction::abort::apply_graph_rollback( + &mut gs, + txn_id, + &graph_undo, + &graph_intents, + ) + }; + for record in wal_records { + shard_databases.wal_append(shard_id, bytes::Bytes::from(record)); + } + let _ = reply_tx.send(crate::protocol::Frame::SimpleString( + bytes::Bytes::from_static(b"OK"), + )); + } + #[cfg(feature = "graph")] ShardMessage::GraphTraverse(payload) => { let crate::shard::dispatch::GraphTraversePayload { graph_name, diff --git a/src/shard/uring_handler.rs b/src/shard/uring_handler.rs index 06593bde..c50226b7 100644 --- a/src/shard/uring_handler.rs +++ b/src/shard/uring_handler.rs @@ -156,19 +156,32 @@ pub(crate) fn handle_uring_event( let sub = args.first().and_then(|f| { if let crate::protocol::Frame::BulkString(b) = f { Some(b.as_ref()) } else { None } }); - let mut reg_guard = shard_databases.workspace_registry(shard_id); + let mut reg_guard = shard_databases.workspace_registry(); let registry = reg_guard.get_or_insert_with(|| Box::new(crate::workspace::registry::WorkspaceRegistry::new())); match sub { Some(s) if s.eq_ignore_ascii_case(b"CREATE") => { let ws_id = crate::workspace::WorkspaceId::new_v7(); let name = args.get(1).and_then(|f| { if let crate::protocol::Frame::BulkString(b) = f { Some(b.clone()) } else { None } - }); + }).unwrap_or_else(|| bytes::Bytes::from_static(b"")); registry.insert(ws_id, crate::workspace::registry::WorkspaceMetadata { id: ws_id, - name: name.unwrap_or_else(|| bytes::Bytes::from_static(b"")), + name: name.clone(), created_at: 0, }); + // WAL: WorkspaceCreate to shard 0 — the global + // registry's single stream (mirrors the conn + // handlers; without it, workspaces created via + // this batch path were lost on restart). + let payload = crate::workspace::wal::encode_workspace_create(ws_id.as_bytes(), &name); + let mut wal_buf = Vec::new(); + crate::persistence::wal_v3::record::write_wal_v3_record( + &mut wal_buf, + 0, + crate::persistence::wal_v3::record::WalRecordType::WorkspaceCreate, + &payload, + ); + shard_databases.wal_append(0, bytes::Bytes::from(wal_buf)); return crate::protocol::Frame::BulkString(bytes::Bytes::from(ws_id.as_hex())); } Some(s) if s.eq_ignore_ascii_case(b"DROP") => { @@ -177,7 +190,19 @@ pub(crate) fn handle_uring_event( }); if let Some(hex) = ws_hex { if let Some(ws_id) = crate::workspace::WorkspaceId::from_hex(hex) { - registry.remove(&ws_id); + // WAL only a real removal; the OK-even-if-absent + // reply is pre-existing batch-path behavior. + if registry.remove(&ws_id).is_some() { + let payload = crate::workspace::wal::encode_workspace_drop(ws_id.as_bytes()); + let mut wal_buf = Vec::new(); + crate::persistence::wal_v3::record::write_wal_v3_record( + &mut wal_buf, + 0, + crate::persistence::wal_v3::record::WalRecordType::WorkspaceDrop, + &payload, + ); + shard_databases.wal_append(0, bytes::Bytes::from(wal_buf)); + } return crate::protocol::Frame::SimpleString(bytes::Bytes::from_static(b"OK")); } } @@ -208,7 +233,10 @@ pub(crate) fn handle_uring_event( } return crate::protocol::Frame::Error(bytes::Bytes::from_static(b"ERR workspace not found")); } - // WS.AUTH not supported in uring_handler (no per-connection state) + // WS.AUTH not supported in uring_handler (no per-connection + // state). CREATE/DROP above DO mutate the global registry, + // so a workspace created here is AUTH-able from any + // standard connection handler. Some(s) if s.eq_ignore_ascii_case(b"AUTH") => { return crate::protocol::Frame::Error(bytes::Bytes::from_static( b"ERR WS.AUTH not available in io_uring batch mode; use standard connection handler", diff --git a/src/storage/db.rs b/src/storage/db.rs index 9aa75c5f..85f5fa1f 100644 --- a/src/storage/db.rs +++ b/src/storage/db.rs @@ -1870,6 +1870,27 @@ impl Database { } } + /// Read-only stream access for the shared-lock read path. + /// + /// Checks expiry using `now_ms` but does NOT remove the expired key or + /// touch LRU (mirrors `get_if_alive` semantics). Returns `Ok(None)` for + /// missing or expired keys, `Err(WRONGTYPE)` for non-stream keys. + pub fn get_stream_if_alive( + &self, + key: &[u8], + now_ms: u64, + ) -> Result, Frame> { + let base_ts = self.base_timestamp; + match self.data.get(key) { + None => Ok(None), + Some(entry) if entry.is_expired_at(base_ts, now_ms) => Ok(None), + Some(entry) => match entry.value.as_redis_value() { + RedisValueRef::Stream(s) => Ok(Some(s)), + _ => Err(Self::wrongtype_error()), + }, + } + } + /// Get a read-only reference to a stream. Returns Ok(None) if key doesn't exist. /// Returns WRONGTYPE error if key holds another type. pub fn get_stream(&mut self, key: &[u8]) -> Result, Frame> { diff --git a/src/storage/entry.rs b/src/storage/entry.rs index 63757213..d848bd87 100644 --- a/src/storage/entry.rs +++ b/src/storage/entry.rs @@ -324,24 +324,33 @@ fn pack_metadata_u32(last_access: u16, version: u8, access_counter: u8) -> u32 { ((last_access as u32) << 16) | ((version as u32) << 8) | (access_counter as u32) } -/// A compact 24-byte entry in the database, wrapping a CompactValue with TTL delta -/// and packed metadata. +/// A compact 32-byte entry in the database, wrapping a CompactValue with TTL and packed metadata. /// -/// Layout: -/// - `value: CompactValue` (16 bytes) -- SSO for small strings, tagged heap pointer otherwise -/// - `ttl_delta: u32` (4 bytes) -- 0 = no expiry, else seconds from base_timestamp -/// - `metadata: u32` (4 bytes) -- packed [last_access:16 | version:8 | counter:8] +/// Layout (repr(C)): +/// - `value: CompactValue` (16 bytes, offset 0) -- SSO for small strings, tagged heap pointer otherwise +/// - `ttl_secs: u64` (8 bytes, offset 16) -- 0 = no expiry, else absolute Unix seconds (u64 supports +/// timestamps up to year ~584 billion; u32 was limited to year 2106 and silently overflowed for +/// timestamps like 9_999_999_999 used in EXPIREAT regression tests). +/// - `metadata: u32` (4 bytes, offset 24) -- packed [last_access:16 | version:8 | counter:8] +/// - _pad: u32 (4 bytes, offset 28) -- alignment padding +/// +/// NOTE: the size changed from 24 → 32 bytes when `ttl_delta: u32` was widened to +/// `ttl_secs: u64`. Memory overhead per key increases by 8 bytes (1/3 overhead on a +/// 100M-key dataset = ~800 MB). The correctness gain (no silent TTL overflow past +/// year 2106) outweighs the cost for all realistic key counts. #[repr(C)] #[derive(Debug, Clone)] pub struct CompactEntry { pub value: CompactValue, - /// TTL delta in seconds from the per-database base_timestamp. 0 = no expiry. - pub ttl_delta: u32, + /// Absolute expiry time in Unix seconds. 0 = no expiry. + /// Widened from u32 to u64 to support timestamps beyond year 2106. + pub ttl_secs: u64, /// Packed metadata: [last_access:16 | version:8 | access_counter:8] pub metadata: u32, + _pad: u32, } -const _: () = assert!(std::mem::size_of::() == 24); +const _: () = assert!(std::mem::size_of::() == 32); /// Type alias for backward compatibility during migration. pub type Entry = CompactEntry; @@ -395,48 +404,41 @@ impl CompactEntry { } // --- Expiry helpers --- - // TTL is stored as a delta from a per-database base_timestamp. - // These methods take base_ts to convert to/from absolute milliseconds. + // TTL is stored as absolute Unix seconds (u64). 0 = no expiry. + // The base_ts parameter is accepted for API consistency but not used. /// Check if this entry is expired at the given time (unix millis). - /// `base_ts` is accepted for API consistency but not used (TTL stored as absolute seconds). #[inline] pub fn is_expired_at(&self, _base_ts: u32, now_ms: u64) -> bool { - if self.ttl_delta == 0 { + if self.ttl_secs == 0 { return false; } - let abs_ms = (self.ttl_delta as u64) * 1000; - now_ms >= abs_ms + // saturating_mul: adversarial EXPIREAT near u64::MAX/1000 must clamp + // to "far future", never wrap to the past (debug builds would panic, + // release builds would silently expire the key immediately). + now_ms >= self.ttl_secs.saturating_mul(1000) } /// Check if this entry has an expiry set. #[inline] pub fn has_expiry(&self) -> bool { - self.ttl_delta != 0 + self.ttl_secs != 0 } /// Get the absolute expiry time in milliseconds (for serialization/TTL commands). - /// `base_ts` is accepted for API consistency but not used (TTL stored as absolute seconds). #[inline] pub fn expires_at_ms(&self, _base_ts: u32) -> u64 { - if self.ttl_delta == 0 { - 0 - } else { - (self.ttl_delta as u64) * 1000 - } + self.ttl_secs.saturating_mul(1000) } /// Set the expiry from absolute milliseconds. Pass 0 to remove expiry. - /// `base_ts` is accepted for API consistency but not used (TTL stored as absolute seconds). #[inline] pub fn set_expires_at_ms(&mut self, _base_ts: u32, ms: u64) { if ms == 0 { - self.ttl_delta = 0; + self.ttl_secs = 0; } else { - // Store as absolute seconds (u32 is good until year 2106) - let abs_secs = ms / 1000; - // Ensure non-zero delta for non-zero input - self.ttl_delta = (abs_secs.max(1)).min(u32::MAX as u64) as u32; + // Store as absolute seconds; ensure non-zero for non-zero input. + self.ttl_secs = (ms / 1000).max(1); } } @@ -465,8 +467,9 @@ impl CompactEntry { pub fn new_string(value: Bytes) -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::String(value)), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -474,8 +477,9 @@ impl CompactEntry { pub fn new_string_with_expiry(value: Bytes, expires_at_ms: u64, base_ts: u32) -> CompactEntry { let mut entry = CompactEntry { value: CompactValue::from_redis_value(RedisValue::String(value)), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, }; entry.set_expires_at_ms(base_ts, expires_at_ms); entry @@ -485,8 +489,9 @@ impl CompactEntry { pub fn new_hash() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::Hash(HashMap::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -494,8 +499,9 @@ impl CompactEntry { pub fn new_list() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::List(VecDeque::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -503,8 +509,9 @@ impl CompactEntry { pub fn new_set() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::Set(HashSet::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -515,8 +522,9 @@ impl CompactEntry { members: HashMap::new(), scores: BTreeMap::new(), }), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -524,8 +532,9 @@ impl CompactEntry { pub fn new_hash_listpack() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::HashListpack(Listpack::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -533,8 +542,9 @@ impl CompactEntry { pub fn new_list_listpack() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::ListListpack(Listpack::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -542,8 +552,9 @@ impl CompactEntry { pub fn new_set_listpack() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::SetListpack(Listpack::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -551,8 +562,9 @@ impl CompactEntry { pub fn new_set_intset() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::SetIntset(Intset::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -563,8 +575,9 @@ impl CompactEntry { tree: BPTree::new(), members: HashMap::new(), }), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -572,8 +585,9 @@ impl CompactEntry { pub fn new_sorted_set_listpack() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::SortedSetListpack(Listpack::new())), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -581,8 +595,9 @@ impl CompactEntry { pub fn new_stream() -> CompactEntry { CompactEntry { value: CompactValue::from_redis_value(RedisValue::Stream(Box::new(StreamData::new()))), - ttl_delta: 0, + ttl_secs: 0, metadata: pack_metadata_u32(current_secs() as u16, 0, LFU_INIT_VAL), + _pad: 0, } } @@ -607,7 +622,7 @@ mod tests { fn test_new_string_no_expiry() { let entry = Entry::new_string(Bytes::from_static(b"hello")); assert!(!entry.has_expiry()); - assert_eq!(entry.ttl_delta, 0); + assert_eq!(entry.ttl_secs, 0); assert_eq!(entry.value.type_name(), "string"); assert_eq!(entry.version(), 0); assert_eq!(entry.access_counter(), LFU_INIT_VAL); @@ -619,7 +634,7 @@ mod tests { let exp_ms = current_time_ms() + 60_000; let entry = Entry::new_string_with_expiry(Bytes::from_static(b"hello"), exp_ms, base_ts); assert!(entry.has_expiry()); - assert!(entry.ttl_delta > 0); + assert!(entry.ttl_secs > 0); // Verify round-trip: expires_at_ms should be approximately exp_ms let recovered_ms = entry.expires_at_ms(base_ts); // Allow 1 second tolerance due to integer division @@ -776,7 +791,7 @@ mod tests { #[test] fn test_compact_entry_size() { - assert_eq!(std::mem::size_of::(), 24); + assert_eq!(std::mem::size_of::(), 32); } #[test] diff --git a/src/transaction/abort.rs b/src/transaction/abort.rs index f709e7c1..e78cb0a5 100644 --- a/src/transaction/abort.rs +++ b/src/transaction/abort.rs @@ -123,190 +123,37 @@ pub fn abort_cross_store_txn( } // ------------------------------------------------------------------ - // 2a. Phase 174 FIX-01: Graph undo — reverse SET/DELETE/MERGE mutations - // in LIFO order BEFORE removing created entities (section 2b). This - // ensures property restores on existing nodes happen before any - // newly-created nodes are removed. + // 2. Graph rollback — undo ops (2a) then create-intent removal (2b), + // both in LIFO order, applied via the shared `apply_graph_rollback` + // helper (also used by the ShardMessage::GraphRollback handler for + // the multi-shard TXN.ABORT legs). Slice-aware: once ShardSlice is + // initialized the graph store is thread-local and the lock-based + // accessor would hit a different store. // ------------------------------------------------------------------ #[cfg(feature = "graph")] { - if !txn.graph_undo.is_empty() { - let mut gs = shard_databases.graph_store_write(shard_id); - for undo_op in txn.graph_undo.iter().rev() { - match undo_op { - crate::transaction::GraphUndoOp::RestoreProperty { - graph_name, - entity_id, - is_node, - prop_key, - old_value, - } => { - let Some(graph) = gs.get_graph_mut(graph_name) else { - tracing::warn!( - txn_id, - graph_name = ?graph_name, - "txn abort: graph missing for RestoreProperty undo, skipping", - ); - continue; - }; - if *is_node { - let nk = NodeKey::from(slotmap::KeyData::from_ffi(*entity_id)); - if let Some(node) = graph.write_buf.get_node_mut(nk) { - match old_value { - Some(val) => { - // Restore old value. - let mut found = false; - for entry in node.properties.iter_mut() { - if entry.0 == *prop_key { - entry.1 = val.clone(); - found = true; - break; - } - } - if !found { - node.properties.push((*prop_key, val.clone())); - } - } - None => { - // Property did not exist before SET — remove it. - node.properties.retain(|(k, _)| *k != *prop_key); - } - } - } - } else { - let ek = EdgeKey::from(slotmap::KeyData::from_ffi(*entity_id)); - if let Some(edge) = graph.write_buf.get_edge_mut(ek) { - if let Some(ref mut props) = edge.properties { - match old_value { - Some(val) => { - let mut found = false; - for entry in props.iter_mut() { - if entry.0 == *prop_key { - entry.1 = val.clone(); - found = true; - break; - } - } - if !found { - props.push((*prop_key, val.clone())); - } - } - None => { - props.retain(|(k, _)| *k != *prop_key); - } - } - } - } - } - } - crate::transaction::GraphUndoOp::UndeleteNode { - graph_name, - node_id, - delete_lsn, - } => { - let Some(graph) = gs.get_graph_mut(graph_name) else { - tracing::warn!( - txn_id, - graph_name = ?graph_name, - "txn abort: graph missing for UndeleteNode undo, skipping", - ); - continue; - }; - let nk = NodeKey::from(slotmap::KeyData::from_ffi(*node_id)); - if let Some(node) = graph.write_buf.get_node_mut(nk) { - if node.deleted_lsn == *delete_lsn { - node.deleted_lsn = u64::MAX; - graph.write_buf.inc_live_node_count(); - } - } - // Un-soft-delete incident edges that were cascade-deleted - // at the same LSN by remove_node. - graph.write_buf.undelete_edges_at_lsn(nk, *delete_lsn); - } - crate::transaction::GraphUndoOp::UndeleteEdge { - graph_name, - edge_id, - } => { - let Some(graph) = gs.get_graph_mut(graph_name) else { - tracing::warn!( - txn_id, - graph_name = ?graph_name, - "txn abort: graph missing for UndeleteEdge undo, skipping", - ); - continue; - }; - let ek = EdgeKey::from(slotmap::KeyData::from_ffi(*edge_id)); - if let Some(edge) = graph.write_buf.get_edge_mut(ek) { - if edge.deleted_lsn != u64::MAX { - edge.deleted_lsn = u64::MAX; - graph.write_buf.inc_live_edge_count(); - } - } - } - } - } - // gs guard drops — release graph_store_write before section 2b. - } - } - - // ------------------------------------------------------------------ - // 2b. Graph rollback — iterate intents in REVERSE (LIFO) so edges are - // removed before their endpoint nodes. Gated on the `graph` - // feature: the `graph_intents` field exists unconditionally on - // `CrossStoreTxn`, but the MemGraph types and GraphStore accessor - // only compile with `--features graph`. - // ------------------------------------------------------------------ - #[cfg(feature = "graph")] - { - if !txn.graph_intents.is_empty() { - let mut gs = shard_databases.graph_store_write(shard_id); - // Rollback LSN: allocate from GraphStore's own LSN stream so - // replay order stays monotonic with forward writes (Phase 158 - // CSR v2 migration tests verify this ordering). - let rollback_lsn = gs.allocate_lsn(); - for intent in txn.graph_intents.iter().rev() { - let Some(graph) = gs.get_graph_mut(&intent.graph_name) else { - // Graph was dropped mid-txn (or never created) — silent - // skip per research § "Graph name changes mid-txn". - tracing::warn!( + if !txn.graph_undo.is_empty() || !txn.graph_intents.is_empty() { + let wal_records = if crate::shard::slice::is_initialized() { + crate::shard::slice::with_shard(|s| { + apply_graph_rollback( + &mut s.graph_store, txn_id, - graph_name = ?intent.graph_name, - entity_id = intent.entity_id, - is_node = intent.is_node, - "txn abort: graph missing at rollback time, skipping intent", - ); - continue; - }; - if intent.is_node { - let nk = NodeKey::from(slotmap::KeyData::from_ffi(intent.entity_id)); - let removed = graph.write_buf.remove_node(nk, rollback_lsn); - if !removed { - tracing::warn!( - txn_id, - entity_id = intent.entity_id, - "txn abort: remove_node returned false (already deleted or invalid key)", - ); - } - } else { - let ek = EdgeKey::from(slotmap::KeyData::from_ffi(intent.entity_id)); - let removed = graph.write_buf.remove_edge(ek, rollback_lsn); - if !removed { - tracing::warn!( - txn_id, - entity_id = intent.entity_id, - "txn abort: remove_edge returned false (already deleted or invalid key)", - ); - } - } - } + &txn.graph_undo, + &txn.graph_intents, + ) + }) + } else { + let mut gs = shard_databases.graph_store_write(shard_id); + apply_graph_rollback(&mut gs, txn_id, &txn.graph_undo, &txn.graph_intents) + // gs guard drops at scope end — releases graph_store_write + // before the next layer (vector_store). + }; // Drain any WAL records produced by remove_node/remove_edge so // replay observes the rollback in the same ordering the live // write path would have produced. - for record in gs.drain_wal() { + for record in wal_records { shard_databases.wal_append(shard_id, Bytes::from(record)); } - // gs guard drops at scope end — releases graph_store_write - // before the next layer (vector_store). } } @@ -370,3 +217,293 @@ pub fn abort_cross_store_txn( shard_databases.kv_intents(shard_id).release_txn(txn_id); shard_databases.hnsw_queue(shard_id).discard_for_txn(txn_id); } + +/// Apply the graph half of a TXN.ABORT to one `GraphStore`. +/// +/// Section 2a (undo ops, LIFO) then section 2b (create-intent removal, LIFO +/// so edges go before their endpoint nodes). Returns the drained graph WAL +/// records — the CALLER appends them on its own shard, which keeps the +/// helper usable from both `abort_cross_store_txn` (connection-local leg) +/// and the `ShardMessage::GraphRollback` handler (multi-shard legs). +#[cfg(feature = "graph")] +pub fn apply_graph_rollback( + gs: &mut crate::graph::store::GraphStore, + txn_id: u64, + graph_undo: &[crate::transaction::GraphUndoOp], + graph_intents: &[crate::transaction::GraphIntent], +) -> Vec> { + // 2a. Phase 174 FIX-01: reverse SET/DELETE/MERGE mutations in LIFO order + // BEFORE removing created entities (2b). This ensures property + // restores on existing nodes happen before any newly-created nodes + // are removed. + for undo_op in graph_undo.iter().rev() { + match undo_op { + crate::transaction::GraphUndoOp::RestoreProperty { + graph_name, + entity_id, + is_node, + prop_key, + old_value, + } => { + let Some(graph) = gs.get_graph_mut(graph_name) else { + tracing::warn!( + txn_id, + graph_name = ?graph_name, + "txn abort: graph missing for RestoreProperty undo, skipping", + ); + continue; + }; + if *is_node { + let nk = NodeKey::from(slotmap::KeyData::from_ffi(*entity_id)); + if let Some(node) = graph.write_buf.get_node_mut(nk) { + match old_value { + Some(val) => { + // Restore old value. + let mut found = false; + for entry in node.properties.iter_mut() { + if entry.0 == *prop_key { + entry.1 = val.clone(); + found = true; + break; + } + } + if !found { + node.properties.push((*prop_key, val.clone())); + } + } + None => { + // Property did not exist before SET — remove it. + node.properties.retain(|(k, _)| *k != *prop_key); + } + } + } + } else { + let ek = EdgeKey::from(slotmap::KeyData::from_ffi(*entity_id)); + if let Some(edge) = graph.write_buf.get_edge_mut(ek) { + if let Some(ref mut props) = edge.properties { + match old_value { + Some(val) => { + let mut found = false; + for entry in props.iter_mut() { + if entry.0 == *prop_key { + entry.1 = val.clone(); + found = true; + break; + } + } + if !found { + props.push((*prop_key, val.clone())); + } + } + None => { + props.retain(|(k, _)| *k != *prop_key); + } + } + } + } + } + } + crate::transaction::GraphUndoOp::UndeleteNode { + graph_name, + node_id, + delete_lsn, + } => { + let Some(graph) = gs.get_graph_mut(graph_name) else { + tracing::warn!( + txn_id, + graph_name = ?graph_name, + "txn abort: graph missing for UndeleteNode undo, skipping", + ); + continue; + }; + let nk = NodeKey::from(slotmap::KeyData::from_ffi(*node_id)); + if let Some(node) = graph.write_buf.get_node_mut(nk) { + if node.deleted_lsn == *delete_lsn { + node.deleted_lsn = u64::MAX; + graph.write_buf.inc_live_node_count(); + } + } + // Un-soft-delete incident edges that were cascade-deleted + // at the same LSN by remove_node. + graph.write_buf.undelete_edges_at_lsn(nk, *delete_lsn); + } + crate::transaction::GraphUndoOp::UndeleteEdge { + graph_name, + edge_id, + } => { + let Some(graph) = gs.get_graph_mut(graph_name) else { + tracing::warn!( + txn_id, + graph_name = ?graph_name, + "txn abort: graph missing for UndeleteEdge undo, skipping", + ); + continue; + }; + let ek = EdgeKey::from(slotmap::KeyData::from_ffi(*edge_id)); + if let Some(edge) = graph.write_buf.get_edge_mut(ek) { + if edge.deleted_lsn != u64::MAX { + edge.deleted_lsn = u64::MAX; + graph.write_buf.inc_live_edge_count(); + } + } + } + } + } + + // 2b. Create-intent removal — iterate in REVERSE (LIFO) so edges are + // removed before their endpoint nodes. + if !graph_intents.is_empty() { + // Rollback LSN: allocate from GraphStore's own LSN stream so + // replay order stays monotonic with forward writes (Phase 158 + // CSR v2 migration tests verify this ordering). + let rollback_lsn = gs.allocate_lsn(); + for intent in graph_intents.iter().rev() { + let Some(graph) = gs.get_graph_mut(&intent.graph_name) else { + // Graph was dropped mid-txn (or never created) — silent + // skip per research § "Graph name changes mid-txn". + tracing::warn!( + txn_id, + graph_name = ?intent.graph_name, + entity_id = intent.entity_id, + is_node = intent.is_node, + "txn abort: graph missing at rollback time, skipping intent", + ); + continue; + }; + if intent.is_node { + let nk = NodeKey::from(slotmap::KeyData::from_ffi(intent.entity_id)); + let removed = graph.write_buf.remove_node(nk, rollback_lsn); + if !removed { + tracing::warn!( + txn_id, + entity_id = intent.entity_id, + "txn abort: remove_node returned false (already deleted or invalid key)", + ); + } + } else { + let ek = EdgeKey::from(slotmap::KeyData::from_ffi(intent.entity_id)); + let removed = graph.write_buf.remove_edge(ek, rollback_lsn); + if !removed { + tracing::warn!( + txn_id, + entity_id = intent.entity_id, + "txn abort: remove_edge returned false (already deleted or invalid key)", + ); + } + } + } + } + + gs.drain_wal() +} + +/// Multi-shard-aware TXN.ABORT. +/// +/// Graphs live on the shard that owns their NAME (`graph_to_shard`), so the +/// graph half of the rollback must run where the entities actually are. +/// This wrapper partitions `txn.graph_undo` / `txn.graph_intents` by owning +/// shard, runs the full local abort (KV undo, local graph ops, vector +/// tombstones, side tables) via `abort_cross_store_txn`, then ships each +/// remote group to its owner via `ShardMessage::GraphRollback` and awaits +/// the acknowledgements. +/// +/// Failure handling: a closed reply channel (owner shard gone) is logged and +/// skipped — abort is already best-effort per-step (see module docs), and +/// the per-shard side tables treat missing entries as no-ops. +/// +/// At `num_shards <= 1` this is exactly `abort_cross_store_txn`. +#[allow(clippy::too_many_arguments)] +pub async fn abort_cross_store_txn_routed( + shard_databases: &ShardDatabases, + shard_id: usize, + selected_db: usize, + num_shards: usize, + dispatch_tx: &std::rc::Rc< + std::cell::RefCell>>, + >, + spsc_notifiers: &[std::sync::Arc], + #[allow(unused_mut)] mut txn: CrossStoreTxn, +) { + // Partition the graph ops by owning shard BEFORE the local abort + // consumes the transaction. Plain data shuffling — no locks held. + #[cfg(feature = "graph")] + let remote: Vec<( + usize, + Vec, + Vec, + )> = if num_shards > 1 && (!txn.graph_undo.is_empty() || !txn.graph_intents.is_empty()) { + use crate::shard::dispatch::graph_to_shard; + let mut by_owner: std::collections::BTreeMap< + usize, + ( + Vec, + Vec, + ), + > = std::collections::BTreeMap::new(); + let mut local_undo = Vec::with_capacity(txn.graph_undo.len()); + for op in txn.graph_undo.drain(..) { + let owner = { + let name = match &op { + crate::transaction::GraphUndoOp::RestoreProperty { graph_name, .. } + | crate::transaction::GraphUndoOp::UndeleteNode { graph_name, .. } + | crate::transaction::GraphUndoOp::UndeleteEdge { graph_name, .. } => { + graph_name + } + }; + graph_to_shard(name, num_shards) + }; + if owner == shard_id { + local_undo.push(op); + } else { + by_owner.entry(owner).or_default().0.push(op); + } + } + txn.graph_undo = local_undo; + let mut local_intents: smallvec::SmallVec<[crate::transaction::GraphIntent; 8]> = + smallvec::SmallVec::new(); + for intent in txn.graph_intents.drain(..) { + let owner = graph_to_shard(&intent.graph_name, num_shards); + if owner == shard_id { + local_intents.push(intent); + } else { + by_owner.entry(owner).or_default().1.push(intent); + } + } + txn.graph_intents = local_intents; + by_owner + .into_iter() + .map(|(owner, (undo, intents))| (owner, undo, intents)) + .collect() + } else { + Vec::new() + }; + #[cfg(not(feature = "graph"))] + let _ = num_shards; + + let txn_id = txn.txn_id; + abort_cross_store_txn(shard_databases, shard_id, selected_db, txn); + + #[cfg(feature = "graph")] + for (owner, graph_undo, graph_intents) in remote { + let (reply_tx, reply_rx) = crate::runtime::channel::oneshot(); + let msg = crate::shard::dispatch::ShardMessage::GraphRollback(Box::new( + crate::shard::dispatch::GraphRollbackPayload { + txn_id, + graph_undo, + graph_intents, + reply_tx, + }, + )); + crate::shard::coordinator::spsc_send(dispatch_tx, shard_id, owner, msg, spsc_notifiers) + .await; + if reply_rx.recv().await.is_err() { + tracing::warn!( + txn_id, + owner, + "txn abort: remote graph rollback reply channel closed" + ); + } + } + #[cfg(not(feature = "graph"))] + let _ = (txn_id, dispatch_tx, spsc_notifiers); +} diff --git a/tests/cross_shard_consistency_red.rs b/tests/cross_shard_consistency_red.rs new file mode 100644 index 00000000..7f2d0d71 --- /dev/null +++ b/tests/cross_shard_consistency_red.rs @@ -0,0 +1,697 @@ +//! ADD task `consistency-dispatch-gaps` — contract v3 failing-first suite. +//! +//! RED tests (fail until the cross-shard COPY/BITOP coordinators and the +//! GRAPH.* owner-shard routing land — frozen contract §3 v3): +//! - cdg6a_bitop_cross_shard — BITOP AND/OR/XOR/NOT with sources and dest +//! PROVABLY on different shards (keys picked via moon's own key_to_shard) +//! computes the Redis-exact result, readable from dest's owning shard. +//! On main: BITOP routes by args[0] (the literal "AND"), reads sources on +//! an arbitrary shard, writes dest there too — Int(0) and a missing dest. +//! - cdg6b_copy_cross_shard — COPY src dst across shards copies the string +//! value + TTL, honours no-REPLACE (Int(0), dst untouched) and REPLACE; +//! co-located non-string COPY (hash tags) keeps full-type fidelity. +//! On main: routed by src, dst written into src's shard — GET dst = nil. +//! - cdg6c_graph_routing_connection_independent — every one of N fresh +//! connections can GRAPH.ADDNODE into one graph, and a final fresh +//! connection sees all N nodes. On main the graph store is per-shard and +//! commands execute on the CONNECTION's shard: whichever connections the +//! kernel lands elsewhere get "ERR graph not found" (red on Linux +//! SO_REUSEPORT spread; macOS may pin all conns to one shard and pass +//! vacuously — red state for this case is established on the Linux VM). +//! +//! Run alone with: cargo test --test cross_shard_consistency_red + +use std::io::{Read, Write}; +use std::net::{TcpStream, ToSocketAddrs}; +use std::process::{Child, Command}; +use std::time::{Duration, Instant}; + +use moon::shard::dispatch::key_to_shard; + +// --------------------------------------------------------------------------- +// Harness (CARGO_BIN_EXE pattern, mirrors wire_reachability_red.rs) +// --------------------------------------------------------------------------- + +fn moon_binary() -> std::path::PathBuf { + std::path::PathBuf::from(env!("CARGO_BIN_EXE_moon")) +} + +fn free_port() -> u16 { + let l = std::net::TcpListener::bind("127.0.0.1:0").expect("bind :0"); + let p = l.local_addr().expect("local_addr").port(); + drop(l); + p +} + +fn spawn_moon(port: u16, dir: &std::path::Path, shards: u32) -> Child { + Command::new(moon_binary()) + .args([ + "--port", + &port.to_string(), + "--dir", + &dir.to_string_lossy(), + "--shards", + &shards.to_string(), + ]) + .stdout(std::fs::File::create(dir.join("moon.stdout.log")).expect("stdout log")) + .stderr(std::fs::File::create(dir.join("moon.stderr.log")).expect("stderr log")) + .spawn() + .expect("spawn moon (CARGO_BIN_EXE_moon)") +} + +struct ServerGuard(Child); +impl Drop for ServerGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +fn connect(port: u16, deadline: Duration) -> TcpStream { + let addr = format!("127.0.0.1:{port}") + .to_socket_addrs() + .expect("addr") + .next() + .expect("one addr"); + let start = Instant::now(); + loop { + match TcpStream::connect_timeout(&addr, Duration::from_millis(200)) { + Ok(s) => { + s.set_read_timeout(Some(Duration::from_secs(5))).ok(); + s.set_write_timeout(Some(Duration::from_secs(5))).ok(); + return s; + } + Err(_) if start.elapsed() < deadline => { + std::thread::sleep(Duration::from_millis(50)); + } + Err(e) => panic!("server never accepted on {port}: {e}"), + } + } +} + +fn wait_ready(port: u16) -> TcpStream { + let mut s = connect(port, Duration::from_secs(30)); + let start = Instant::now(); + loop { + s.write_all(b"PING\r\n").expect("write PING"); + let mut buf = [0u8; 64]; + if let Ok(n) = s.read(&mut buf) { + if n > 0 && buf[..n].windows(4).any(|w| w == b"PONG") { + return s; + } + } + assert!( + start.elapsed() < Duration::from_secs(10), + "server accepted TCP but never answered PING" + ); + std::thread::sleep(Duration::from_millis(100)); + s = connect(port, Duration::from_secs(5)); + } +} + +// --------------------------------------------------------------------------- +// Minimal RESP2 reader (same shape as wire_reachability_red.rs) +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, PartialEq)] +enum Resp { + Simple(String), + Error(String), + Int(i64), + Bulk(Option>), + Array(Option>), +} + +impl Resp { + fn flat(&self) -> String { + match self { + Resp::Simple(s) | Resp::Error(s) => s.clone(), + Resp::Int(i) => i.to_string(), + Resp::Bulk(Some(b)) => String::from_utf8_lossy(b).into_owned(), + Resp::Bulk(None) => "".into(), + Resp::Array(Some(items)) => items.iter().map(Resp::flat).collect::>().join(" "), + Resp::Array(None) => "".into(), + } + } +} + +struct Conn { + s: TcpStream, + buf: Vec, + pos: usize, +} + +impl Conn { + fn new(s: TcpStream) -> Self { + Conn { + s, + buf: Vec::with_capacity(16 * 1024), + pos: 0, + } + } + + fn open(port: u16) -> Self { + Conn::new(connect(port, Duration::from_secs(10))) + } + + fn cmd(&mut self, parts: &[&[u8]]) -> Resp { + let mut req = Vec::with_capacity(64); + req.extend_from_slice(format!("*{}\r\n", parts.len()).as_bytes()); + for p in parts { + req.extend_from_slice(format!("${}\r\n", p.len()).as_bytes()); + req.extend_from_slice(p); + req.extend_from_slice(b"\r\n"); + } + self.s.write_all(&req).expect("write cmd"); + self.frame() + } + + fn cmd_s(&mut self, parts: &[&str]) -> Resp { + let v: Vec<&[u8]> = parts.iter().map(|p| p.as_bytes()).collect(); + self.cmd(&v) + } + + fn fill(&mut self) { + let mut chunk = [0u8; 16 * 1024]; + let n = self.s.read(&mut chunk).expect("read"); + assert!(n > 0, "connection closed mid-frame"); + self.buf.extend_from_slice(&chunk[..n]); + } + + fn line(&mut self) -> String { + loop { + if let Some(rel) = self.buf[self.pos..].windows(2).position(|w| w == b"\r\n") { + let line = + String::from_utf8_lossy(&self.buf[self.pos..self.pos + rel]).into_owned(); + self.pos += rel + 2; + return line; + } + self.fill(); + } + } + + fn exact(&mut self, n: usize) -> Vec { + while self.buf.len() - self.pos < n + 2 { + self.fill(); + } + let out = self.buf[self.pos..self.pos + n].to_vec(); + self.pos += n + 2; + out + } + + fn frame(&mut self) -> Resp { + if self.pos > 0 && self.pos == self.buf.len() { + self.buf.clear(); + self.pos = 0; + } + let line = self.line(); + let (tag, rest) = line.split_at(1); + match tag { + "+" => Resp::Simple(rest.to_string()), + "-" => Resp::Error(rest.to_string()), + ":" => Resp::Int(rest.parse().unwrap_or(0)), + "$" => { + let n: i64 = rest.parse().unwrap_or(-1); + if n < 0 { + Resp::Bulk(None) + } else { + Resp::Bulk(Some(self.exact(n as usize))) + } + } + "*" => { + let n: i64 = rest.parse().unwrap_or(-1); + if n < 0 { + Resp::Array(None) + } else { + let mut items = Vec::with_capacity(n as usize); + for _ in 0..n { + items.push(self.frame()); + } + Resp::Array(Some(items)) + } + } + other => panic!("unexpected RESP tag {other:?} (line {line:?})"), + } + } +} + +// --------------------------------------------------------------------------- +// Key picking: provably-cross-shard names via moon's own hash +// --------------------------------------------------------------------------- + +const SHARDS: u32 = 4; + +/// Return one key per shard (index = shard id), generated deterministically. +fn keys_per_shard(prefix: &str, num_shards: usize) -> Vec { + let mut out: Vec> = vec![None; num_shards]; + let mut found = 0; + for i in 0..10_000 { + let k = format!("{prefix}{i}"); + let s = key_to_shard(k.as_bytes(), num_shards); + if out[s].is_none() { + out[s] = Some(k); + found += 1; + if found == num_shards { + break; + } + } + } + out.into_iter() + .map(|o| o.expect("found a key for every shard")) + .collect() +} + +// --------------------------------------------------------------------------- +// cdg6a — BITOP across shards +// --------------------------------------------------------------------------- + +#[test] +fn cdg6a_bitop_cross_shard() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let ks = keys_per_shard("cdg6a:", SHARDS as usize); + // sources on shards 0 and 1, dest on shard 2 — all distinct by construction + let (s1, s2, dest) = (&ks[0], &ks[1], &ks[2]); + + let mut c = Conn::open(port); + assert_eq!(c.cmd_s(&["SET", s1, "abc"]), Resp::Simple("OK".into())); + assert_eq!(c.cmd_s(&["SET", s2, "abd"]), Resp::Simple("OK".into())); + + // AND: byte-wise; equal length, no padding involved + let r = c.cmd_s(&["BITOP", "AND", dest, s1, s2]); + assert_eq!(r, Resp::Int(3), "BITOP AND result length (got {r:?})"); + let v = c.cmd_s(&["GET", dest]); + assert_eq!( + v, + Resp::Bulk(Some(b"ab`".to_vec())), + "AND of abc/abd (c&d = `)" + ); + + // OR + assert_eq!(c.cmd_s(&["BITOP", "OR", dest, s1, s2]), Resp::Int(3)); + assert_eq!(c.cmd_s(&["GET", dest]), Resp::Bulk(Some(b"abg".to_vec()))); + + // XOR + assert_eq!(c.cmd_s(&["BITOP", "XOR", dest, s1, s2]), Resp::Int(3)); + assert_eq!( + c.cmd_s(&["GET", dest]), + Resp::Bulk(Some(vec![0x00, 0x00, 0x07])) + ); + + // NOT (single source on a different shard than dest) + assert_eq!(c.cmd_s(&["BITOP", "NOT", dest, s1]), Resp::Int(3)); + assert_eq!( + c.cmd_s(&["GET", dest]), + Resp::Bulk(Some(vec![!b'a', !b'b', !b'c'])) + ); + + // NOT arity error preserved + let e = c.cmd_s(&["BITOP", "NOT", dest, s1, s2]); + assert!( + matches!(&e, Resp::Error(m) if m.contains("NOT")), + "BITOP NOT arity error (got {e:?})" + ); + + // Unequal lengths: shorter source zero-padded to the longest + assert_eq!(c.cmd_s(&["SET", s2, "abcdef"]), Resp::Simple("OK".into())); + assert_eq!(c.cmd_s(&["BITOP", "OR", dest, s1, s2]), Resp::Int(6)); + assert_eq!( + c.cmd_s(&["GET", dest]), + Resp::Bulk(Some(b"abcdef".to_vec())) + ); + + // All sources missing: dest deleted, Int(0) + let m1 = &ks[3]; // shard 3, never written + assert_eq!( + c.cmd_s(&["BITOP", "AND", dest, m1, "cdg6a:missing:zz"]), + Resp::Int(0) + ); + assert_eq!(c.cmd_s(&["GET", dest]), Resp::Bulk(None), "dest deleted"); + + // WRONGTYPE propagates from a cross-shard source + assert_eq!(c.cmd_s(&["RPUSH", m1, "x"]), Resp::Int(1)); + let e = c.cmd_s(&["BITOP", "AND", dest, s1, m1]); + assert!( + matches!(&e, Resp::Error(m) if m.starts_with("WRONGTYPE")), + "wrongtype source (got {e:?})" + ); +} + +// --------------------------------------------------------------------------- +// cdg6b — COPY across shards +// --------------------------------------------------------------------------- + +#[test] +fn cdg6b_copy_cross_shard() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let ks = keys_per_shard("cdg6b:", SHARDS as usize); + let (src, dst, dst2) = (&ks[0], &ks[1], &ks[2]); + + let mut c = Conn::open(port); + assert_eq!( + c.cmd_s(&["SET", src, "copy_value"]), + Resp::Simple("OK".into()) + ); + assert_eq!(c.cmd_s(&["PEXPIRE", src, "500000"]), Resp::Int(1)); + + // Plain cross-shard COPY: value lands on dst's shard, TTL preserved + let r = c.cmd_s(&["COPY", src, dst]); + assert_eq!(r, Resp::Int(1), "COPY src dst (got {r:?})"); + assert_eq!( + c.cmd_s(&["GET", dst]), + Resp::Bulk(Some(b"copy_value".to_vec())), + "dst readable on its own shard" + ); + match c.cmd_s(&["PTTL", dst]) { + Resp::Int(ttl) => assert!(ttl > 0 && ttl <= 500_000, "COPY preserves TTL (PTTL={ttl})"), + other => panic!("PTTL reply {other:?}"), + } + + // Existing destination without REPLACE → Int(0), dst untouched + assert_eq!(c.cmd_s(&["SET", dst2, "old"]), Resp::Simple("OK".into())); + assert_eq!(c.cmd_s(&["COPY", src, dst2]), Resp::Int(0)); + assert_eq!(c.cmd_s(&["GET", dst2]), Resp::Bulk(Some(b"old".to_vec()))); + + // REPLACE overwrites + assert_eq!(c.cmd_s(&["COPY", src, dst2, "REPLACE"]), Resp::Int(1)); + assert_eq!( + c.cmd_s(&["GET", dst2]), + Resp::Bulk(Some(b"copy_value".to_vec())) + ); + + // Missing source → Int(0) + assert_eq!(c.cmd_s(&["COPY", "cdg6b:nope", dst2]), Resp::Int(0)); + + // Co-located (hash-tagged) non-string COPY keeps full fidelity + assert_eq!(c.cmd_s(&["RPUSH", "{cdg6b}list", "a", "b"]), Resp::Int(2)); + assert_eq!( + c.cmd_s(&["COPY", "{cdg6b}list", "{cdg6b}list2"]), + Resp::Int(1) + ); + assert_eq!( + c.cmd_s(&["LRANGE", "{cdg6b}list2", "0", "-1"]).flat(), + "a b" + ); + + // Cross-shard NON-string COPY: explicit error, not silent corruption. + // Find a list key on a different shard than its copy target. + let lsrc = &ks[3]; + assert_eq!(c.cmd_s(&["DEL", lsrc]), Resp::Int(0)); + assert_eq!(c.cmd_s(&["RPUSH", lsrc, "x"]), Resp::Int(1)); + let e = c.cmd_s(&["COPY", lsrc, dst2, "REPLACE"]); + assert!( + matches!(&e, Resp::Error(_)), + "cross-shard non-string COPY must error explicitly (got {e:?})" + ); + // and the destination must be untouched by the failed copy + assert_eq!( + c.cmd_s(&["GET", dst2]), + Resp::Bulk(Some(b"copy_value".to_vec())) + ); +} + +// --------------------------------------------------------------------------- +// cdg6c — GRAPH commands are connection-independent +// --------------------------------------------------------------------------- + +#[test] +fn cdg6c_graph_routing_connection_independent() { + if !cfg!(feature = "graph") { + eprintln!("skip: graph feature compiled out (tokio CI feature set)"); + return; + } + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let mut c0 = Conn::open(port); + assert_eq!( + c0.cmd_s(&["GRAPH.CREATE", "cdg6graph"]), + Resp::Simple("OK".into()) + ); + + // N fresh connections — the kernel spreads them across shard listeners + // (Linux SO_REUSEPORT). Every single one must reach the graph. + const N: usize = 12; + for i in 0..N { + let mut c = Conn::open(port); + let r = c.cmd_s(&["GRAPH.ADDNODE", "cdg6graph", ":CdgLabel"]); + assert!( + matches!(r, Resp::Int(_)), + "conn {i}: GRAPH.ADDNODE must succeed from ANY connection (got {r:?})" + ); + } + + // A final fresh connection sees all N nodes. + let mut c = Conn::open(port); + let info = c.cmd_s(&["GRAPH.INFO", "cdg6graph"]).flat(); + assert!( + info.contains(&N.to_string()), + "GRAPH.INFO must report {N} nodes from any connection (got: {info})" + ); +} + +/// Pull `node_count` out of a GRAPH.INFO reply (RESP2 flattens the map to +/// an array of alternating key/value items). +fn graph_node_count(c: &mut Conn, graph: &str) -> i64 { + let r = c.cmd_s(&["GRAPH.INFO", graph]); + let items = match r { + Resp::Array(Some(items)) => items, + other => panic!("GRAPH.INFO reply {other:?}"), + }; + for pair in items.chunks(2) { + if pair.len() == 2 && pair[0].flat() == "node_count" { + if let Resp::Int(n) = pair[1] { + return n; + } + return pair[1].flat().parse().expect("node_count value"); + } + } + panic!("node_count missing from GRAPH.INFO reply"); +} + +// --------------------------------------------------------------------------- +// cdg6d — TEMPORAL.INVALIDATE is connection-independent +// --------------------------------------------------------------------------- + +#[test] +fn cdg6d_temporal_invalidate_cross_connection() { + if !cfg!(feature = "graph") { + eprintln!("skip: graph feature compiled out (tokio CI feature set)"); + return; + } + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let mut c0 = Conn::open(port); + assert_eq!( + c0.cmd_s(&["GRAPH.CREATE", "cdg6dgraph"]), + Resp::Simple("OK".into()) + ); + let node_id = match c0.cmd_s(&["GRAPH.ADDNODE", "cdg6dgraph", ":CdgLabel"]) { + Resp::Int(id) => id.to_string(), + other => panic!("ADDNODE reply {other:?}"), + }; + + // TEMPORAL.INVALIDATE mutates the graph store, so it must reach the + // graph's owner shard no matter which shard the connection landed on. + // On main it executes on the CONNECTION's shard — non-owner connections + // answer "ERR graph not found". + for i in 0..12 { + let mut c = Conn::open(port); + let r = c.cmd_s(&["TEMPORAL.INVALIDATE", &node_id, "NODE", "cdg6dgraph"]); + assert_eq!( + r, + Resp::Simple("OK".into()), + "conn {i}: TEMPORAL.INVALIDATE must succeed from ANY connection" + ); + } +} + +// --------------------------------------------------------------------------- +// cdg6e — TXN.ABORT rolls back graph writes routed to other shards +// --------------------------------------------------------------------------- + +#[test] +fn cdg6e_txn_abort_rolls_back_routed_graph_write() { + if !cfg!(feature = "graph") { + eprintln!("skip: graph feature compiled out (tokio CI feature set)"); + return; + } + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let mut c0 = Conn::open(port); + assert_eq!( + c0.cmd_s(&["GRAPH.CREATE", "cdg6egraph"]), + Resp::Simple("OK".into()) + ); + assert!(matches!( + c0.cmd_s(&["GRAPH.ADDNODE", "cdg6egraph", ":Base"]), + Resp::Int(_) + )); + let baseline = graph_node_count(&mut c0, "cdg6egraph"); + assert_eq!(baseline, 1, "baseline committed node"); + + // From many fresh connections (the kernel spreads them across shard + // listeners on Linux): add a node inside a TXN, then abort. The routed + // ADDNODE's intent must be captured from the response id and the abort + // must remove the node on the graph's OWNER shard, not the conn's shard. + for i in 0..12 { + let mut c = Conn::open(port); + assert_eq!( + c.cmd_s(&["TXN", "BEGIN"]), + Resp::Simple("OK".into()), + "conn {i}: TXN BEGIN" + ); + let r = c.cmd_s(&["GRAPH.ADDNODE", "cdg6egraph", ":Aborted"]); + assert!( + matches!(r, Resp::Int(_)), + "conn {i}: in-txn ADDNODE must succeed from ANY connection (got {r:?})" + ); + assert_eq!( + c.cmd_s(&["TXN", "ABORT"]), + Resp::Simple("OK".into()), + "conn {i}: TXN ABORT" + ); + } + + // All 12 aborted adds must be gone, from any connection's view. + let mut c = Conn::open(port); + let after = graph_node_count(&mut c, "cdg6egraph"); + assert_eq!( + after, baseline, + "TXN.ABORT must roll back routed graph writes (still-visible nodes leak)" + ); +} + +// --------------------------------------------------------------------------- +// cdg6f — workspaces are connection-independent (contract v4) +// --------------------------------------------------------------------------- + +#[test] +fn cdg6f_workspace_cross_connection() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let mut c0 = Conn::open(port); + let ws_id = match c0.cmd_s(&["WS", "CREATE", "cdg6fws"]) { + Resp::Bulk(Some(id)) => String::from_utf8(id).expect("ws id utf8"), + other => panic!("WS CREATE reply {other:?}"), + }; + + // The registry must be visible from EVERY connection, whichever shard + // the kernel landed it on. On main the registry is per-shard: AUTH from + // a non-creating shard answers "ERR workspace not found". + for i in 0..12 { + let mut c = Conn::open(port); + let r = c.cmd_s(&["WS", "AUTH", &ws_id]); + assert_eq!( + r, + Resp::Simple("OK".into()), + "conn {i}: WS AUTH must succeed from ANY connection" + ); + let list = c.cmd_s(&["WS", "LIST"]).flat(); + assert!( + list.contains("cdg6fws"), + "conn {i}: WS LIST must include the workspace (got: {list})" + ); + } + + // Bound writes are scoped: SET inside the workspace, visible to a bound + // GET on the same connection; invisible to an unbound connection. + let mut bound = Conn::open(port); + assert_eq!( + bound.cmd_s(&["WS", "AUTH", &ws_id]), + Resp::Simple("OK".into()) + ); + assert_eq!( + bound.cmd_s(&["SET", "cdg6f:key", "wsval"]), + Resp::Simple("OK".into()) + ); + assert_eq!( + bound.cmd_s(&["GET", "cdg6f:key"]), + Resp::Bulk(Some(b"wsval".to_vec())) + ); + let mut unbound = Conn::open(port); + assert_eq!( + unbound.cmd_s(&["GET", "cdg6f:key"]), + Resp::Bulk(None), + "workspace keys must be invisible to unbound connections" + ); +} + +// --------------------------------------------------------------------------- +// cdg6g — MQ queues are connection-independent (contract v4) +// --------------------------------------------------------------------------- + +#[test] +fn cdg6g_mq_cross_connection() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), SHARDS)); + drop(wait_ready(port)); + + let mut c0 = Conn::open(port); + assert_eq!( + c0.cmd_s(&["MQ", "CREATE", "cdg6gq", "MAXDELIVERY", "3"]), + Resp::Simple("OK".into()) + ); + + // PUSH from fresh connections (kernel spreads them across shards on + // Linux). On main the stream lives in the CONNECTION's shard db, so a + // non-creating shard answers "queue is not durable / not found". + for i in 0..6 { + let mut c = Conn::open(port); + let r = c.cmd(&[b"MQ", b"PUSH", b"cdg6gq", format!("f{i}").as_bytes(), b"v"]); + assert!( + matches!(&r, Resp::Bulk(Some(_))), + "conn {i}: MQ PUSH must succeed from ANY connection (got {r:?})" + ); + } + + // POP from another fresh connection must see all 6 messages. + let mut c = Conn::open(port); + let popped = c.cmd_s(&["MQ", "POP", "cdg6gq", "COUNT", "6"]).flat(); + for i in 0..6 { + assert!( + popped.contains(&format!("f{i}")), + "MQ POP must return message f{i} pushed from another connection (got: {popped})" + ); + } + + // DLQ routing: MAXDELIVERY 1 → first un-ACKed POP dead-letters the + // message; DLQLEN from yet another connection reports it. + let mut c1 = Conn::open(port); + assert_eq!( + c1.cmd_s(&["MQ", "CREATE", "cdg6gdlq", "MAXDELIVERY", "1"]), + Resp::Simple("OK".into()) + ); + let mut c2 = Conn::open(port); + assert!(matches!( + c2.cmd_s(&["MQ", "PUSH", "cdg6gdlq", "df", "dv"]), + Resp::Bulk(Some(_)) + )); + let mut c3 = Conn::open(port); + let _ = c3.cmd_s(&["MQ", "POP", "cdg6gdlq"]); + let mut c4 = Conn::open(port); + assert_eq!( + c4.cmd_s(&["MQ", "DLQLEN", "cdg6gdlq"]), + Resp::Int(1), + "DLQLEN must report the dead-lettered message from any connection" + ); +} diff --git a/tests/wire_reachability_red.rs b/tests/wire_reachability_red.rs new file mode 100644 index 00000000..1ed9cc3c --- /dev/null +++ b/tests/wire_reachability_red.rs @@ -0,0 +1,757 @@ +//! ADD task `consistency-dispatch-gaps` — failing-first suite. +//! +//! RED tests (fail until the 25 dispatch_read arms + the CDC.READ monoio port +//! land — frozen contract §3 v2): +//! - cdg1_registry_sweep_no_unknowns — every command in the COMMAND_META +//! registry answers something other than "unknown command" over a real +//! connection. On main this fails with 26 violations (XRANGE, XREAD, +//! XREVRANGE, XINFO, XPENDING, ZDIFF, ZINTER, ZUNION, ZINTERCARD, +//! ZRANDMEMBER, GEOPOS, GEODIST, GEOHASH, GEOSEARCH, LCS, RANDOMKEY, +//! EXPIRETIME, PEXPIRETIME, TOUCH, LOLWUT, TIME, WAIT, SLOWLOG, ZMSCORE, +//! XLEN, CDC.READ): the handlers' local read path consults only +//! `dispatch_read`, which lacks their arms (CDC.READ: tokio-handler-only +//! special case, missing on monoio). +//! - cdg2_twenty_commands_answer_correctly — value asserts per command on a +//! 2-shard server, fixtures spread across hash tags {t0}..{t7} so both the +//! local and cross-shard read paths are exercised wherever the kernel +//! lands the connection (macOS REUSEPORT pins; Linux splits). +//! - cdg3_read_arms_are_pure_reads — the new arms never mutate: expired keys +//! are invisible (missing-form replies) and AOF does not grow from reads +//! of live keys. +//! +//! Run alone with: cargo test --test wire_reachability_red + +use std::io::{Read, Write}; +use std::net::{TcpStream, ToSocketAddrs}; +use std::process::{Child, Command}; +use std::time::{Duration, Instant}; + +// --------------------------------------------------------------------------- +// Shared harness (CARGO_BIN_EXE pattern, mirrors spsc_wake_floor_red.rs) +// --------------------------------------------------------------------------- + +fn moon_binary() -> std::path::PathBuf { + std::path::PathBuf::from(env!("CARGO_BIN_EXE_moon")) +} + +fn free_port() -> u16 { + let l = std::net::TcpListener::bind("127.0.0.1:0").expect("bind :0"); + let p = l.local_addr().expect("local_addr").port(); + drop(l); + p +} + +/// Fresh `--dir` per server (CWD persistence-reload trap: an inherited +/// appendonlydir would replay stale state into a throwaway test server). +fn spawn_moon(port: u16, dir: &std::path::Path, shards: u32, extra: &[&str]) -> Child { + let mut args: Vec = vec![ + "--port".into(), + port.to_string(), + "--dir".into(), + dir.to_string_lossy().into_owned(), + "--shards".into(), + shards.to_string(), + ]; + for e in extra { + args.push((*e).into()); + } + Command::new(moon_binary()) + .args(&args) + .stdout(std::fs::File::create(dir.join("moon.stdout.log")).expect("stdout log")) + .stderr(std::fs::File::create(dir.join("moon.stderr.log")).expect("stderr log")) + .spawn() + .expect("spawn moon (CARGO_BIN_EXE_moon)") +} + +struct ServerGuard(Child); +impl Drop for ServerGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +fn connect(port: u16, deadline: Duration) -> TcpStream { + let addr = format!("127.0.0.1:{port}") + .to_socket_addrs() + .expect("addr") + .next() + .expect("one addr"); + let start = Instant::now(); + loop { + match TcpStream::connect_timeout(&addr, Duration::from_millis(200)) { + Ok(s) => { + s.set_read_timeout(Some(Duration::from_secs(5))).ok(); + s.set_write_timeout(Some(Duration::from_secs(5))).ok(); + return s; + } + Err(_) if start.elapsed() < deadline => { + std::thread::sleep(Duration::from_millis(50)); + } + Err(e) => panic!("server never accepted on {port}: {e}"), + } + } +} + +/// Wait for the server to answer PING. Generous connect deadline: the FIRST +/// exec of a freshly-linked binary on macOS can stall >10s in code-signature +/// validation (dyld/amfi). +fn wait_ready(port: u16) -> TcpStream { + let mut s = connect(port, Duration::from_secs(30)); + let start = Instant::now(); + loop { + s.write_all(b"PING\r\n").expect("write PING"); + let mut buf = [0u8; 64]; + if let Ok(n) = s.read(&mut buf) { + if n > 0 && buf[..n].windows(4).any(|w| w == b"PONG") { + return s; + } + } + assert!( + start.elapsed() < Duration::from_secs(10), + "server accepted TCP but never answered PING" + ); + std::thread::sleep(Duration::from_millis(100)); + s = connect(port, Duration::from_secs(5)); + } +} + +// --------------------------------------------------------------------------- +// Minimal RESP2 reader — enough to read one complete reply frame of any +// nesting (the sweep and the value asserts both need real frame boundaries, +// not "read once and hope"). +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, PartialEq)] +enum Resp { + Simple(String), + Error(String), + Int(i64), + /// None = null bulk ($-1) + Bulk(Option>), + /// None = null array (*-1) + Array(Option>), +} + +impl Resp { + fn is_error_containing(&self, needle: &str) -> bool { + matches!(self, Resp::Error(e) if e.to_ascii_lowercase().contains(&needle.to_ascii_lowercase())) + } + fn bulk_str(&self) -> Option { + match self { + Resp::Bulk(Some(b)) => Some(String::from_utf8_lossy(b).into_owned()), + Resp::Simple(s) => Some(s.clone()), + _ => None, + } + } + /// Flatten every bulk/simple leaf into one lossy string (for contains asserts). + fn flat(&self) -> String { + match self { + Resp::Simple(s) | Resp::Error(s) => s.clone(), + Resp::Int(i) => i.to_string(), + Resp::Bulk(Some(b)) => String::from_utf8_lossy(b).into_owned(), + Resp::Bulk(None) => "".into(), + Resp::Array(Some(items)) => items.iter().map(Resp::flat).collect::>().join(" "), + Resp::Array(None) => "".into(), + } + } +} + +struct Conn { + s: TcpStream, + buf: Vec, + pos: usize, +} + +impl Conn { + fn new(s: TcpStream) -> Self { + Conn { + s, + buf: Vec::with_capacity(64 * 1024), + pos: 0, + } + } + + fn cmd(&mut self, parts: &[&[u8]]) -> Resp { + let mut req = Vec::with_capacity(64); + req.extend_from_slice(format!("*{}\r\n", parts.len()).as_bytes()); + for p in parts { + req.extend_from_slice(format!("${}\r\n", p.len()).as_bytes()); + req.extend_from_slice(p); + req.extend_from_slice(b"\r\n"); + } + self.s.write_all(&req).expect("write cmd"); + self.frame() + } + + fn cmd_s(&mut self, parts: &[&str]) -> Resp { + let v: Vec<&[u8]> = parts.iter().map(|p| p.as_bytes()).collect(); + self.cmd(&v) + } + + fn fill(&mut self) { + let mut chunk = [0u8; 16 * 1024]; + let n = self.s.read(&mut chunk).expect("read"); + assert!(n > 0, "connection closed mid-frame"); + self.buf.extend_from_slice(&chunk[..n]); + } + + fn line(&mut self) -> String { + loop { + if let Some(rel) = self.buf[self.pos..].windows(2).position(|w| w == b"\r\n") { + let line = + String::from_utf8_lossy(&self.buf[self.pos..self.pos + rel]).into_owned(); + self.pos += rel + 2; + return line; + } + self.fill(); + } + } + + fn exact(&mut self, n: usize) -> Vec { + while self.buf.len() - self.pos < n + 2 { + self.fill(); + } + let out = self.buf[self.pos..self.pos + n].to_vec(); + self.pos += n + 2; // skip trailing CRLF + out + } + + fn frame(&mut self) -> Resp { + // Compact the buffer between top-level frames so it never grows unbounded. + if self.pos > 0 && self.pos == self.buf.len() { + self.buf.clear(); + self.pos = 0; + } + let line = self.line(); + let (tag, rest) = line.split_at(1); + match tag { + "+" => Resp::Simple(rest.to_string()), + "-" => Resp::Error(rest.to_string()), + ":" => Resp::Int(rest.parse().expect("int frame")), + "$" => { + let len: i64 = rest.parse().expect("bulk len"); + if len < 0 { + Resp::Bulk(None) + } else { + Resp::Bulk(Some(self.exact(len as usize))) + } + } + "*" => { + let len: i64 = rest.parse().expect("array len"); + if len < 0 { + Resp::Array(None) + } else { + let mut items = Vec::with_capacity(len as usize); + for _ in 0..len { + items.push(self.frame()); + } + Resp::Array(Some(items)) + } + } + other => panic!("unexpected RESP tag {other:?} in line {line:?}"), + } + } +} + +// --------------------------------------------------------------------------- +// cdg1 — registry sweep: no implemented command may answer "unknown command" +// --------------------------------------------------------------------------- + +/// M1/M5: send EVERY registry command over the wire. Each command gets a +/// FRESH connection so connection-mode commands (MULTI, SUBSCRIBE, MONITOR, +/// RESET, QUIT) cannot pollute later sends. No arguments are passed — a +/// wrong-arity error is a PASS (the dispatcher recognized the command); +/// only "unknown command" (or a dead connection) is a violation. +#[test] +fn cdg1_registry_sweep_no_unknowns() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), 1, &["--appendonly", "no"])); + drop(wait_ready(port)); + + // Backlogged by user decision (contract v2, 2026-06-11 "Fix 26, backlog the + // 10"): these are advertised in COMMAND_META but implemented NOWHERE — + // verified unknown on both runtimes including the v0.3.0 release binary. + // They are missing FEATURES (WATCH semantics, DUMP/RESTORE serialization, + // sharded pub/sub, latency/module admin), not dispatch-routing bugs, and + // are tracked as an observe-phase delta: implement or deregister. + const BACKLOGGED_UNIMPLEMENTED: &[&str] = &[ + "WATCH", + "UNWATCH", + "RESET", + "SSUBSCRIBE", + "SUNSUBSCRIBE", + "LATENCY", + "MODULE", + "DUMP", + "RESTORE", + "RECLAMATION", + ]; + + let mut violations: Vec = Vec::new(); + for name in moon::command::metadata::COMMAND_META.keys() { + // SHUTDOWN with no args halts the server — the one operational skip. + if *name == "SHUTDOWN" || BACKLOGGED_UNIMPLEMENTED.contains(name) { + continue; + } + // M1 covers commands WITH an arm in dispatch. GRAPH.* arms are + // feature-gated (#[cfg(feature = "graph")]) — when the feature is + // compiled out (the tokio CI feature set), no arm exists and + // "unknown command" is the correct reply. + if !cfg!(feature = "graph") && name.starts_with("GRAPH.") { + continue; + } + // PSYNC is a monoio-handler special case (replication is monoio-only + // today); the tokio handler has no replication support. Runtime + // divergence tracked as an observe-phase delta alongside the 10 + // backlogged commands. + if !cfg!(feature = "runtime-monoio") && *name == "PSYNC" { + continue; + } + let mut c = Conn::new(connect(port, Duration::from_secs(10))); + let reply = + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| c.cmd(&[name.as_bytes()]))); + match reply { + Ok(r) if r.is_error_containing("unknown command") => { + violations.push(format!("{name}: {}", r.flat())); + } + Ok(_) => {} + Err(_) => violations.push(format!("{name}: NO REPLY (read failed/timeout)")), + } + } + assert!( + violations.is_empty(), + "{} registry commands are unreachable over the wire:\n {}", + violations.len(), + violations.join("\n ") + ); + + // Reject pin (unknown_must_stay_unknown): a genuinely unknown command + // still gets the error, from the single source of truth. + let mut c = Conn::new(connect(port, Duration::from_secs(10))); + let r = c.cmd_s(&["FOOBAR"]); + assert!( + r.is_error_containing("unknown command"), + "FOOBAR must remain an unknown command, got: {r:?}" + ); +} + +// --------------------------------------------------------------------------- +// cdg2 — the 20 commands answer CORRECTLY (value asserts), local + cross-shard +// --------------------------------------------------------------------------- + +#[test] +fn cdg2_twenty_commands_answer_correctly() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), 2, &["--appendonly", "no"])); + drop(wait_ready(port)); + let mut c = Conn::new(connect(port, Duration::from_secs(10))); + + // Hash tags {t0}..{t7} spread fixtures across both shards: whichever shard + // this connection landed on, some tags are local and some cross-shard, so + // both read paths are exercised by the same loop. + for t in 0..8 { + let tag = format!("{{t{t}}}"); + + // ---- fixtures (write commands — these work today) ---- + let s_key = format!("s:{tag}"); + assert_eq!( + c.cmd_s(&["XADD", &s_key, "1-1", "f", "v"]) + .bulk_str() + .as_deref(), + Some("1-1"), + "XADD fixture" + ); + assert_eq!( + c.cmd_s(&["XADD", &s_key, "2-2", "f", "w"]) + .bulk_str() + .as_deref(), + Some("2-2"), + "XADD fixture" + ); + let grp = c.cmd_s(&["XGROUP", "CREATE", &s_key, "g", "0"]); + assert_eq!( + grp, + Resp::Simple("OK".into()), + "XGROUP CREATE fixture: {grp:?}" + ); + + let z1 = format!("z1:{tag}"); + let z2 = format!("z2:{tag}"); + c.cmd_s(&["ZADD", &z1, "1", "a", "2", "b"]); + c.cmd_s(&["ZADD", &z2, "2", "b"]); + + let g_key = format!("g:{tag}"); + let geoadd = c.cmd_s(&[ + "GEOADD", + &g_key, + "13.361389", + "38.115556", + "Palermo", + "15.087269", + "37.502669", + "Catania", + ]); + assert_eq!(geoadd, Resp::Int(2), "GEOADD fixture: {geoadd:?}"); + + let l1 = format!("l1:{tag}"); + let l2 = format!("l2:{tag}"); + c.cmd_s(&["SET", &l1, "ohmytext"]); + c.cmd_s(&["SET", &l2, "mynewtext"]); + + let e_key = format!("e:{tag}"); + c.cmd_s(&["SET", &e_key, "v"]); + assert_eq!( + c.cmd_s(&["EXPIREAT", &e_key, "9999999999"]), + Resp::Int(1), + "EXPIREAT fixture" + ); + + // ---- the 20 reads (RED on main: all "unknown command") ---- + + // XRANGE: both entries, in order. + let xr = c.cmd_s(&["XRANGE", &s_key, "-", "+"]); + let flat = xr.flat(); + assert!( + flat.contains("1-1") + && flat.contains("2-2") + && flat.contains('v') + && flat.contains('w'), + "XRANGE {tag}: {xr:?}" + ); + + // XREVRANGE: newest first. + let xrev = c.cmd_s(&["XREVRANGE", &s_key, "+", "-"]); + match &xrev { + Resp::Array(Some(items)) if items.len() == 2 => { + assert!( + items[0].flat().contains("2-2"), + "XREVRANGE order {tag}: {xrev:?}" + ); + } + other => panic!("XREVRANGE {tag}: {other:?}"), + } + + // XREAD (non-blocking form). + let xread = c.cmd_s(&["XREAD", "COUNT", "10", "STREAMS", &s_key, "0"]); + let flat = xread.flat(); + assert!( + flat.contains(&s_key) && flat.contains("1-1") && flat.contains("2-2"), + "XREAD {tag}: {xread:?}" + ); + + // XINFO STREAM: reports length 2. + let xinfo = c.cmd_s(&["XINFO", "STREAM", &s_key]); + let flat = xinfo.flat(); + assert!(flat.contains("length"), "XINFO STREAM {tag}: {xinfo:?}"); + + // XPENDING summary form on an idle group: count 0. + let xp = c.cmd_s(&["XPENDING", &s_key, "g"]); + match &xp { + Resp::Array(Some(items)) if !items.is_empty() => { + assert_eq!(items[0], Resp::Int(0), "XPENDING count {tag}: {xp:?}"); + } + other => panic!("XPENDING {tag}: {other:?}"), + } + + // ZDIFF / ZINTER / ZUNION / ZINTERCARD / ZRANDMEMBER. + let zdiff = c.cmd_s(&["ZDIFF", "2", &z1, &z2]); + assert_eq!(zdiff.flat(), "a", "ZDIFF {tag}: {zdiff:?}"); + let zinter = c.cmd_s(&["ZINTER", "2", &z1, &z2]); + assert_eq!(zinter.flat(), "b", "ZINTER {tag}: {zinter:?}"); + let zunion = c.cmd_s(&["ZUNION", "2", &z1, &z2]); + assert_eq!(zunion.flat(), "a b", "ZUNION {tag}: {zunion:?}"); + let zic = c.cmd_s(&["ZINTERCARD", "2", &z1, &z2]); + assert_eq!(zic, Resp::Int(1), "ZINTERCARD {tag}: {zic:?}"); + let zrm = c.cmd_s(&["ZRANDMEMBER", &z1]); + let m = zrm.bulk_str().unwrap_or_default(); + assert!(m == "a" || m == "b", "ZRANDMEMBER {tag}: {zrm:?}"); + + // GEOPOS: coordinate prefixes (float formatting is A1 territory — the + // consistency suite arbitrates exact bytes; here we pin the values). + let gp = c.cmd_s(&["GEOPOS", &g_key, "Palermo"]); + let flat = gp.flat(); + assert!( + flat.contains("13.36") && flat.contains("38.11"), + "GEOPOS {tag}: {gp:?}" + ); + + // GEODIST in km: Palermo–Catania ≈ 166.27 km. + let gd = c.cmd_s(&["GEODIST", &g_key, "Palermo", "Catania", "km"]); + let km: f64 = gd + .bulk_str() + .unwrap_or_default() + .parse() + .unwrap_or_else(|_| panic!("GEODIST {tag}: {gd:?}")); + assert!((166.0..167.0).contains(&km), "GEODIST {tag}: {km}"); + + // GEOHASH: canonical Redis hash for Palermo starts with sqc8b. + let gh = c.cmd_s(&["GEOHASH", &g_key, "Palermo"]); + assert!(gh.flat().starts_with("sqc8b"), "GEOHASH {tag}: {gh:?}"); + + // GEOSEARCH: ASC by distance from Palermo → Palermo, Catania. + let gs = c.cmd_s(&[ + "GEOSEARCH", + &g_key, + "FROMMEMBER", + "Palermo", + "BYRADIUS", + "200", + "km", + "ASC", + ]); + assert_eq!(gs.flat(), "Palermo Catania", "GEOSEARCH {tag}: {gs:?}"); + + // LCS. + let lcs = c.cmd_s(&["LCS", &l1, &l2]); + assert_eq!( + lcs.bulk_str().as_deref(), + Some("mytext"), + "LCS {tag}: {lcs:?}" + ); + + // EXPIRETIME / PEXPIRETIME. + let et = c.cmd_s(&["EXPIRETIME", &e_key]); + assert_eq!(et, Resp::Int(9_999_999_999), "EXPIRETIME {tag}: {et:?}"); + let pet = c.cmd_s(&["PEXPIRETIME", &e_key]); + assert_eq!( + pet, + Resp::Int(9_999_999_999_000), + "PEXPIRETIME {tag}: {pet:?}" + ); + + // TOUCH: both keys exist. + let touch = c.cmd_s(&["TOUCH", &e_key, &l1]); + assert_eq!(touch, Resp::Int(2), "TOUCH {tag}: {touch:?}"); + + // (v2) XLEN: the stream holds exactly the two fixture entries. + let xlen = c.cmd_s(&["XLEN", &s_key]); + assert_eq!(xlen, Resp::Int(2), "XLEN {tag}: {xlen:?}"); + + // (v2) ZMSCORE: existing member scores + nil for a missing member. + let zms = c.cmd_s(&["ZMSCORE", &z1, "a", "b", "nosuch"]); + match &zms { + Resp::Array(Some(items)) if items.len() == 3 => { + assert_eq!( + items[0].bulk_str().as_deref(), + Some("1"), + "ZMSCORE a {tag}: {zms:?}" + ); + assert_eq!( + items[1].bulk_str().as_deref(), + Some("2"), + "ZMSCORE b {tag}: {zms:?}" + ); + assert_eq!(items[2], Resp::Bulk(None), "ZMSCORE missing {tag}: {zms:?}"); + } + other => panic!("ZMSCORE {tag}: {other:?}"), + } + } + + // RANDOMKEY: keyspace is non-empty. + let rk = c.cmd_s(&["RANDOMKEY"]); + assert!(rk.bulk_str().is_some(), "RANDOMKEY: {rk:?}"); + + // LOLWUT: any non-error, non-empty reply. + let lol = c.cmd_s(&["LOLWUT"]); + assert!( + !matches!(lol, Resp::Error(_)) && !lol.flat().is_empty(), + "LOLWUT: {lol:?}" + ); + + // (v2) TIME: [unix-seconds, microseconds] as two integer-parseable bulks. + let time = c.cmd_s(&["TIME"]); + match &time { + Resp::Array(Some(items)) if items.len() == 2 => { + let secs: i64 = items[0] + .bulk_str() + .unwrap_or_default() + .parse() + .unwrap_or_else(|_| panic!("TIME seconds: {time:?}")); + assert!(secs > 1_700_000_000, "TIME plausibility: {time:?}"); + } + other => panic!("TIME: {other:?}"), + } + + // (v2) WAIT 0 0: no replication → 0 replicas acked. + let wait = c.cmd_s(&["WAIT", "0", "0"]); + assert_eq!(wait, Resp::Int(0), "WAIT: {wait:?}"); + + // (v2) SLOWLOG LEN / GET. + let sl_len = c.cmd_s(&["SLOWLOG", "LEN"]); + assert!(matches!(sl_len, Resp::Int(_)), "SLOWLOG LEN: {sl_len:?}"); + let sl_get = c.cmd_s(&["SLOWLOG", "GET"]); + assert!( + matches!(sl_get, Resp::Array(Some(_))), + "SLOWLOG GET: {sl_get:?}" + ); + + // (v2) CDC.READ with no args: an ARITY error proves the dispatcher knows + // the command (the monoio gap answers "unknown command" instead). + let cdc = c.cmd_s(&["CDC.READ"]); + assert!( + matches!(&cdc, Resp::Error(_)) && !cdc.is_error_containing("unknown command"), + "CDC.READ must be recognized (arity error), got: {cdc:?}" + ); + + // Reject pin (fastpath_regression): plain reads on the same connection + // still answer via the existing arms. + c.cmd_s(&["SET", "plain", "x"]); + assert_eq!(c.cmd_s(&["GET", "plain"]).bulk_str().as_deref(), Some("x")); +} + +// --------------------------------------------------------------------------- +// cdg3 — the new arms are PURE reads: expired keys invisible, AOF untouched +// --------------------------------------------------------------------------- + +/// Total AOF bytes for the server's `--dir`, across both layouts: +/// monoio writes the multi-part `appendonlydir/`, tokio writes a single +/// top-level `appendonly.aof`. +fn aof_bytes(dir: &std::path::Path) -> u64 { + let mut total = 0u64; + if let Ok(rd) = std::fs::read_dir(dir.join("appendonlydir")) { + total += rd + .filter_map(|e| e.ok()) + .filter_map(|e| e.metadata().ok()) + .map(|m| m.len()) + .sum::(); + } + if let Ok(m) = std::fs::metadata(dir.join("appendonly.aof")) { + total += m.len(); + } + total +} + +/// Wait until the AOF length is non-zero (if `expect_nonzero`) and stable +/// across two consecutive samples. AOF flush cadence is runtime-dependent +/// (monoio flushes on a 1ms tick; tokio batches on the everysec schedule), +/// so a fixed sleep under-waits on one runtime and over-waits on the other. +/// The asserts that consume this value are unchanged — this only makes the +/// sample point deterministic. +fn aof_quiesce(dir: &std::path::Path, expect_nonzero: bool) -> u64 { + let deadline = Instant::now() + Duration::from_secs(10); + let mut prev = aof_bytes(dir); + loop { + std::thread::sleep(Duration::from_millis(250)); + let cur = aof_bytes(dir); + if cur == prev && (!expect_nonzero || cur > 0) { + return cur; + } + if Instant::now() >= deadline { + return cur; + } + prev = cur; + } +} + +#[test] +fn cdg3_read_arms_are_pure_reads() { + let port = free_port(); + let dir = tempfile::tempdir().expect("tempdir"); + let _guard = ServerGuard(spawn_moon(port, dir.path(), 1, &["--appendonly", "yes"])); + drop(wait_ready(port)); + let mut c = Conn::new(connect(port, Duration::from_secs(10))); + + // ---- (a) expired keys are invisible through every new arm ---- + c.cmd_s(&["SET", "exp:k", "v", "PX", "60"]); + c.cmd_s(&["XADD", "exp:s", "1-1", "f", "v"]); + c.cmd_s(&["PEXPIRE", "exp:s", "60"]); + c.cmd_s(&["ZADD", "exp:z", "1", "a"]); + c.cmd_s(&["PEXPIRE", "exp:z", "60"]); + c.cmd_s(&["GEOADD", "exp:g", "13.361389", "38.115556", "Palermo"]); + c.cmd_s(&["PEXPIRE", "exp:g", "60"]); + std::thread::sleep(Duration::from_millis(200)); // all TTLs lapsed + + let et = c.cmd_s(&["EXPIRETIME", "exp:k"]); + assert_eq!(et, Resp::Int(-2), "EXPIRETIME on expired key: {et:?}"); + let touch = c.cmd_s(&["TOUCH", "exp:k"]); + assert_eq!(touch, Resp::Int(0), "TOUCH on expired key: {touch:?}"); + let xr = c.cmd_s(&["XRANGE", "exp:s", "-", "+"]); + assert_eq!( + xr, + Resp::Array(Some(vec![])), + "XRANGE on expired stream: {xr:?}" + ); + let zd = c.cmd_s(&["ZDIFF", "1", "exp:z"]); + assert_eq!( + zd, + Resp::Array(Some(vec![])), + "ZDIFF on expired zset: {zd:?}" + ); + let gp = c.cmd_s(&["GEOPOS", "exp:g", "Palermo"]); + match &gp { + Resp::Array(Some(items)) if items.len() == 1 => assert!( + matches!(items[0], Resp::Array(None) | Resp::Bulk(None)), + "GEOPOS on expired geo key must be a nil position: {gp:?}" + ), + other => panic!("GEOPOS on expired geo key: {other:?}"), + } + let lcs = c.cmd_s(&["LCS", "exp:k", "exp:missing"]); + assert_eq!( + lcs.bulk_str().as_deref(), + Some(""), + "LCS on missing keys is empty string: {lcs:?}" + ); + // (v2) XLEN / ZMSCORE on expired keys: missing-form replies. + let xlen = c.cmd_s(&["XLEN", "exp:s"]); + assert_eq!(xlen, Resp::Int(0), "XLEN on expired stream: {xlen:?}"); + let zms = c.cmd_s(&["ZMSCORE", "exp:z", "a"]); + assert_eq!( + zms, + Resp::Array(Some(vec![Resp::Bulk(None)])), + "ZMSCORE on expired zset: {zms:?}" + ); + // Only expired keys ever existed → RANDOMKEY must not resurrect one. + let rk = c.cmd_s(&["RANDOMKEY"]); + assert_eq!( + rk, + Resp::Bulk(None), + "RANDOMKEY over an all-expired keyspace: {rk:?}" + ); + + // ---- (b) reads of LIVE keys never append to the AOF ---- + c.cmd_s(&["SET", "live:k", "v"]); + c.cmd_s(&["XADD", "live:s", "1-1", "f", "v"]); + c.cmd_s(&["ZADD", "live:z", "1", "a"]); + c.cmd_s(&["GEOADD", "live:g", "13.361389", "38.115556", "Palermo"]); + let before = aof_quiesce(dir.path(), true); + assert!(before > 0, "fixtures must have hit the AOF"); + for _ in 0..200 { + c.cmd_s(&["TOUCH", "live:k"]); + c.cmd_s(&["EXPIRETIME", "live:k"]); + c.cmd_s(&["XRANGE", "live:s", "-", "+"]); + c.cmd_s(&["ZDIFF", "1", "live:z"]); + c.cmd_s(&["GEOPOS", "live:g", "Palermo"]); + c.cmd_s(&["RANDOMKEY"]); + // (v2) the new arms are reads too. + c.cmd_s(&["XLEN", "live:s"]); + c.cmd_s(&["ZMSCORE", "live:z", "a"]); + c.cmd_s(&["TIME"]); + c.cmd_s(&["SLOWLOG", "LEN"]); + } + let after = aof_quiesce(dir.path(), true); + assert_eq!( + before, after, + "reads must never append to the AOF (before={before} after={after})" + ); + + // ---- (c) the write path is intact: a SET grows the AOF ---- + c.cmd_s(&["SET", "live:k2", "v2"]); + // Wait for growth (flush cadence is runtime-dependent), then sample. + let grown = { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let cur = aof_bytes(dir.path()); + if cur > after || Instant::now() >= deadline { + break cur; + } + std::thread::sleep(Duration::from_millis(100)); + } + }; + assert!( + grown > after, + "a write after the read batch must append (after={after} grown={grown})" + ); +}