Make fallback discovery and keep-alive probes cluster-slot aware

[HarnageaGabriel]

Summary

Fixes #2970.

On OSS Redis Cluster, the internal discovery/keep-alive probe messages sent by ServerEndPoint (the replica_read_only SET fallback, the tie-breaker GET, and the EXISTS tracer fallback) are written directly to a specific node's connection with CommandFlags.NoRedirect, bypassing ServerSelectionStrategy's slot-aware routing. If the probe key's hash slot isn't owned by that node, the server replies MOVED instead of the expected reply, and because NoRedirect is set the client never follows it — so the probe silently fails on cluster.

• AutoConfigureAsync: skip the SET $uniqueid$ replica_read_only PX 1 NX fallback once cluster topology (CLUSTER NODES) already tells us our role — it's both redundant and slot-unsafe there.
• AutoConfigureAsync: skip the tie-breaker GET fallback on cluster, where a tie-breaker key isn't meaningful.
• GetTracerMessage: when ECHO/PING/TIME are all disabled and we fall back to EXISTS, build the key with a hash-tag targeting a slot this endpoint actually owns (reusing the existing ServerSelectionStrategy.HashTags cache), so the probe always lands on a slot the node itself serves.

Standalone/Twemproxy/Envoyproxy/Sentinel behavior is unchanged, as is behavior when cluster topology isn't known yet (falls back to the prior plain-key behavior, matching current best-effort semantics).

Note: this is unrelated to #3175/#2968, which addressed a different problem (ACL key-pattern restrictions breaking the replica-detection probe) — this change is additive to that fix, not a replacement.

Test plan

• dotnet build Build.csproj -c Release — 0 errors, 0 warnings
• New unit tests: HashTagUnitTests.TestHashTagPrefixTargetsSlot (slots 0, 1, 8191, 16383) and ServerEndPointClusterProbeUnitTests (tracer key selection with/without known owned slots) — all pass
• Verified pre-existing/unrelated failures (MultiPrimaryTests.TestMultiWithTiebreak, requires a live standalone/failover Redis server not available in this sandbox) are present identically on main before this change

[mgravell]

Thanks - right diagnosis, and it follows the issue closely; builds clean and the new tests pass locally. A few things before this goes in, structural first.

1. This duplicates the existing InventKey concept

We already have IServer.InventKey(RedisKey prefix) (RedisServer.cs:63) - same idea: make a key that will route to this endpoint. It is used by KeyWriteHealthCheckProbe (Availability/HealthCheckProbe.cs:49) and documented in docs/Failover.md. Right now we would have two implementations of one concept, differing in every detail:

	RedisServer.InventKey	new GetTracerKey
slot source	ServerSelectionStrategy.GetHashTag(endpoint) - an O(16384) scan of the slot map, self-described in the code as "inefficient way"	the node's Slots[0].From
tag placement	suffix: prefix + guid + ":{tag}"	prefix: "{tag}" + uniqueid
cache	HashTags.Cache (strings)	new HashTags.PrefixCache (byte[])
no slot available	RedisKey.Null, callers must check	plain untagged key

The new slot lookup is strictly better than the map scan, so please make one internal primitive and route both through it: RedisServer.InventKey becomes a thin wrapper, and the map-scanning GetHashTag(ServerEndPoint) overload can go.

2. GetPrefix should encapsulate the whole key composition

Something like internal static RedisKey CreateKeyForSlot(int slot, RedisKey suffix) rather than handing out a prefix. As written, GetPrefix returns the cached byte[] and it escapes to callers as a RedisKey (which does not copy), so we are publishing shared mutable state - and the new test pins that with Assert.Same. Encapsulating composition means nothing internal escapes, the cache can hold an immutable string (or be deleted outright) with no API change, and one place decides prefix-vs-suffix so this and InventKey cannot drift.

I would drop the byte[] cache entirely while you are there: it is a 16384-entry static array (128KB, allocated even in processes that never touch cluster) plus a lock, to avoid ASCII-encoding five bytes - and we allocate in Prepend on every call anyway. If you want the heartbeat allocation-free, memoize the composed key on ServerEndPoint and invalidate when the served slot changes; the tracer key is stable per endpoint but is currently rebuilt on every heartbeat.

3. Cluster replicas still get a broken tracer

node.Slots is empty for a replica (CLUSTER NODES reports ranges against primaries only), so GetTracerKey falls through to the plain untagged key - and a replica answers MOVED for a slot outside its primary's range just as a primary does. So in exactly the scenario being fixed here, replica connections are still broken, and the tracer gates connection completion.

A cluster replica can serve reads for its primary's slots, and we already send READONLY there (ServerEndPoint.RequiresReadMode), so for a replica take the tag from the parent's slots. Note ClusterNode.Parent is unusable as written:

public ClusterNode? Parent => (parent is not null) ? parent = configuration[ParentNodeId!] : null;

that returns null whenever the backing field is null, and the field is assigned nowhere else, so Parent is always null (which also silently kills the " at {endpoint}" branch in ToString()). Pre-existing and separate, but it needs fixing regardless; use configuration[node.ParentNodeId] in the meantime.

The same blind spot exists in GetHashTag(ServerEndPoint)/InventKey, which return ""/RedisKey.Null for replicas - harmless for a write probe, wrong once one primitive serves both.

4. Both AutoConfigureAsync guards are inert on the first handshake

serverType starts as Standalone (ServerEndPoint.cs:59) and only becomes Cluster when the CLUSTER NODES reply is parsed (ResultProcessor.cs:1237). AutoConfigureAsync writes its entire batch before any reply lands, so on the first connect ServerType != Cluster and ClusterConfiguration is null: the unslotted SET probe and the tie-breaker GET both still go out and take a silent MOVED (NoRedirect, fire-and-forget). The guards only bite from the second handshake onwards.

Not fatal - the role arrives from CLUSTER NODES moments later - but the description reads as though the probes are suppressed on cluster, and they are not on the path that matters most. Please add a comment saying so, and adjust the description.

Also, in !(ServerType == ServerType.Cluster && GetClusterNode(ClusterConfiguration) is not null): what is the second clause for? If we are cluster, CLUSTER NODES tells us the role whether or not we already hold topology. Drop it or comment why it is there.

Skipping the SET probe rather than slot-tagging it is the right call, and the reason is worth stating in the comment: a cluster replica answers MOVED (not -READONLY) to a write for its primary's slot, so that probe cannot work on cluster at all, tagged or not.

5. Tie-breaker skip: agreed, and there is prior art to cite

NominatePreferredPrimary is inside if (clusterCount == 0), so TieBreakerResult is never consumed on cluster and the GET is pure waste. docs/Configuration.md already documents tie-breaking as "not including redis cluster, where multiple primaries are expected", so this aligns code with documented behaviour rather than changing it - worth saying that in the description. No doc change needed.

6. Tests

• Nothing covers the two AutoConfigureAsync changes, which are the actual behaviour changes here. Assert that the tie-breaker GET and the SET probe are not issued against a cluster endpoint; the in-process server can record received commands.
• No replica case - see (3); that test should fail today, which is the point of adding it.
• ExistsTracerUsesPlainKeyWithoutKnownOwnedSlots hand-builds a ServerEndPoint and never disposes it (it is IDisposable).
• Assert.Same on the cached prefix pins an implementation detail that (2) would remove.
• In that theory the Standalone and Cluster cases exercise nearly the same path (the server is standalone either way); the distinction actually worth covering is "cluster, topology not known yet" - please say that in a comment.

7. Minor

• GetClusterNode should use the endpoint indexer: configuration?[EndPoint] is an O(1) dictionary hit (nodeLookup is keyed by EndPoint), versus a LINQ scan plus closure. It matters more now this is on the heartbeat path, and it improves the existing UpdateNodeRelations too.
• (RedisValue)UniqueId becoming a RedisKey on the EXISTS message is wire-identical, so no compatibility concern; it does now report a hash slot, which is what we want.
• Two files get BOM-only additions (ServerSelectionStrategy.HashTags.cs, HashTagUnitTests.cs). Consistent with the repo rule, but unrelated churn - either call it out or split it off.

[HarnageaGabriel]

Thanks for the detailed review - pushed three commits addressing the structural points:

1. InventKey/tracer unification: RedisServer.InventKey and ServerEndPoint.GetTracerKey now both route through one primitive, ServerEndPoint.GetServableSlot(), which uses the node's own Slots[0].From (no more O(16384) map-scan). Deleted the map-scanning ServerSelectionStrategy.GetHashTag(ServerEndPoint) overload entirely.
2. Key composition encapsulated, cache dropped: Replaced GetHashTagPrefix/HashTags.PrefixCache (the 16384-entry static byte[]?[], lock, and the shared-mutable-state RedisKey it published) with ServerSelectionStrategy.CreateKeyForSlot(slot, suffix), which composes the whole key per call. Since GetTracerKey is on the heartbeat path, I memoized the composed key on ServerEndPoint instead (invalidated when the servable slot changes) rather than caching at the static level.
3. Replica tracer fixed + Parent bug fixed: GetServableSlot() now falls back to node.Parent's slots when the node's own Slots is empty (the replica case). That only works because ClusterNode.Parent is fixed too - it was structured so it only looked up a parent when the backing field was already non-null, which it never was, so Parent always returned null. Fixed to ParentNodeId is null ? null : (parent ??= configuration[ParentNodeId]).
4. AutoConfigureAsync guards: Added a comment explaining the first-handshake window (serverType is seeded Standalone until the CLUSTER NODES reply is processed, so neither guard suppresses anything on the very first call for a cluster node - only from the next handshake/heartbeat onward). Also added a comment explaining why the SET probe can't work on cluster at all regardless of slot-tagging: a replica answers MOVED, not READONLY, for a write to its primary's slot, so the probe can't distinguish "I'm a replica of this slot's owner" from "wrong slot" - tagging wouldn't fix it. Confirmed the GetClusterNode(ClusterConfiguration) is not null clause was redundant and dropped it (ClusterConfiguration is populated by the same CLUSTER NODES reply that flips ServerType, so by the time ServerType == Cluster is observable here, topology is either known or this guard has already been skipped on the prior first-handshake pass).
5. Tie-breaker skip - agreed, citing docs/Configuration.md's existing "not including redis cluster" note in the PR description, no doc change needed.
6. Tests: Added AutoConfigureSkipsKeyProbesWhenClusterTopologyKnown (asserts no tie-breaker GET and no SET ... replica_read_only against a cluster endpoint once topology is known, using a command-recording in-process server), ExistsTracerOnClusterReplicaUsesPrimarySlot (a replica's tracer key targets its primary's slot - fails without the fix), and ExistsTracerUsesPlainKeyWhileClusterTopologyIsNotKnown (the "cluster, topology not known yet" case). Fixed ExistsTracerUsesPlainKeyWithoutKnownOwnedSlots to dispose its hand-built ServerEndPoint, and dropped the Assert.Same prefix-caching assertion from HashTagUnitTests now that there's no cache to prove identity of.
7. GetClusterNode: now uses ClusterConfiguration's this[EndPoint] indexer (O(1) dict hit) instead of the LINQ scan - this also speeds up the existing UpdateNodeRelations caller. (RedisValue)UniqueId -> RedisKey on the EXISTS message is unchanged, still wire-identical. On the BOM-only churn in ServerSelectionStrategy.HashTags.cs/HashTagUnitTests.cs: calling it out here as unrelated formatting noise from the original commit rather than splitting it off, since it's a single line each and not worth a separate commit at this point.

Build is clean (dotnet build Build.csproj -c Release /p:CI=true, 0 warnings/errors) and the hash-tag/cluster-probe/tracer test filter passes (29/29). Also ported the same fixes to #3187's branch as a separate commit there, since it carries this PR's original commit in its history and inherits the same bugs until this lands.