Preparation work for smart-client-handoffs (#3174)
* fix unknown PUSH handling
* Toy server: node identity groundwork for endpoint reconciliation
Test-only; prerequisite for the endpoint-identity work (#2826) and for
reacting to endpoints we did not choose the form of.
- node aliases, resolved through TryGetNode, so one node answers to
several names; kept in their own map rather than as extra _nodes
entries, since EndPointComparer cannot order mixed endpoint types and
AddEmptyNode derives the next endpoint's form from an arbitrary key
- CLUSTER NODES emits ip:port@cport[,hostname]; the cport was previously
string-concatenated as "@1" + port, which is accidentally right for
four-digit ports and wrong from 10000 up
- per-node cluster-preferred-endpoint-type, with the complement metadata
map in CLUSTER SLOTS and the prescribed placeholder endpoint values
(null, "", ?); ? is derived from hostname-preferred-but-unannounced
rather than being settable, so combinations the real server cannot
produce cannot be expressed
- gated on a 7.0 server version: below that the preference is inert, the
hostname is not emitted, and the SLOTS node block is three elements
- CONFIG GET reports the answering node's view of the announce settings,
which costs CLUSTER CONFIG GET its LockFree status
- AddEmptyNode(EndPoint) overload, since the existing one copies the
form of an existing key
Verified against a real 8.9.241 cluster at RESP2 and RESP3: the
preference belongs to the *answering* node and governs every entry in
its reply, while hostname availability is per described node - so asking
a hostname-preferring node about a peer with no hostname yields ?.
Also tightens the in-process TLS harness: the certificate is built on
first use so it can cover aliases registered after construction, and
validation no longer forgives a name mismatch on thumbprint alone.
* ClusterNode: expose the endpoint trailer instead of truncating it
CLUSTER NODES reports "ip:port@cport[,hostname[,aux-field=value]*]" since
4.0, and we discarded everything from the '@' onwards. The text was never
lost (ClusterNode.Raw has it) but the parsed view did not offer it, and
the hostname is a node's second identity - the thing endpoint
reconciliation needs (#2826).
Additive on ClusterNode:
- ClusterBusPort (int?), null on pre-4.0 lines that report no cport
- Hostname (string?), null when unannounced or the slot is empty
- AuxFields, preserving unrecognized keys since the set is documented as
extensible
No named ShardId/HumanNodename properties: neither is observable on a
real 8.9 server (cluster-announce-human-nodename does not appear in the
reply at all), so naming them would mean shipping API for fields we
cannot demonstrate. A named accessor stays additive if that changes.
Parsing is lenient by design - an exception here does silent damage to
topology, per the warning on the constructor - so an unparseable cport
yields null and aux entries without '=' are skipped.
Toy server gains SetAuxField, and CLUSTER NODES now handles the
positional subtlety that the hostname slot can be empty while aux fields
follow ("@17000,,shard-id=abc").
Tests: grammar cases against the parser directly, realistic shapes round
tripped through the toy server, and end-to-end against a real cluster
with version gates (4.0 for the bus port, 7.0 for hostnames) - including
setting cluster-announce-hostname and restoring it. Those run per
protocol, so they also cover CLUSTER NODES arriving as a RESP3 verbatim
string, which had no end-to-end coverage before.
* Add CLUSTER SLOTS: object model, parser, IServer surface, export
The second topology view, and a prerequisite for endpoint reconciliation
(#2826): CLUSTER SLOTS is the only source that conveys which naming form
a deployment prefers, and it carries the node-id, which is the one
identity that does not depend on who is answering.
Laid out per the HotKeys precedent rather than growing the existing
files: ClusterSlots.cs (IServer partial + model),
ClusterSlots.ResultProcessor.cs (the parse, on RespReader),
ClusterSlots.Server.cs (RedisServer implementation).
Model: ClusterSlotsResult.Assignments -> ClusterSlotAssignment{Slots,
Primary, Replicas} -> ClusterSlotNode. Deliberately lossless:
AnnouncedEndpoint keeps the literal the server sent, Metadata keeps
unrecognized keys, and EndPoint is populated only when the reported value
is usable as one.
The three placeholder endpoint values are encoded rather than merely
documented: null ("the server does not know this node's address"), ""
("the node does not know its own") and "?" ("an unknown node") all yield
a null EndPoint, since substituting the connection's own address is a
caller decision - and is outright wrong for "?". Metadata is walked
pairwise rather than by declared length, so the RESP3 map and the RESP2
flat array need no special casing.
ExportConfiguration now writes slots.txt alongside nodes.txt, one line
per node per range so it greps like nodes.txt; ExportOptions.Cluster
covers it (comment updated), and ExportOptions.All is -1 so it is
included by default. Rendered from the model rather than dumped as RESP:
users do not want to read RESP, CONFIG GET already sets the precedent for
rendering a structured reply as flat text, and the rendering is faithful
because the model is lossless.
Tests: the naming matrix against the toy server (ip/hostname/unknown
preference, unannounced hostnames, both address placeholders, pre-7.0
three-element node blocks, migrated slots, node-id stability across
forms), plus real-cluster coverage at both protocols for keyspace
coverage, replicas, the export, and SLOTS/NODES agreeing on primary
node-ids - which is the premise id-keyed reconciliation rests on.
* Fault unroutable -MOVED/-ASK targets instead of dialling "?"
A hostname-preferring node redirecting to a peer that has announced no
hostname reports "MOVED <slot> ?:<port>", and "?" denotes an *unknown*
node - so unlike a missing or empty endpoint it cannot be resolved to the
node that answered. Reachable on any OSS cluster with
cluster-preferred-endpoint-type=hostname and a node whose
cluster-announce-hostname is unset; most plausibly one that had it set via
CONFIG SET without CONFIG REWRITE and was then restarted.
Demonstrated before fixing, and it was worse than a bad message:
Format.TryParseEndPoint accepts "?" as a hostname, so TryResend called
GetServerEndPoint(endpoint) - which defaults to activate:true - and thus
created and dialled a ServerEndPoint for a host literally named "?". The
phantom stayed in GetEndPoints() with a live connect loop, the command sat
in the backlog for the full async timeout, and the caller got a timeout
exception recommending they check connectTimeout, with a link to the
timeouts troubleshooting article.
Now: unusable targets are rejected before any endpoint is created ("?", an
empty host, or port 0 - the last covering the unknown-endpoint form, which
parsed to the nonsense DnsEndPoint(":<port>", 0)); a topology refresh is
still requested, which genuinely recovers, since CLUSTER NODES reports
addresses positionally whatever the preference; and the command faults
immediately with a message naming the cause and quoting what the server
actually sent.
New RedisErrorKind.UnknownRedirectTarget, appended so existing values keep
their numbers, and documented as a client-side classification rather than a
server error code. Included in FaultContext's known-not-applied set
alongside Moved/Ask: the redirect proves the command did not run, so a
retry is a first attempt rather than a repeat, which is what lets WithRetry
recover once the refresh has landed.
* Shadow the CLUSTER SLOTS topology alongside CLUSTER NODES
Builds an internal id-keyed view of the cluster from CLUSTER SLOTS and
records it during autoconfigure, without routing on it. The point is to
have an oracle: each subsequent step (slot map, then discovery, then
pruning) can be taken with the two views still demonstrably agreeing.
Keyed on node-id because it is the only identity in a reply that does not
depend on which node answered - names vary with the answering node's
preferred endpoint type, so keying on a name means one node can produce two
entries. Keyed on the id they merge, and the names become attributes.
Identity accumulation is the union of the primary endpoint field and the
metadata complement, and the primary is classified by *content* rather than
position: a first cut read only the metadata and consequently lost whichever
form the answering node happened to prefer, which the tests caught.
Tests assert the shadow view agrees with the routing view on primary
node-ids and on slot ownership per node - as sorted slot sets, so differing
range fragmentation between the two views is not mistaken for disagreement.
Covered against the toy server across naming configurations, and against a
live six-node cluster whose ownership is heavily fragmented, which is the
stronger check on range boundaries.
* Do not invoke CLUSTER SLOTS from autoconfigure yet
The ClusterTopology model and SetClusterSlots stay - they are the id-keyed
view the routing work needs - but the autoconfigure request is commented out
with the reason in place.
This PR is scoped to work that cannot destabilise a connection, and asking
every server for an extra command on every autoconfigure is a new failure
surface on the connect path (an unexpected error reply to an internal call, or
a proxy that mangles the command) for no user-visible benefit until routing
consumes it. It is enabled in the follow-up, where ordering also matters: the
request must precede CLUSTER NODES so identities are known before NODES starts
creating servers by address.
The topology tests consequently source the reply explicitly via the public
ClusterSlotsAsync rather than relying on autoconfigure, so they cover the model
and the parser here; the wiring is covered where it is switched on.
* Recognize CLUSTER SLOTS metadata keys without allocating
The metadata keys come from a fixed vocabulary, so match them over the raw
bytes via the usual [AsciiHash] enum rather than materializing a string per
entry: ClusterSlotMetadataKey plus a case-insensitive generated TryParse, since
the contract renders these keys as IP/Hostname in prose but lower-case in its
examples and the casing cannot be relied on.
Recognized keys are surfaced as ClusterSlotNode.Ip/Hostname and no longer
appear in Metadata, which now holds only what we do not recognize. That is what
makes the saving real - keeping them in a string-keyed list would allocate the
key anyway - and it matches the ClusterNode.Hostname/AuxFields precedent, where
the hostname likewise is not repeated in the collection.
Worth more than it first appears: ranges repeat a node whenever its slot
ownership is not contiguous, so this is one allocation avoided per key per
*range*, not per node - on the live test cluster that is 43 ranges rather than
6 nodes.
Toy server gains SetSlotsMetadata so the forward-compatibility path is actually
exercised: an unrecognized key must be preserved as declared, and an
upper-case known key must still be recognized rather than landing in Metadata.
* Report IReadOnlyList rather than IList on the new cluster members
IList advertises Add/Remove/set that throw at runtime on the collections we
actually hand back, which is a lie in the type system. The only argument for it
was matching ClusterNode.Slots/Children, and those are shipped so they cannot
move; every member changed here is unshipped, so consistency-with-a-wart buys
nothing.
Changed: ClusterSlotsResult.Assignments, ClusterSlotAssignment.Replicas,
ClusterSlotNode.Metadata, ClusterNode.AuxFields.
Also unpicks the `x?.AsReadOnly() ?? (IList<T>)Array.Empty<T>()` expressions,
which were dense enough to misread, into a plain null check returning `[]`. No
allocation difference: an empty collection expression targeting an interface
compiles to Array.Empty<T>(), so only a populated list pays for the wrapper.
* Fix Windows CI: colliding toy-server node ids, and a test asserting fleet shape
Two unrelated failures, one real bug.
Node.NewId created a Random *per call*, and .NET Framework seeds Random from
Environment.TickCount - so two nodes constructed within the same tick received
identical 40-character ids. Only reachable on the net481 leg, which is why Linux
never showed it. The failure was maximally misleading: the shadow topology keys
on node-id, so it correctly merged the two lying nodes into one, and the
assertion that broke was the comparison against CLUSTER NODES - the *fake* was
wrong and the client looked wrong. Now one shared Random on that path, plus an
interlocked counter woven into the first eight characters so ids are unique by
construction rather than by luck, with a regression test that creates 26 nodes
back to back.
ClusterSlotsTests.ReplicasAreReported asserted that some assignment reports
replicas, which is a statement about the deployment rather than the parser: the
Windows CI fleet is smaller than the local compose and has none. Skips in that
case instead, so it still exercises the replica parsing wherever replicas exist. M
Marc Gravell committed
ac4850f497168dff882ed3f68d55c5e322d20acb
Parent: 0d9f24f
Committed by GitHub <noreply@github.com>
on 8/14/2026, 3:44:59 PM