[meerasridharan-28]

When cluster-preferred-endpoint-type=hostname, CLUSTER SLOTS can announce shard hostnames while CLUSTER NODES reports companion pod IPs. In deployments where those IPs are not reachable, selecting an IP endpoint can prevent the client from connecting to a shard after topology changes.

This PR makes cluster discovery and slot routing prefer the endpoint form announced by CLUSTER SLOTS. When that form is a hostname, the companion IP is treated as an alias rather than preferred as a separate dialable server. IP-preferred clusters continue to prefer IP endpoints, and the existing CLUSTER NODES behavior is preserved when there is no usable CLUSTER SLOTS topology.

Regression coverage checks endpoint preference for IP- and hostname-preferred topologies. It also uses a single configured seed, moves a slot to a hostname-announcing shard, and verifies the shard is discovered by hostname while its companion IP resolves to the same server.

Related: PR #3250
[ISSUE] (#3309 (comment))

Checklist

• I fully and freely contribute this code in accordance with the project license (and am legally able to do so)
• I take responsibility for this contribution's quality and correctness, including any portions produced with AI assistance (see CONTRIBUTING.md).

[mgravell]

Thank you for digging into this — the scenario is nasty (SLOTS announcing hostnames while NODES reports unreachable companion pod IPs), the intent here is right, and the SNI/hostname angle genuinely matters for these deployments. The test that moves a slot to a hostname-announcing shard and checks the companion IP resolves to the same server is a good addition.

That said, review turned up some structural concerns — the main one being that the fix appears to hold only on first discovery, with the underlying adoption mechanism unchanged. In rough order of importance:

1. The new branches lean on _serverIdentities aliases harder than the alias machinery can bear. Three variations of the same weakness:

   • Missing alias → duplicate server. ResolveOrCreate's DNS-preferred branch deliberately skips anyKnown, so if the node is already held as a companion-IP ServerEndPoint that has no hostname alias (created between topology applications — a MOVED/redirect to the IP, or a NODES-only refresh after SLOTS was refused once), GetServerEndPoint(preferred) creates a second ServerEndPoint for the same node — the duplication the deleted comment ("that is what keeps one node from becoming two ServerEndPoints") existed to prevent. The duplicate is only merged by ApplyTopologyGenerationAsync, which runs in the discovery path, not on autoconfigure, so traffic splits across two sockets — and if the hostname isn't resolvable from the client, those slots route to a server that can never connect while the working IP server sits idle.
   • Stale alias → wrong node. RegisterServerIdentities uses TryAdd, which never overwrites a live mapping. Pod-IP reuse — this PR's own target environment — is exactly what produces a hostname identity still aliased to a different, still-live server; SelectIdentity now returns the raw hostname (where the old code returned the first known identity), and the reconnect iteration resolves through the stale alias to the wrong node.
   • Alias present → fix undone. Once a dns→ipServer alias is registered, GetServerEndPoint(preferred) resolves straight back to the IP server, so the slot map re-adopts the pod IP on every later refresh. One SLOTS-refused refresh (the supported NODES fallback) is enough to seed that state, and after a pod restart the originally-reported failure recurs.

   Together these suggest the durable fix lives at the adoption mechanism rather than the selection branches: key the decision on the canonical endpoint form at resolve time, or re-point an alias-resolved server to its preferred dial form — so the invariant holds on every refresh, not just the first.

2. The old regression case was repointed rather than kept. HostnamePreferredClusterRoutesWithoutDuplicatingEndpoints moved from ConnectAsync() (all endpoints seeded) to ConnectAsync(defaultOnly: true), which deletes the case the test existed for — a slot map fed from SLOTS must still resolve to servers we already hold. That seeded-IP path is exactly the fragile one in (1), and it now has no coverage. Could you keep both: the original seeded variant and your new single-seed discovery variant?

3. Quality items, non-blocking but worth folding into the rework:

   • ResolveOrCreate duplicates SelectIdentity's whole preferredKnown/dnsKnown/anyKnown scan and selection policy (~2323-2342 vs ~2592-2624), differing only in return type — any fix to the rules in (1) currently has to land twice. A shared helper would collapse them.
   • FindPreferredDialEndPoint rescans every node's identity list per CLUSTER NODES entry, and the per-node RegisterServerIdentities re-runs whole-topology registration (largely redundant with the call at ~2313) — O(N²) per reconfigure on a large cluster. The loop that builds listedInSlots already visits every identity; a Dictionary<EndPoint, EndPoint> of identity → preferred dial form built there removes both.
   • A few dead null checks (preferred is not null ? inside if (preferred is DnsEndPoint), the unreachable ?? node.EndPoint) imply states that can't occur — stating the real invariant reads better.
   • In the tests, Assert.Equal(node.Identities[0], node.PreferredDialEndPoint) is tautological (that's the property's definition); the preceding assertion against the expected value already covers the contract.

I realize (1) is a bigger ask than a touch-up — it's a genuinely tricky corner of the codebase, and the fact that the aliasing subtleties surfaced at all is down to this PR putting a concrete scenario and tests on the table, which is valuable in itself. Happy to talk through the resolve-time approach on the PR or the linked issue if that helps. Thanks again for taking this on.

[meerasridharan-28]

@mgravell Thanks for the detailed feedback, I mainly tested scenario we reproduced, where a cluster shard is moved/ restarted with host-name based routing and this PR fix was working for that case. I see from your review that there are additional complex scenarios that current approach may not cover. I am still trying to understand al the scenarios you listed above.

Since we need this fix fairly soon, would you be able to take on the structural changes you are suggesting?
i can continue looking into it as well but it may take some time to fully understand the scenarios ,code and implement the right fix.

[mgravell]

As discussed, I've taken on the structural rework and pushed it to this branch (a09a6a3) — your commit and tests stay as the foundation, and your scenario is still the thing the PR fixes.

What moved, and why:

• The preference now lives where identities are canonicalized. The selection branches (SelectIdentity/ResolveOrCreate) are back to their simple "any identity we already hold" form, and the announced-hostname preference is applied in ApplyTopologyGenerationAsync instead — the pass that already owns survivor choice, alias overwrite and duplicate merging, and that runs on every applied topology. A node held by its companion IP while CLUSTER SLOTS announces a hostname is re-homed there: a server is adopted at the hostname form and the IP server is retired through the existing merge machinery, which also re-points every alias. That closes the three cases from the earlier review (duplicate servers on the autoconfigure path, stale aliases after pod-IP reuse, and the fix being undone by a later refresh) in one place.
• Fresh discovery never creates the companion server at all. The loop that already visits every identity now remembers each node's announced dial form, so the CLUSTER NODES remainder branch creates a SLOTS-listed node at its hostname in the first place — your first-discovery fix, kept, just without the per-node rescans.
• Configured endpoints are exempt: an explicitly seeded address is never re-homed from under the caller.

Tests: your single-seed discovery test stays (made deterministic with an explicit applied-topology pass), the original seeded regression test is restored alongside it, and there's a new case covering the durable half — a shard already held by its IP moving to its hostname when the announced preference changes. Full suite is green locally apart from a pre-existing environment flake unrelated to this area.

Thanks again for the repro, the tests and the patience — the concrete scenario is what made the right fix findable. Please give it a try against your deployment: the behaviour you fixed should hold, and now also survive pod restarts and later topology refreshes.

[meerasridharan-28]

@mgravell Thanks for taking time to fix this PR. Really appreciate your help getting this issue addressed.
I tested latest changes by killing primary shard and restarting the shards and client recovered successfully in both cases.
Everything looks good from my side. Are we good to merge this?