Request response policies#2996
Draft
nkaradzhov wants to merge 51 commits into
Draft
Conversation
nkaradzhov
force-pushed
the
request-response-policies
branch
2 times, most recently
from
June 17, 2025 08:14
e74bb9a to
2ea355f
Compare
nkaradzhov
force-pushed
the
request-response-policies
branch
from
June 9, 2026 08:18
fd526fa to
e857c1b
Compare
nkaradzhov
force-pushed
the
request-response-policies
branch
3 times, most recently
from
June 30, 2026 09:54
a85904d to
25716d1
Compare
nkaradzhov
force-pushed
the
request-response-policies
branch
2 times, most recently
from
July 2, 2026 17:37
55e1732 to
fb6d15e
Compare
nkaradzhov
force-pushed
the
request-response-policies
branch
2 times, most recently
from
July 14, 2026 15:21
c413da5 to
005db4a
Compare
for now we only extract: - request and response policy -> from tips - is the command keyless -> from key specs
actually fetched from server using the dynamic resolver
Move each switch case body in `_execute` into a typed router/reducer function in `request-response-policies/dispatch.ts`. Switches still dispatch by policy enum; behavior unchanged. Prepares for replacing the switches with a strategy registry in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Define `REQUEST_ROUTERS` and `RESPONSE_REDUCERS` as policy-keyed records of the dispatch helpers introduced in the previous commit. The two switches in `_execute` collapse to a single registry lookup each, with `Unknown policy` errors preserved. `satisfies Record<PolicyName, ...>` keeps the lookup exhaustive at the type level. Adding a new policy now requires only a new entry in the registry — no `_execute` changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- `CommandIdentifier.subcommand` is now `string | undefined`, mirroring the fact that single-word commands have no second arg. The parser emits `undefined` instead of an implicit empty value. - `StaticPolicyResolver` lowercases the entire policy table at construction time, so lookups are case-insensitive regardless of the casing used in the static data file (which today mixes upper- and lowercase module/command keys). - Subcommand lookups also lowercase the incoming identifier and guard against `undefined`, so `MEMORY USAGE` resolves to the `usage` subcommand policy regardless of caller casing. - Drop the stray `console.log` left in the resolver. - Error messages that referenced `parser.commandIdentifier` now print a `command [subcommand]` label instead of `[object Object]`. - Update `dynamic-policy-resolver.spec.ts` to pass `CommandIdentifier` objects to `resolvePolicy` (previously was passing raw strings). - Add `static-policy-resolver.spec.ts` covering FT.SEARCH, MEMORY USAGE, GET, COMMAND INFO, FT.SUGADD, casing, errors and fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The static policy table had three problems for the Search module: - The `FT` module entries were upper-cased while every other module uses lower-case keys. - A second `search` module held three RediSearch cluster-admin commands (`CLUSTERSET`, `CLUSTERINFO`, `CLUSTERREFRESH`) that are not part of the public FT.* surface. - A `_FT` module held private debug subcommands (`_FT.CONFIG`, `_FT.DEBUG`, etc.) and a long list of deprecated / private FT.* entries (`_LIST`, `_CREATEIFNX`, `_DROPIFX`, `_DROPINDEXIFX`, `_ALIASADDIFNX`, `_ALIASDELIFX`, `_ALTERIFNX`, `ADD`, `DEL`, `GET`, `MGET`, `SYNADD`) which are not user-facing and shouldn't participate in policy resolution. Replace those three modules with a single lower-case `ft` module containing exactly the 25 commands listed in the HLD Command Routing Policy Table, each with the policy the client should apply per the HLD client-interpretation column. `ft.cursor` is left at `default-keyless`/`default-keyless` for now; the HLD specifies `special` (sticky cursor) but that depends on the not-yet-built special-handler registry and cursor binding map. Add `ft-policies.spec.ts`, a parameterised test that asserts every HLD command resolves to the prescribed `request`/`response` pair and that the dropped debug / admin commands no longer resolve. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- extract DynamicPolicyResolverFactory.buildModulePolicyRecords so the generator and the runtime resolver share one derivation path - scripts/generate-static-policies-data.ts (npm run generate:policies -- <redis-url>) regenerates static-policies-data.ts from a live COMMAND reply; keys lowercased and sorted for stable diffs - HLD curation codified in scripts/static-policies-overrides.ts: internal/deprecated FT commands and _ft/search cluster-admin modules excluded, ft.cursor pinned to default-keyless until the special-handler registry lands - data regenerated against Redis 8.8.0: adds 42 new std commands (msetex was missing entirely), json.debug flips to keyless per server-reported key specs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
COMMAND tips carry no ordering guarantee; commands like INFO declare nondeterministic_output before their policy tips, so positional parsing silently dropped request/response policies for 13 commands (scan, info, memory/latency subcommands, function stats, cluster slot-stats, hotkeys get) and they fell back to default routing. Scan the whole tips array by prefix instead and regenerate static-policies-data from Redis 8.8.0. Surfaced 'special' policies now reach the throwing routeSpecial / reduceSpecial handlers on cluster dispatch; safe fallback deferred.
getAllMasterClients/getAllClients silently skipped nodes without a connected client. With minimizeConnections (or a partially connected cluster) all_shards/all_nodes commands fanned out to a subset of nodes — or none, resolving undefined: PING via sendCommand returned undefined, DBSIZE could sum a subset without any error. Route through nodeClient(), the same lazy connect-or-reuse accessor keyed routing uses. getAllClients no longer yields dedicated PubSub connections; they cannot run regular commands. Also guard the policy dispatch against an empty fan-out — failing loud beats resolving undefined.
Make the multi_shard request policy actually split commands per hash slot instead of sending the full command to one client per key. - Routers now return a plan (RoutedCommand[]) rather than bare clients: pass-through policies set `client`; routeMultiShard calls the splitter and builds a sub-parser per slot (buildSubParser marks keys at the splitter's new keyPositions so `firstKey` routes each sub-command). - The engine runs each plan entry through core `_execute` with its own sub-parser, so every shard gets its own sub-args; MOVED/ASK use the sub-command's key. - Response reducers gain optional positionHints; reduceDefaultKeyed scatters split replies by groupIndices so MGET preserves caller key order across slots. Aggregating reducers (agg_sum, all_succeeded) ignore the hint. - Splitter SubCommand now carries keyPositions for sub-parser key marking. Also merge _executePolicyPlan into _executeWithPolicies — the old name implied a plan object that never existed; it is now the single engine (resolve policy -> plan -> execute -> reduce). Router/reducer types are intentionally non-generic (routing sits below the typed command surface; clients are opaque pass-through), erased to the base cluster types instead of `any`; the engine re-narrows the client at the `_execute` boundary. Working end-to-end: DEL/UNLINK/EXISTS/TOUCH, MSET/MSETEX, MGET. Raw sendCommand multi_shard and cluster-docker integration tests remain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_executeWithPolicies routed every command through policy resolution, but
the cluster's resolver (StaticPolicyResolver(POLICIES)) has no fallback,
so any command absent from the policy table resolved to { ok: false }
and the engine threw "Policy resolution error". This broke user-defined
custom commands, scripts/functions whose dispatched name is not in the
table, and module commands the resolver was not built with.
Such commands have no request/response policy and nothing to split or
aggregate. Instead of throwing, fall back to a default policy
(default_keyed when the parser carries keys, else default_keyless) and
route through the existing pass-through default router/reducer — the
same single-client, key-routed behaviour as a direct _execute. Known
module commands the dynamic resolver does recognise still get their real
policy, so a module declaring multi_shard keeps working. Known commands
that cannot be split still throw from the splitter.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The policy refactor folded the raw cluster sendCommand into _executeWithPolicies but built its parser wrong, regressing behaviour master got right by routing on the explicit firstKey: - firstKey was prepended to args, so redisArgs[0] became a key instead of the command name (breaking policy resolution) and no key was ever marked, leaving parser.firstKey unset so routing fell back to undefined. - makeFn ignored the per-entry sub-parser and always sent the original args, so a split multi_shard command sent the full command to every shard. Build redisArgs as an exact copy of args (command name at index 0) and register the caller's firstKey via a new BasicCommandParser.markRoutingKey, which records the key for routing without re-appending it to redisArgs. Send p.redisArgs from the closure so split sub-commands each carry their own slot's arguments. A known multi_shard command sent raw now splits correctly, since the resolver supplies its key specs and the splitter operates on the flat redisArgs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the throw-on-`special` behavior in the cluster policy dispatch with a per-command reducer registry plus a safe generic fallback: - routeSpecial: route to a single node + console.warn instead of throwing, so an unhandled special-request command still works (reply may be partial). - reduceSpecial: dispatch by uppercased command identifier to a dedicated reducer; unhandled commands fall back to the default-keyless reduction (sole reply / merge) with a warn. - reduceRandomKey (RANDOMKEY): fan out per all_shards, return one non-nil reply at random; all-empty -> nil. Avoids false-nil on empty shards. - reduceFirstReply: for fan-out diagnostics with a single-node reply type (INFO, MEMORY DOCTOR/MALLOC-STATS/STATS, FUNCTION STATS, LATENCY DOCTOR/GRAPH/HISTOGRAM/HISTORY/LATEST) — fan out per the request tip, return one node's reply so the reply type stays honest. FT.CURSOR sticky routing and SCAN/HOTKEYS remain unhandled (fall through to the safe generic fallback) — tracked as separate work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FT.CURSOR READ/DEL carry no key, so hash-slot routing can't reach the coordinator that minted the cursor via FT.AGGREGATE ...WITHCURSOR and the server rejects the unknown cursor. Flip ft.cursor to the HLD `special` request policy and add client-side sticky machinery: - cluster-slots: per-instance cursor binding registry ((index,cursorId) -> address) with bind/lookup/evict + opportunistic idle sweep, plus nodeAddressByClient reverse-lookup. - ft-cursor: routeFtCursor (pin bound node / throw MISS before any network call), SPECIAL_REQUEST_ROUTERS, extractCursorId, and the captureCursorBinding post-reply hook (AGGREGATE bind / READ rebind-refresh-evict / DEL evict). - dispatch: routeSpecial short-circuits into SPECIAL_REQUEST_ROUTERS, keeps the warn-and-random fallback for unregistered special commands. - _executeWithPolicies: best-effort capture hook after the reply resolves; _execute untouched. - static policies: override ft.cursor -> special (+ READ/DEL subcommands, response stays default-keyless), regenerated data table. Response stays single-node pass-through (no special reducer). Bindings are per client instance; cross-instance/cross-process cursors MISS by design. Unit tests cover router/capture/lifecycle/collision; cluster integration tests cover pagination-to-completion, DEL->READ MISS, and cross-instance MISS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…MAND metadata
Several Command fields hardcode information the Redis server already reports via
COMMAND, and the hand-maintained values had drifted across the hundreds of
command definitions (writes marked read-only, read-only keyed commands never
marked cacheable, zrange* cacheable but zrevrange* not, ...). Derive them
instead from a generated metadata table sourced from the live COMMAND reply,
keeping the hardcoded fields only as a fallback for user scripts/functions and
for the handful of commands intentionally excluded from the table (no breaking
change to the Command type).
- Move the policy table out of cluster/ into a shared lib/command-metadata/
module (COMMAND_METADATA, StaticMetadataResolver, generator + overrides), now
that it is consumed by CSC and routing alike.
- Store the raw server signals (flags, tips) in the table instead of precomputed
booleans, so the static table and a future dynamic live-COMMAND resolver feed
the identical derivation.
- Add isReplicaSafe / isCacheable predicates that own the derivation plus the
resolve-then-fallback to Command.IS_READ_ONLY / Command.CACHEABLE:
- replica-safety is the negation of the server `write` flag, matching the
server's own read-only-replica gate in processCommand, NOT the broader
`readonly` flag whose definition is not 1:1 with replica routing.
- CSC eligibility follows the cross-client Command Eligibility algorithm:
readonly flag, takes a key argument, no nondeterministic_output tip, no
script/script_runner flag, no dont_cache tip.
- Rewire the cluster, sentinel and CSC readers to the predicates; sentinel
resolves once per command via a closure memo.
- Add a narrow dont_cache override for TOUCH (the server still doesn't tag it);
EVAL_RO/EVALSHA_RO/FCALL_RO are excluded by the server `script_runner` flag
and TS.READ by its native `dont_cache` tip, so no overrides are needed there.
Overrides shallow-merge onto the generated entry.
- Deprecate the now-unused NOT_KEYED_COMMAND / IS_FORWARD_COMMAND type fields
and remove the hardcoded IS_READ_ONLY / CACHEABLE / NOT_KEYED_COMMAND values
from the built-in command definitions in client, bloom, json, search and
time-series (259 files). Commands excluded from the table (FT._LIST,
FT.CONFIG, FT.HYBRID) keep their hardcoded values as the fallback.
The full hardcoded-vs-derived behavior delta was audited against Redis 8.10
(55 replica-safety changes, 90 cacheability changes, incl. the EVAL/EVALSHA/FCALL
replica-routing change accepted by keeping the write-flag rule pure).
Metadata regenerated against Redis 8.10 (dev build) with all bundled modules loaded.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Of the commands the server tips `special`, only three have a meaningful client-side interpretation; implement the remaining two and revert the rest to master's behavior: - SCAN: cluster-wide iteration behind a client-minted virtual cursor. The router pins the chain's current master (substituting the real per-node cursor) and a post-reply hook advances the chain to the next unvisited master, so the usual `cursor \!== "0"` loop now walks the whole cluster instead of hitting a random node per call. Chain state lives on cluster-slots with the same idle sweep as FT.CURSOR bindings; visited nodes are tracked by address so topology changes mid-scan neither rescan nor wedge. Unknown/expired cursors throw with a "restart from 0" hint. - RANDOMKEY: all_shards fan-out reduced to one non-nil reply at random — never a false nil while any shard holds keys. - INFO, MEMORY DOCTOR/MALLOC-STATS/STATS, LATENCY diagnostics, FUNCTION STATS and HOTKEYS have no reply merge that produces a single honest value, so they are pinned back to default-keyless (single random node, sole reply passed through — master parity, no warn) via generator overrides. The override merge now deep-merges subcommands so siblings (memory purge/usage, latency reset, hotkeys help) keep their server-derived policies. The SPECIAL_REQUEST_ROUTERS registry moves from ft-cursor.ts to dispatch.ts (routers stay leaf modules, no import cycle), and special router/reducer lookup falls back to the bare command name because SCAN's second argument is a cursor, not a subcommand. The dead per-command special response reducers (reduceFirstReply and the INFO/MEMORY/LATENCY/FUNCTION entries) are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nkaradzhov
force-pushed
the
request-response-policies
branch
from
July 16, 2026 16:26
a745ca1 to
b3371b2
Compare
…tadata
Table hits discarded declared intent: defineScript({ IS_READ_ONLY: false })
write scripts routed to replicas (EVALSHA carries no write flag), sentinel
flipped SHUTDOWN/FAILOVER/HELLO to replica routing, and the explicit master
pin in sendCommand(key, false, ...) was ignored.
Defined Command.IS_READ_ONLY/CACHEABLE (or the raw sendCommand isReadonly
argument) always wins; undeclared intent derives from the table: no write
flag, no script_runner flag, and keyed. Keyless flagless commands
(admin/connection/pub-sub) default to master — the flag space cannot separate
ACL GETUSER from ACL SETUSER — so read-ish ones opt back in via restored
IS_READ_ONLY: true (66 audit-confirmed files + EVAL_RO/EVALSHA_RO/FCALL_RO +
FUNCTION LIST). TOUCH declares CACHEABLE: false directly; the generator
override file is now table-shape/policy curation only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Numeric reducers (agg_sum/min/max/logical_*) validate for numbers, but per-node replies arrive already type-mapped, so NUMBER: String broke DEL, EXISTS, TOUCH, UNLINK (agg_sum even single-slot), DBSIZE, WAIT, and SCRIPT EXISTS with 'All replies must be numbers' errors. Strip the caller's type mapping from per-node executions of numeric-agg plans and re-apply it to the aggregated result (remapAggregateReply: scalar + element-wise arrays), matching standalone reply shapes. Pass-through policies keep the per-node mapping. Aggregation runs in JS number space, so NUMBER: String does not preserve >2^53 precision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Splitting MSETEX broke its atomic contract: NX/XX is all-or-nothing across ALL keys but each shard evaluated the condition against its own subset (partial writes), and all_succeeded returned responses[0], masking another shard's 0 as success. Curate MSETEX out of multi_shard (std.msetex override, default-keyed) — consistent with the server's own exclusion of MSETNX. Single-slot calls keep full server-side atomicity and an honest reply; cross-slot calls surface the server's CROSSSLOT error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Server cursor ids are minted per node, so two shards can mint the same
id for one index; the '${index}:${cursorId}' binding key let the second
bind overwrite the first and route a READ to the wrong node — worst
case silently reading another query's result stream.
Mirror the cluster-wide SCAN design: swap the cursor id in FT.AGGREGATE
WITHCURSOR / FT.CURSOR READ replies for a token from the client's own
sequence (collision-free), bind token -> (address, real id), and
rewrite the token back to the real id on the wire. Real ids are kept as
strings, so uint64 cursors no longer lose precision above 2^53
client-side. READ rebinds now carry MAXIDLE through instead of
downgrading to the 300s default, and MAXIDLE 0 maps to the default TTL
instead of expiring instantly.
Tokens are per client instance and not portable to other clients or
redis-cli mid-stream — same property as SCAN's virtual cursors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
multi_shard routing makes cross-slot mGet succeed; assert ordered reassembly and null placement instead of the old rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
routeDefaultKeyless and the special-policy fallback read .client\! off the random node, which is undefined under minimizeConnections until first use — the plan then carried no client and the FT cursor post-reply hook could not bind the serving node. Resolve through nodeClient(), which connects on demand. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…egateError Promise.any wraps the all-rejected case (e.g. SCRIPT KILL with nothing running) in AggregateError, hiding the Redis error users expect; rethrow the first shard's error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Seeded with 1s, the OR of all-zero shard replies aggregated to all ones; it also crashed on an empty replies array. Seed with 0s and add the same array-of-numbers validation as the AND sibling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
getAllClients/getAllMasterClients/getRandomNode skipped #assertReady, so policy-routed commands on a closed or offline cluster surfaced a generic 'produced no target nodes' error (or a TypeError) instead of ClientClosedError/ClientOfflineError like every other path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
transformCommandReply destructured and iterated reply fields 8-10 (tips, key specs, subcommands — added in Redis 7.0) unguarded, so client.command()/commandInfo() against older servers or truncating proxies threw 'tips is not iterable'. Default the missing fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
routeFtCursor/routeScan indexed redisArgs positionally, so a raw sendCommand missing the cursor argument threw a client-side TypeError before any I/O; forward short commands to a node so the server's own arity error surfaces, as on master. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The policies refactor dropped the { ...options, slotNumber } per-attempt
options master passed to the node client, so queued commands carried no
slot metadata and extractCommandsForSlots could not relocate them to the
destination node during an SMIGRATED maintenance event. Pinned plans
(default-keyed pins, multi_shard sub-commands) derive the slot from the
parser's first key; keyless fan-outs carry none.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The server does not tag the vector-set family with nondeterministic_output (unlike SRANDMEMBER/HRANDFIELD/ZRANDMEMBER), so the derived metadata marked VRANDMEMBER cacheable and CSC froze the random pick until the key changed. Override with CACHEABLE: false until the server tags it; the rest of the V-family is deterministic for a given data state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_executeCommand ran the command-identifier decode and metadata lookup on every command even without a client-side cache; gate it behind csc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The getter decodes Buffer args and allocates on every read; cluster commands read it up to three times (policy resolution plus both post-reply hooks). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every single-key command paid the full plan/reducer/positionHints/ post-reply-hook machinery (~12-20 allocations and extra promise hops vs master) although all of it is a no-op for the default-keyed shape. Route by firstKey and pass the sole reply through directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
After a MOVED/ASK redirect the reply comes from a different node than plan[0].client, so cursor bindings could pin the stale original target. _execute now records the client that served each attempt; single-target plans pass it to the finalize hooks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
command-router.ts (fully commented out), the assertion-free test.spec.ts docker suite, the readonly-discrepancies audit artifacts (fully mined into command definitions and the deviations ledger), and a stray TODO in the lua-multi-incr example. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dary The MOVED/ASK attribution cast compared this instantiation's client type against the plan's erased base client type; a clean (non- incremental) tsc build rejects both the comparison and the direct cast. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… API - five reduce* wrappers collapse into a reduceWith lift - command-name uppercase gates share upperCommand (dispatch, ft-cursor, scan-cursor) - module/command dot-split shares parseCommandName (resolver + dynamic factory) - default-policy synthesis shares defaultCommandPolicies (cluster fallback + dynamic factory) - PolicyResolver.withFallback removed (zero production callers; the constructor fallback stays) along with the two never-produced PolicyResult error variants Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- aggregateMerge handles plain-object replies (RESP3 maps under the default type mapping) instead of throwing after all nodes executed - the response reducer is validated before any per-node promise is dispatched, so an unknown policy no longer orphans in-flight rejections - resolver lookup tables are null-prototype: command names like 'constructor' or '__proto__' miss the table instead of resolving an Object.prototype member as metadata Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Description
Checklist
npm testpass with this change (including linting)?