Skip to content

perf: N-shard CLUSTER SLOTS projection + MOVED = hop eliminated (#517 PR4)#525

Merged
ELares merged 2 commits into
mainfrom
perf/shard-owner-projection
Jul 6, 2026
Merged

perf: N-shard CLUSTER SLOTS projection + MOVED = hop eliminated (#517 PR4)#525
ELares merged 2 commits into
mainfrom
perf/shard-owner-projection

Conversation

@ELares

@ELares ELares commented Jul 6, 2026

Copy link
Copy Markdown
Owner

What (the #517 milestone)

A shard-owners node now boots the N-shard projection instead of single-node-owns-all: N synthetic cluster nodes, node i at (advertised_host, base_port + i) owning the contiguous slot range shard_slot_range(i, N) -- the same slot_to_shard partition the internal router uses.

  • CLUSTER SLOTS / SHARDS / NODES advertise N owners at N ports (fully map-driven -- zero command-layer changes); CLUSTER INFO reports cluster_size:N, all 16384 slots assigned, state:ok.
  • A cluster-aware client dials each key's owner port -> the connection homes on the owning shard (PR3 listeners) -> owner_shard(key,N) == home.index -> the request takes the local fast path with no cross-shard hop. The hop is eliminated.
  • A key sent to the wrong port gets -MOVED <slot> host:base+owner, so clients converge to zero hops.

Mechanism (per the recorded design)

Thread the home ShardId into the redirect: in shard-owners mode moved_if_unowned decides ownership by the contiguous partition (slot_to_shard(slot, home.total) == home.index) -- the identical predicate the hop uses, so the two can never disagree. No per-shard SlotMap duplication (one shared display map); Static/Raft pass home_owner = None and are byte-unchanged.

Also fixes a latent MOVED-target bug: a 0.0.0.0/:: bind is not dialable, so the advertised host falls back to loopback (shard_owner_announce_host). synth_node_id is made pub for the synthetic per-shard ids (node 0 = the announce id = self, so CLUSTER MYID is stable).

Tests

  • owner_port_serves_locally_and_a_misrouted_key_gets_moved (the milestone assertion, 7 keys x 4 shards): owner port -> +OK/value with no MOVED; wrong port -> -MOVED pointing at the exact owner port.
  • cluster_info_reflects_the_n_shard_projection (size:4, 16384 assigned, state:ok), N=1 single-node (never MOVED), per-shard liveness; PR2/PR3 tests updated for the owns/MOVED semantics.
  • Regression: cluster_countkeysinslot + cross_shard_pipeline + 272 ironcache-lib + 573 ironcache-server tests pass unchanged; clippy -D warnings (tokio + io_uring) + fmt + invariant lints clean.

Behavioral note (intentional, standard Redis Cluster semantics): a plain non-cluster client pinned to one port is served only for the keys that port owns and gets MOVED for the rest -- exactly how a real Redis Cluster node behaves. Cluster-aware clients (redis-cli -c, every cluster driver) handle this transparently.

Ref #507, #517. Next (PR5, final): driver-matrix cluster leg + zero-hop metric assertion + docs.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

perf-gate (A5)

Same-runner ratchet of HEAD against the merge-base (both rebuilt and measured in this job).
PASS = within the noise band, WARN = a real move inside budget (does not fail), FAIL = past budget in the bad direction.

metric base head delta% band budget verdict
qps_median (peak) 97375.99 97696.30 0.33% +/-5.00% drop <= 15% PASS
bytes_per_key int 45.02 45.02 0.00% det rise <= 5% PASS
bytes_per_key embstr 61.07 61.07 0.00% det rise <= 5% PASS
bytes_per_key raw 333.15 333.15 0.00% det rise <= 5% PASS

Overall: PASS

  • qps: noisy on shared CI, so the band comes from the base reps spread (floored at 5%); a drop is only a regression past the 15% budget.
  • bytes_per_key: deterministic (allocator-true memmodel), so a tight 5% rise budget; any rise beyond it FAILs.
  • Open-loop tails / criterion micro-benches are reported-not-failed (tail noise is high) and are not part of this ratchet.
  • An intentional perf trade is landed by raising the relevant budget in this PR with a documented reason (CI never auto-commits a baseline).

@ELares

ELares commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

Deep self-review: verdict CORRECT (routing core), two peripheral findings addressed

The adversarial review confirmed the milestone mechanics end to end: the slot math is proven (shard_slot_range is the exact inverse of slot_to_shard; no underflow for any n <= 256), home.total always equals the projection's N (both derive from config.shards.max(1)), moved_target(slot) always returns the slot's owner node at base+owner, Static/Raft paths are byte-unchanged (home_owner = None at every call site), in-MULTI foreign keys MOVED-dirty the txn per Redis semantics, CROSSSLOT fires before any fan-out, and the IPv6 MOVED format follows the raw host:port Redis convention. Nothing misroutes data.

Findings addressed:

  1. Port-block validation (fixed): port .. port + shards - 1 was unvalidated, so an overflowing config hit a boot panic in the projection build, pre-empting PR3's graceful bind error. Config::validate (new validate_shard_owners helper) now rejects it with one clean error; boundary tests added (65500+100 rejected; 65532+4 = top port exactly 65535 accepted).
  2. Whole-keyspace divergence (documented + tracked as shard-owners: scope whole-keyspace commands (KEYS/SCAN/DBSIZE/RANDOMKEY/FLUSH) to the connecting shard for Redis Cluster parity #526): KEYS/SCAN/DBSIZE/RANDOMKEY/FLUSH* are redirect-exempt and fan out across all shards from any port, so per-node cluster aggregators over-count by N vs real Redis Cluster. Documented as a KNOWN DIVERGENCE in the CHANGELOG. The clean fix is natural post-perf: align the internal shard owner to the CRC16 cluster slot (#517 PR1) #520 (shard i's store holds exactly its slot range, so home-only answering gives exact parity) but changes SCAN cursor semantics, deserving its own PR + tests: shard-owners: scope whole-keyspace commands (KEYS/SCAN/DBSIZE/RANDOMKEY/FLUSH) to the connecting shard for Redis Cluster parity #526 with full acceptance criteria. The review's informational note (SPUBLISH broadcasts globally, pre-existing superset-delivery fidelity gap) is folded into shard-owners: scope whole-keyspace commands (KEYS/SCAN/DBSIZE/RANDOMKEY/FLUSH) to the connecting shard for Redis Cluster parity #526.

Merging on green.

ELares added a commit that referenced this pull request Jul 6, 2026
…loses #533) (#537)

* ci: run tests under cargo-nextest to fix the load-dependent deadlock (#533)

DIAGNOSIS. The intermittent ~50-min CI hang (blocked #520/#521/#523/#524/#525 merges)
is the io_uring `fixed_datapath` ring tests deadlocking under parallel load. Evidence:
the hang signature was always a "6-test binary that never returns"; `fixed_datapath::tests`
is exactly a 6-test module; running the full workspace under nextest reproduced it as 4
of those tests TIMING OUT (hanging) rather than completing. io_uring rings + registered
buffers are a limited kernel resource, and running many ring lifecycles concurrently with
the rest of the workspace blocks on a submission that never completes. `raft_cluster`
(the earlier suspect) passes 5/5 in isolation, consistent with the true cause being
resource contention, not that test.

FIX (two layers, both in this change):
- Switch the `test` and `io-uring` CI jobs to `cargo nextest run --profile ci`, which runs
  each test in its OWN process (isolating shared-state / port-contention coupling) and
  enforces a per-test timeout (.config/nextest.toml: SIGKILL after 120s), so any residual
  hang becomes a fast, named failure instead of burning the runner. `timeout-minutes: 30`
  caps the whole job as a backstop. nextest does not run doctests, so a `cargo test --doc`
  step is added.
- A nextest test-group pins every io_uring test (`package(=ironcache-runtime) and
  (io_uring|fixed_datapath|uring)`) to `max-threads = 1`, so ring lifecycles run SERIALLY
  and never contend -- removing the contention that triggers the hang at the source.

Installed via `taiki-e/install-action@nextest` (prebuilt, no compile). Local validation:
2224 non-io_uring tests pass under `nextest --profile ci`; the config + the io-uring group
filter parse and target the fixed_datapath tests (io_uring itself is unrunnable in the
local sandbox -- PermissionDenied on ring init -- but runs on GitHub's ubuntu runners,
where the dedicated io-uring job already proves it green).

Closes #533. Supersedes the cancel-and-rerun stopgap.

Signed-off-by: Zeke <ezequiel.lares@outlook.com>

* ci: optimize the DST simulation crates in dev builds (the real deadlock cause)

REFINED DIAGNOSIS. Running the workspace under nextest with a per-test timeout revealed the
actual cause of the "50-min CI hang": it was NOT a deadlock and NOT io_uring. It is the Raft
DETERMINISTIC-SIMULATION seed sweeps (ironcache-raft tests::determinism_and_safety_seed_sweep,
failover_split_brain_gate, snapshot_catchup_gate*, migration_*_gate -- a 6-test set matching the
observed "6-test binary" signature). Measured in DEBUG, in isolation, they take 145s / 256s / 107s
EACH; under the parallel `cargo test --workspace` run they were CPU-starved into the tens of minutes,
which looked like a hang. In release they are fast (the whole crate runs in seconds), because the
simulation loop is compute-bound and unoptimized codegen is ~8x slower.

FIX. Optimize ONLY the simulation crates in dev/test builds via
`[profile.dev.package.{ironcache-raft,ironcache-sim,ironcache-cluster,ironcache-store}] opt-level = 3`
(root Cargo.toml). Measured result: the three heaviest DST tests drop from 145/256/107s to
18/33/14s (~8x), and all 76 ironcache-raft tests pass under `nextest --profile ci` in 100s total
with no timeout. debug-assertions + overflow-checks stay ON (this is dev, not release), so the sims
still exercise the checked build -- the standard DST posture (optimized harness, asserts kept).

The nextest switch (previous commit) remains: process-per-test isolation + a per-test timeout
(now 90s soft / 270s SIGKILL, headroom for the optimized DST under CI load) + the 30-min job cap
turn any FUTURE genuine hang into a fast named failure. The io_uring serial test-group stays as a
cheap precaution (rings are a limited kernel resource), though it was not the cause.

Local: 2224 non-io_uring tests + all 76 ironcache-raft tests green under nextest; fmt clean.
Ref #533, #527 (M1).

Signed-off-by: Zeke <ezequiel.lares@outlook.com>

---------

Signed-off-by: Zeke <ezequiel.lares@outlook.com>
ELares and others added 2 commits July 6, 2026 13:46
…PR4)

The hop-elimination MILESTONE. A shard-owners node now boots the N-SHARD PROJECTION
instead of single-node-owns-all: N synthetic cluster nodes, node i at
(advertised_host, base_port + i) owning the contiguous slot range shard_slot_range(i, N)
-- the SAME slot_to_shard partition the internal router uses. So:

- CLUSTER SLOTS / SHARDS / NODES advertise N owners at N ports (map-driven, no cmd
  change); CLUSTER INFO reports cluster_size:N, all 16384 slots assigned, state:ok.
- A cluster-aware client dials each key's OWNER port -> the connection homes on the
  owning shard (PR3 listeners) -> owner_shard(key,N) == home.index -> the request takes
  the LOCAL fast path with NO cross-shard hop. The hop is eliminated.
- A key sent to the WRONG port gets -MOVED <slot> host:base+owner, so the client
  reconnects to the owner and converges to zero hops.

Mechanism (Approach A, per the design): thread the home ShardId into the redirect. In
shard-owners mode `moved_if_unowned` decides ownership by the contiguous partition
(slot_to_shard(slot, home.total) == home.index) -- the identical predicate the hop uses,
so the two always agree. This needs no per-shard SlotMap duplication (one shared display
map); Static/Raft pass home_owner=None and are byte-unchanged.

Also fixes a latent MOVED-target bug: a 0.0.0.0/:: bind is not dialable, so the advertised
host falls back to loopback (shard_owner_announce_host). synth_node_id is made pub for the
synthetic per-shard node ids (node 0 = the announce id, so it is self + CLUSTER MYID).

Tests (shard_owners_mode.rs): owner_port_serves_locally_and_a_misrouted_key_gets_moved
(the milestone: owner port -> +OK no MOVED; wrong port -> -MOVED to owner port),
cluster_info_reflects_the_n_shard_projection (size:4, 16384 assigned), N=1 single-node,
per-shard liveness. Updated the PR2/PR3 tests for the new owns/MOVED semantics.
cluster_countkeysinslot + cross_shard_pipeline + 272 ironcache-lib + 573 ironcache-server
tests pass unchanged; clippy -D warnings (tokio + io_uring) + fmt + invariant lints clean.

Ref #507, #517.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Zeke <ezequiel.lares@outlook.com>
…ce divergence (#517 review)

Adversarial review verdict on the projection + MOVED: the routing core is CORRECT and
internally consistent (slot math proven, home.total == projection N from the same
config.shards.max(1), moved_target always the slot owner, Static/Raft byte-unchanged,
in-MULTI MOVED-dirties correctly, IPv6 MOVED parses per Redis convention). Two genuine
peripheral findings, both addressed:

- [config] the per-shard port block `port .. port + shards - 1` was not validated, so
  an overflowing config (e.g. port 65500 + 100 shards) hit a boot panic in the
  projection build, pre-empting PR3's graceful bind error. Extracted a
  validate_shard_owners helper (also keeps validate under the clippy line limit) that
  rejects the overflow with one clean config error; boundary tests added (65500+100
  rejected, 65532+4 = top port 65535 accepted).
- [docs] the whole-keyspace commands (KEYS/SCAN/DBSIZE/RANDOMKEY/FLUSHDB/FLUSHALL) are
  redirect-exempt and fan out across ALL shards from ANY per-shard port, so a
  cluster-aware per-node aggregator over-counts by the shard factor N vs a real Redis
  Cluster node (which answers only for its own slots). Documented as a KNOWN DIVERGENCE
  in the CHANGELOG; the scoping fix (natural post-#520: shard i's store holds exactly
  its slot range, so home-only answering gives exact Redis Cluster parity) is tracked
  as issue #526 with acceptance criteria (also notes the pre-existing SPUBLISH
  global-broadcast fidelity gap).

70 config tests pass; clippy -D warnings (incl. io_uring targets) + fmt + invariant
lints clean; 0 dashes.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Zeke <ezequiel.lares@outlook.com>
@ELares ELares force-pushed the perf/shard-owner-projection branch from d5710b3 to 1cd4d7e Compare July 6, 2026 20:46
@ELares ELares merged commit 2f54e32 into main Jul 6, 2026
15 checks passed
@ELares ELares deleted the perf/shard-owner-projection branch July 6, 2026 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant