feat(sorted_set): implement ZRANGEBYLEX, ZREVRANGEBYLEX, ZREMRANGEBY{RANK,SCORE,LEX}, ZDIFFSTORE and ZADD INCR (moon#959) [stacked on #991] - #1003
Draft
TinDang97 wants to merge 9 commits into
Conversation
…rs as syntax errors
Two inputs moon ACCEPTED that Redis rejects, plus a sweep of wrong error
classes. Every expectation here was taken from a live redis-server 8.6.1 on a
second port, command by command, before anything was changed.
Accepted where Redis errors:
* `ZADD k GT LT 1 m` returned `(integer) 1`. The guard only rejected
`nx && (gt || lt)`; Redis rejects all three pairings. Worse, the pairing then
reached TWO mutation sites that answered it with a SILENT no-op — the
listpack arm and the B+tree arm each carried a `gt && lt => false`
fallthrough, so a client asking for an impossible combination got `0` and an
unchanged score instead of an error. Both arms are removed rather than left
unreachable, so relaxing the guard cannot quietly resurrect the no-op. The
message also said "GT, LT, and NX"; Redis says "and/or".
* `ZUNIONSTORE d 1 k WEIGHTS nan` returned `(integer) 1`, and the read twin
`ZUNION 1 k WEIGHTS nan` returned the member. Rust's `str::parse::<f64>`
accepts "nan" where C's `strtod` + `isnan` check in
`getDoubleFromObjectOrReply` rejects it, and only a `None` mapped to the
error. A NaN weight poisons every aggregated score. Infinities stay legal, as
they are on Redis.
Wrong error CLASS. This matters beyond wording: redis-py raises a distinct
exception type per class, so a client branching on the exception takes the
wrong branch — and retries a request that can never succeed.
command was now
ZPOPMIN k notanint / k -1 not an integer value is out of range, must be positive
ZINTERCARD 0 k numkeys can't be ... at least 1 input key is needed for 'zintercard' command
ZUNIONSTORE d 0 k wrong number of args at least 1 input key is needed for 'zunionstore' command
ZUNIONSTORE d -1 k not an integer (same)
ZUNIONSTORE d 1 k WEIGHTS wrong number of args syntax error
ZUNIONSTORE d 1 k AGGREGATE wrong number of args syntax error
ZUNIONSTORE d 2 k wrong number of args syntax error
ZADD k 1 a 2 wrong number of args syntax error
ZMPOP 0 k MIN numkeys can't be ... numkeys should be greater than 0
The set-operation family splits into TWO classes and the split is the point:
`getLongFromObjectOrReply(..., NULL)` answers the generic integer error for
bytes that are not a number, and only then does `if (setnum < 1)` name the
command. ZMPOP does NOT split — `getRangeLongFromObject` carries one message
for every failure. Arity is checked FIRST, so `ZUNION 0`, which names no key at
all, stays an arity error and never reaches the numkeys rules.
Two divergences found while reproducing the above, in neither issue:
* `ZINTERCARD k LIMIT -1` — `limit: usize` failed a negative into the generic
message; Redis says `LIMIT can't be negative`.
* `ZMPOP 1 k MIN COUNT 0` / `COUNT -1` — Redis says
`count should be greater than 0`.
And one moon#967 leftover: that sweep rewrote every zset option loop to reject
an unrecognised token except `zstore_impl`, so `ZUNIONSTORE d 1 k BOGUS`
stepped over `BOGUS` and answered a different, successful command.
Deliberately NOT changed. moon#969 also cites four ZRANGE-family sites as
wrong. They are not. `mod.rs:317/:329/:639/:649` are rank-index parses, and the
`LIMIT offset count` parses next to them take the same shape; Redis reads all
of them with `getLongFromObjectOrReply(..., NULL)`, which produces exactly the
`ERR value is not an integer or out of range` moon already answers — confirmed
against the oracle, which disagrees with the issue. Those sites also accept
negatives, which the new helpers do not. `rank_and_limit_parses_keep_the_
generic_integer_error` pins all eight so a later reading of moon#969 cannot
"fix" them into a divergence.
Two shared helpers carry the two shapes so eleven call sites do not each
transcribe them, and the boundary against the correct sites is explicit:
`parse_numkeys` (two classes, names the command) and `parse_bounded_count`
(one bespoke message, minimum bound). The ZADD option loop itself changes by
one condition and one error class only.
BEHAVIOUR CHANGE: `ZADD ... GT LT` and `WEIGHTS nan` now error where they
previously succeeded.
Closes #969
author: Tin Dang
…ilon window
Both ZADD mutation loops decided `changed` with an ABSOLUTE `f64::EPSILON`
window — `(old - new).abs() > f64::EPSILON` — where Redis's `zsetAdd` compares
the two scores EXACTLY (`if (score != curscore)`).
`f64::EPSILON` is the gap between 1.0 and the next double, a RELATIVE
quantity. Used as an absolute tolerance it swallows real differences at every
magnitude below 1, and the error grows as the scores shrink:
ZADD k 0.0000000001 m
ZADD k CH 0.00000000010000001 m -> moon: 0 redis: 1
ZSCORE k m -> 0.00000000010000001
The reply said nothing changed; the very next read said otherwise. That is not
a rounding nicety — a change of six significant figures was reported as no
change, because the ABSOLUTE difference (1e-17) sits under a window sized for
magnitude 1. Any client using `CH` as a did-anything-happen signal — cache
invalidation, a change feed, a dirty flag — silently skipped the update.
The window also disagreed with the code that does the writing.
`zset_update_existing` decides whether to move the member on `to_bits()`
inequality, so a sub-epsilon rescore really was written to both the hash and
the B+tree, and only the tally pretended otherwise.
Both arms are fixed, because they are separate code: the listpack loop and the
B+tree loop each carried their own copy. `ch_counts_a_sub_epsilon_rescore`
drives both — a short member for the listpack encoding, a 73-byte member to
force the B+tree — and asserts the reply AND the stored score, so the two can
no longer disagree. It uses `nextafter(1.0)`, whose distance from 1.0 is
exactly `f64::EPSILON` and so failed the old strict `>`.
Neither side can be NaN: `parse_zadd_pair` rejects a NaN score before either
loop runs, so `!=` is total here. `-0.0 != 0.0` is false, matching Redis.
The range-bound half of the same epsilon defect was fixed separately by
moon#966; this is the remaining `CH` counter.
Closes #792
author: Tin Dang
…counter Neither harness had a single row for any of these forms, which is exactly how they were free to drift: `scripts/test-consistency.sh` and `scripts/test-commands.sh` both compare moon against a live redis, so a row here is a standing oracle assertion. 69 `assert_both` rows in the consistency suite and 26 `assert_match` rows in the command suite, covering: the three GT/LT/NX pairings and the score being left alone at BOTH encodings; the odd-tail-vs-no-pairs split; a NaN weight on all four set-operation commands plus the infinite weight that stays legal; the ZPOPMIN/ZPOPMAX count class; the two-class numkeys split and the arity floor above it; ZMPOP's single-message numkeys and COUNT; ZINTERCARD's LIMIT; and the six syntax-vs-arity forms. Proven red before the fix, against the pre-fix binary and the same redis 8.6.1 oracle: 48 of the 69 rows FAILED, and the 21 that passed are precisely the anti-regression rows plus the forms that were already correct. All 69 pass after. The rows were extracted verbatim from the committed script to run that comparison, so the evidence cannot have drifted from what landed here. The anti-regression rows are the point of the last block. moon#969 names four ZRANGE-family sites as wrong; the oracle says they are not, because Redis reads a rank index and a `LIMIT offset count` with `getLongFromObjectOrReply(..., NULL)` and that IS the generic integer error moon already answers. Those ten rows passed identically before and after this branch, on both servers, and exist so a later reading of the issue cannot "fix" them into a divergence. CHANGELOG records the behaviour change: `ZADD GT LT` and `WEIGHTS nan` now error where they previously succeeded. Refs #969 Refs #792 author: Tin Dang
ZRANGEBYLEX, ZREVRANGEBYLEX, ZREMRANGEBYRANK, ZREMRANGEBYSCORE, ZREMRANGEBYLEX and ZDIFFSTORE answered `ERR unknown command` on moon, and `ZADD ... INCR` — what redis-py's `zadd(..., incr=True)` sends — answered an arity error (moon#959). docs/commands.md advertised ZRANGEBYLEX all along. Every reply, error surface included, was read off a live redis-server 8.6.1 BEFORE the code was written (206 probes, 165 of them diverging on the pre-fix binary a8eb2ef), and the implementation reuses the bound parsers and range helpers ZRANGE already owns rather than growing a second grammar: * ZRANGEBYLEX / ZREVRANGEBYLEX (`sorted_set_lex.rs`): the same shape as `zrangebyscore_readonly` — one implementation behind both spellings, through the shared-borrow view so a listpack survives the read (moon#928), wired into `dispatch`, `dispatch_read` and the read prefilter. Redis's precedence is kept: option loop, then the range grammar (checked BEFORE the key, so a bad bound on a missing key is an error), then the legacy spelling's own `WITHSCORES not supported in combination with BYLEX`. * ZREMRANGEBY{RANK,SCORE,LEX} (`sorted_set_write.rs`): `zrem`'s two-arm shape — a listpack is trimmed in place and never converted, the B+tree arm credits each member and the table shrink, and a drained key is deleted on both arms (which is also what reclaims the container the accessor fabricates for a missing key). Rank normalisation follows `zremrangeGenericCommand` through a new `rank_window` helper: a stop still negative after `len + stop` is NOT clamped, so `-10 -6` removes nothing. (`zrange_by_rank` clamps it and answers `[a]` where redis answers `[]` — a pre-existing ZRANGE divergence reported separately, not changed here.) * ZDIFFSTORE (`sorted_set_store.rs`, the store family moved out of the write half when it crossed the 1500-line rule): a third `SetOp` arm of `zstore_impl`, so it inherits the two-class numkeys split, the overrun rule and the unknown-token rule of moon#969; WEIGHTS/AGGREGATE are refused as `syntax error`, which is what redis does for `SET_OP_DIFF`. Two family-wide corrections rode along: the sources are now looked up BEFORE the options are parsed, so `ZUNIONSTORE d 1 <string> BOGUS` is WRONGTYPE as on redis; and they are read through `get_sorted_set_ref_if_alive`, so reading a listpack source no longer flattens it to a skiplist. * ZADD INCR: `zincrby` is refactored into `zincr_member` taking the NX/XX/GT/LT flags, in `zsetAdd`'s decision order — NX refuses a present member before the sum is formed, a NaN sum errors with nothing written, GT/LT refuse a sum that does not move the right way (zero included), only XX refuses an absent member, and a refusal on a key the call had to fabricate leaves nothing behind. Reply is the new score as a bulk string, or nil. Pair count is checked after the parity and flag-pairing rules, as redis does. Registered in the phf table as @sortedset with redis's arities; the `used_memory` ledger is asserted exact against a full recount on both encodings for every new write; the cold-tier read test gains the two lex reads. `try_inline_dispatch` inlines only GET and a plain SET, so there is no third arm for these to be missing from. Refs moon#959. Stacked on #991 (moon#969), whose error conventions the new commands follow. author: Tin Dang
…n#959 Neither scripts/test-commands.sh (504 rows) nor scripts/test-consistency.sh (132 rows) named any of the six commands or ZADD INCR, which is how a command docs/commands.md advertised went missing without a harness noticing. 29 rows join the first script and 87 the second, every one a reply read off redis 8.6.1: the happy paths, the bounds grammar (checked before the key), LIMIT, the numkeys classes and the refused option tokens of ZDIFFSTORE, WRONGTYPE from every command without clobbering the value, drained keys deleted, and the NX/XX/GT/LT decision order of ZADD INCR. Run verbatim against the pre-fix binary a8eb2ef: 109 of 116 rows red (62 of them `unknown command`, the rest the arity error ZADD INCR used to give); the 7 green are the fence rows that hold either way. Exclusive/inclusive bound tokens are quoted (`'(d'`): a bare `(` is a bash syntax error that kills the sourced script at that line, which the runner caught on the first attempt. docs/commands.md: the sorted-set line now lists exactly what dispatch accepts on a live server — the six new commands plus nine that were implemented but unlisted (ZRANGESTORE, ZDIFF, ZUNION, ZINTER, ZINTERCARD, ZMSCORE, ZRANDMEMBER, ZMPOP, BZMPOP), each sent to the server to confirm. Refs moon#959. author: Tin Dang
… isolated) ISOLATED COMMIT — the only change to src/server/conn/shared.rs in this branch, confined to `touches_a_key_it_did_not_route_on`'s doc block and its match. `is_inline_intercepted` and the `INTERCEPTED_NOT_DECLARED` waiver (moon#937 / moon#946, owned by a parallel change) are untouched. That doc block listed ZDIFFSTORE as "not implemented, so nothing to misplace" and named `tests/two_key_write_cross_shard.rs::t2k4` as the tripwire that fires the moment it starts working — which moon#959 did. Without the guard, `ZDIFFSTORE dst 1 src` at --shards >= 2 would ack +OK and write `dst` into the wrong shard's slice (the moon#592 defect). The arm is the same shape as ZUNIONSTORE/ZINTERSTORE's: routed on `dst`, reads every source. t2k4 migrates ZDIFFSTORE into PROBES, exactly as GEORADIUS/ GEORADIUSBYMEMBER migrated under moon#645, and keeps the two `_RO` rows. Refs moon#959, moon#592. author: Tin Dang
…bounds The first draft validated the lex bounds before the WITHSCORES refusal; the oracle sweep of the built binary (295 probes vs redis 8.6.1) caught the one probe that disagreed: `ZRANGEBYLEX k a b WITHSCORES` is `ERR syntax error, WITHSCORES not supported in combination with BYLEX` on redis, not the bound error. The unit test had encoded the wrong guess and is corrected; a row pinning the precedence joins both harnesses, plus one for the option loop still outranking WITHSCORES (`... WITHSCORES LIMIT 1` is a plain `syntax error`). Also: the registry comment no longer cites `COMMAND INFO` as a source (every arity was compared on the wire), and the docs tip states what was actually done to re-verify the sorted-set table. Refs moon#959. author: Tin Dang
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
Ruling on the
|
Collaborator
Author
Independent verification — my own build, live redis 8.6.1 oracleAll nine probe rows match redis byte-for-byte, and every one is
The The
|
TinDang97
changed the base branch from
fix/969-zset-option-semantics
to
main
September 17, 2026 04:31
…d guard #1003 was stacked on #991, which has since merged to main along with #990, #987, #993, #996 and #985. Retargeting to `main` left four conflicted files. Resolutions, each argued from behaviour rather than from which side is longer: * `src/command/sorted_set/sorted_set_write.rs` (3 hunks) — OURS, all three. The conflict looks like "main added `zunionstore`/`zinterstore`/`zstore_impl` and this branch did not", but `git diff c3c5e33 0cfab03 -- src/command/ sorted_set` is EMPTY: main's sorted-set content is exactly #991's, which is already in this branch's history, and #1003 then MOVED that family into the new `sorted_set_store.rs` when the file crossed 1500 lines. Taking main's side would have duplicated three functions. The `use` list is likewise ours: main's extra imports (`AggregateOp`, `parse_numkeys`, `zadd_member`, `zrange_by_*`, `clamp_nan_to_zero`) serve only the code that moved out, and `clippy --all-targets -D warnings` confirms nothing is unused or missing. * `scripts/test-commands.sh`, `scripts/test-consistency.sh` — both sides append independent sections and main's half of each conflict hunk is EMPTY, so the markers were removed in place rather than resolved by file. (`--ours` would have silently dropped the 616 lines main added elsewhere in `test-consistency.sh`; the merged files are pure additions against main — zero deleted lines — and `bash -n` is clean on both.) One row is ADDED by this merge: `test-consistency.sh`'s moon#592 `XW_CASES` sweep enumerates the cross-shard write family, and `ZDIFFSTORE` is now a member, so it gains a `zdiffstore` case beside `zunionstore`/`zinterstore` — the same row moon#645 added for `georadiusstore` when that command joined. * `src/server/conn/shared.rs` — the dangerous one, and not just a doc comment. ## ZDIFFSTORE: the routing invariant the two sides disagree about Main's doc block says `ZDIFFSTORE` is deliberately EXCLUDED from `touches_a_key_it_did_not_route_on` because it is "not implemented in moon (unknown command), so there is no write to misplace". moon#959 implements it. That sentence is now false, and shipping it as written would leave a cross-shard `ZDIFFSTORE` acking a destination that lands nowhere — the moon#592 misdirected destructive write that main's moon#962 work just closed for twelve other commands. The mechanical trap is that `ZDIFFSTORE` and `ZINTERCARD` are both ten bytes beginning with `z`, so the two sides edited the SAME match arm: HEAD: (10, b'z') => cmd.eq_ignore_ascii_case(b"ZDIFFSTORE") main: (10, b'z') => cmd.eq_ignore_ascii_case(b"ZINTERCARD") Keeping either arm alone drops the other command out of the guard, and keeping both as separate arms is an unreachable pattern. Resolved as one arm naming both spellings, with the collision documented at the arm so a future narrowing is not mistaken for a simplification. Two sites in main's own test code merged CLEANLY and were left asserting the pre-#959 world. Both are corrected here, because a conflict-free merge is not a correct one: * `out_of_family_and_malformed_argvs_keep_their_own_answers` asserted that `ZDIFFSTORE` must NOT be refused ("ZDIFFSTORE is unimplemented; t2k4 owns the moment that changes"). That assertion is now the bug. Removed, with a note saying where it went. * `ZDIFFSTORE` added to `FAMILY`, so `every_family_member_is_refused_across_a_boundary_and_only_there` now covers it positively in all four positions the table checks: refused across a boundary, NOT refused co-located, NOT refused under a `{hash}` tag, and NOT refused at `--shards 1`. The family doc block keeps main's moon#962 text verbatim (it is the accurate, more detailed version) minus the now-false `ZDIFFSTORE` bullet, and gains a short section recording the migration and the arm collision. `tests/two_key_write_cross_shard.rs` needed no change: #1003 had already moved `ZDIFFSTORE` from the `t2k4` unimplemented-store tripwire into `t2k1`'s `PROBES`, which is precisely the migration `t2k4` exists to force. CHANGELOG: the moon#959 entry now states that `ZDIFFSTORE` joins the moon#592 cross-shard write guard and shares moon#962's `(10, b'z')` arm. Verified on macOS (aarch64): `cargo clippy --all-targets -- -D warnings` clean; `cargo check --no-default-features --features runtime-tokio,jemalloc --all-targets` clean. Tests, release-fast: lib 5608 passed, plus `two_key_write_cross_shard` 5/5 (t2k1's 180 live placements at `--shards 4` now include `ZDIFFSTORE`), `multikey_read_cross_shard` 3/3, `zset_read_cold_tier_928` 5/5, `workspace_key_walker_668` 6/6, `tracking_movablekeys` 2/2. No behaviour from either side was dropped. One lib test is red and it is PRE-EXISTING, not merge-induced: `scripting::bridge::tests::gate_is_skipped_with_spill_sender_when_no_limit_is_ configured` fails 3/3 in the full suite and passes alone — its own doc comment says `MAXMEMORY_GLOBAL`/`DB_MAXMEMORY_ANY_SET` are process-global and other tests in the binary publish them. A/B settles it rather than argument: `0cfab03c` (origin/main, untouched) fails the SAME test, 5586 passed / 1 failed; this merge is 5608 passed / 1 failed, the +22 being moon#959's own unit tests, and skipping all 22 still leaves it red. This merge does not touch `src/scripting/` or `src/storage/eviction` at all (`git diff origin/main -- src/scripting/` is empty). author: Tin Dang
main advanced to 43ad387 while the previous merge was being verified. #1000 touches `src/shard/spsc_handler.rs`, `src/shard/event_loop.rs` and the two connection handlers, plus a new `tests/cross_shard_command_telemetry_982.rs`; none of that overlaps moon#959's sorted-set work or the cross-shard write guard, so the merge is conflict-free. Conflict-free is not correct, so it was re-verified rather than assumed: * `clippy --all-targets -- -D warnings` clean, and the crate really was re-checked (325 rmeta artefacts rewritten, not a cached "Finished"). * `cargo check --no-default-features --features runtime-tokio,jemalloc --all-targets` clean. * `scripts/test-consistency.sh` is +121/-0 and `scripts/test-commands.sh` +39/-0 against main — pure additions, both `bash -n` clean, so #1000's 61 new rows and moon#959's block coexist. * `two_key_write_cross_shard` 5/5, `cross_shard_command_telemetry_982` 5/5, `multikey_read_cross_shard` 5/5, `zset_read_cold_tier_928` 2/2, lib 5609 passed. ## The ZDIFFSTORE guard, proved on the MERGED arm rather than the PR's The previous commit's argument was that `ZDIFFSTORE` must stay in `touches_a_key_it_did_not_route_on`'s `(10, b'z')` arm, which it now shares with moon#962's `ZINTERCARD`. That argument was re-run as an experiment against this tree, because a guard nobody has seen fail is not a guard: - (10, b'z') => { - cmd.eq_ignore_ascii_case(b"ZINTERCARD") || cmd.eq_ignore_ascii_case(b"ZDIFFSTORE") - } + (10, b'z') => cmd.eq_ignore_ascii_case(b"ZINTERCARD"), t2k1 ... FAILED 12/180 placements broke the acknowledgement contract (moon#592): ZDIFFSTORE [t2k:ZDIFFSTORE:0:s -> t2k:ZDIFFSTORE:0:d2]: ACKED ":0\r\n" but the destination does not hold the write: ZCARD answered ":0\r\n", expected ":2\r\n" — this is acknowledged data loss That is a live `--shards 4` server acknowledging a destructive write that landed nowhere — the exact defect main's doc comment asserted could not happen because the command was unimplemented. The one-line edit was then restored from a copy (not `git checkout`) and the suite is green again; `git diff -- src/server/conn/shared.rs` is empty. ## Correction to the previous commit message It mis-attributed four per-suite counts: cargo's `Running`/`test result` lines were read from two separate greps and paired wrongly. The authoritative pairing is `multikey_read_cross_shard` 5, `tracking_movablekeys` 3, `two_key_write_cross_shard` 5, `workspace_key_walker_668` 6, `zset_read_cold_tier_928` 2. No conclusion changes — every suite was green either way — but the numbers as written were not measured. ## Known-red, pre-existing, not from this branch * `scripting::bridge::tests::gate_is_skipped_with_spill_sender_when_no_limit_ is_configured` — fails in the full lib suite, passes alone. A/B: `0cfab03c` (main, untouched) fails it too, 5586 passed / 1 failed. This branch does not touch `src/scripting/`. * `admin::footprint::footprint_tests::footprint_is_phys_footprint_not_resident _size` — failed once, under two concurrent cargo builds, and passed on the runs before and after. It samples macOS `phys_footprint`; it is load sensitive, not a merge effect. author: Tin Dang
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #959.
Six sorted-set commands answered
ERR unknown commandandZADD ... INCRanswered an arity error.
docs/commands.md:52advertisedZRANGEBYLEXallalong, and neither parity harness named any of them. Every reply below was
read off a live redis-server 8.6.1 on the wire before the code was
written — never from
COMMAND INFO.What changed
ZRANGEBYLEX,ZREVRANGEBYLEXsorted_set_lex.rsdispatch,dispatch_readand the read prefilterZREMRANGEBYRANK/SCORE/LEXsorted_set_write.rszrem's two-arm shape: listpack trimmed in place and never converted, B+tree credited exactly, drained key deleted; newrank_windowhelper followszremrangeGenericCommandZDIFFSTOREsorted_set_store.rs(the store family moved out of the write half when it crossed 1500 lines)SetOparm ofzstore_impl, so it inherits #991's numkeys classes;WEIGHTS/AGGREGATEaresyntax erroras on redisZADD ... INCRsorted_set_write.rszincrbyrefactored intozincr_member(flags);zsetAdd's decision order for NX/XX/GT/LT; bulk-string score or nilRegistered in the
phftable as@sortedsetwith redis's arities.Two family-wide corrections that rode along (one function, now shared
by three commands): the store family reads its sources before parsing
options, so
ZUNIONSTORE d 1 <string> BOGUSisWRONGTYPEas on redis(#991 had made it
syntax error); and it reads them throughget_sorted_set_ref_if_alive, so a listpack source is no longer flattenedto
skiplistby being read.The one
shared.rscommit — please read0d05221fis the only commit touchingsrc/server/conn/shared.rs. It isconfined to
touches_a_key_it_did_not_route_on: its doc block (theZDIFFSTORE — not implementedbullet) and one match arm directly below it(
(10, b'z') => ZDIFFSTORE). The arm goes one step past "doc block only",and it is not optional: with the arm removed and everything else in place,
tests/two_key_write_cross_shard.rs::t2k1at--shards 4reports12/180 placements acking
ZDIFFSTOREwhile the destination lands nowhere(the moon#592 defect) — that is the attack result below, verbatim. The doc
block itself names the t2k4 tripwire as the thing that forces this
migration.
is_inline_intercepted/INTERCEPTED_NOT_DECLAREDare untouched.If the arm must ship separately, say so and I will split it out.
Evidence
Red — the 119 harness rows (30 in
test-commands.sh, 89 intest-consistency.sh) extracted verbatim from the committed scripts and runagainst the pre-fix control
a8eb2efcnext to redis 8.6.1:112 red / 7 green. 65 reds are
unknown command, 22 are the arity errorZADD INCRused to give, the rest are wrong answers downstream of those.The 7 greens are fence rows that hold either way (
EXISTSof a key nothingcreated,
GETof the WRONGTYPE string).Green — the same 119 rows against this branch's binary: 119/119.
A 301-probe oracle sweep (every happy path and error form I could think of,
plus encodings and
DEBUG DIGEST) leaves 16 differences, every onepre-existing and listed under "out of scope" below; the sweep also caught
one wrong guess of mine (WITHSCORES/bounds precedence), fixed in
10bd1ff0.Unit tests — 22 new in
sorted_set/mod.rs::missing_commands_959_tests,22/22; the broader
sorted_set::+command::tests+metadata::set225/225;
zset_read_cold_tier_928(now with the two lex reads) 5/5;two_key_write_cross_shard5/5 withZDIFFSTOREinPROBES.Attack (apply, build, read, restore,
git diff HEAD --stat= 0 files,rebuilt green):
rank_windowmade to clamp a still-negative stop →rank_window_follows_the_redis_ruleandzremrangebyrank_normalises_ranks_like_redisred.dispatch_read's(14, b'z')arm emptied →dispatch_read_serves_the_lex_readsred (ZREVRANGEBYLEX came backunknown commandon the read path).zrangebylex_error_surface_matches_the_oraclered.ZDIFFSTOREdropped from the cross-shard write guard →t2k1red:12/180 placements broke the acknowledgement contract ... ZDIFFSTORE ACKED ":0" but the destination does not hold the write.All four attacked trees compiled; the tests, not the compiler, caught them.
Dispatch-path coverage, per command (tested vs reasoned):
command::dispatch— every new command has tests through the realdispatch()(call(...)in the module) and wire rows on a live server.command::dispatch_read—ZRANGEBYLEX/ZREVRANGEBYLEXtested throughdispatch_read()and the prefilter (dispatch_read_serves_the_lex_reads). The five write commands areW-flagged and never reach this path (reasoning:is_read/prefilter route on the registry flag; a wire row on a bare connection exercises the real routing).server::conn::try_inline_dispatch— reasoning only: it inlines exactlyGETand a plainSET(blocking.rs, the[G,E,T]/[S,E,T]match); every other command falls through to generic dispatch. No test, because there is no arm for a zset command to be missing from.Gates (exit codes):
cargo fmt --check0 ·cargo clippy --all-targets -- -D warnings0 ·cargo check --all-targets --no-default-features --features runtime-tokio,jemalloc0 · release-fast build 0. Not run:scripts/ci-local.sh, the hosted matrix, any Linux/VM leg, the fullscripts/test-*.shscripts end to end (their moon#959 blocks were run verbatim against both binaries instead).Found while reproducing, NOT changed here
ZRANGE z -10 -6answers [a] where redis answers [] #1001 (filed):ZRANGE z -10 -6(alsoZREVRANGE,ZRANGE ... REV,ZRANGESTORE) answers[a]; redis answers[]—zrange_by_rankclamps a still-negative stop.rank_windowdoes not, soZREMRANGEBYRANKis right.ZRANGEBYSCORE nokey a b/ZRANGE nokey a b BYSCORE|BYLEXreply*0; redis replies the bound error (range parsed before lookup). In ZRANGE/ZREVRANGE/ZRANGESTORE clamp a still-negative stop to 0:ZRANGE z -10 -6answers [a] where redis answers [] #1001.ZUNION 1 <string> BOGUS/ZDIFF 1 <string> BOGUSreplysyntax error; redisWRONGTYPE— the read-only family still parses options first.ZADD k NX XX(no pairs) replies the NX/XX error; redissyntax error. fix(sorted_set): reject GT+LT and NaN weights, report syntax errors as syntax errors, and count CH exactly #991 territory.ZADD ... 1e400accepted asinf,1e-7rendered0.0000001,-0kept — all Zset scores diverge from Redis on the wire: no exponent form, -0 preserved, 1e400 silently saturated to inf, empty bound rejected #968.ZINCRBY/ZADD INCRreply a bulk string under RESP3 where redis replies,double. Pre-existing forZINCRBY; kept consistent.skiplistwhere redis builds alistpack(OBJECT ENCODINGof aZ*STOREresult). Pre-existing forZUNIONSTORE.