Skip to content

Prefer announce hostname when dialing cluster node on reconnect #3333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
meerasridharan-28 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Prefer announce hostname when dialing cluster node on reconnect #3333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/StackExchange.Redis/ClusterTopology.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ internal ClusterTopologyNode(ClusterSlotNode node, string? primaryNodeId)
/// </summary>
public IReadOnlyList<EndPoint> Identities => _identities ?? [];

/// <summary>
/// The announced endpoint form to use for dialing. Companion identities such as an IP address must not be
/// registered as separate dialable servers when this preferred endpoint is a hostname.
/// </summary>
public EndPoint? PreferredDialEndPoint => Identities.Count > 0 ? Identities[0] : null;

/// <summary>The slots this node serves as primary; empty for a replica.</summary>
public IReadOnlyList<SlotRange> Slots => _slots ?? [];

Expand Down
81 changes: 64 additions & 17 deletions src/StackExchange.Redis/ConnectionMultiplexer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2286,12 +2286,12 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) =>
else if (listedInSlots!.Contains(node.EndPoint))
{
// SLOTS lists it, so it is in clusterEndpoints and we are about to connect to it and
// wait for it. Only the *existence* of a server was ever checked here, which a node
// in this state fails - a replica of a shard we have not reached yet is created by
// its primary's genealogy pass, and that has not run. Registering it inert then left
// it waited on but undialled, for the whole ConnectTimeout (#3232)
log?.LogInformationRegisteringSlotMapNode(new(node.EndPoint));
GetServerEndPoint(node.EndPoint, ServerProvenance.ClusterTopology, log);
// wait for it. Prefer the SLOTS hostname over its companion IP from NODES: that IP
// may be an unreachable pod address using the ingress client port.
var dialEndPoint = FindPreferredDialEndPoint(topology, node.EndPoint) ?? node.EndPoint;
log?.LogInformationRegisteringSlotMapNode(new(dialEndPoint));
GetServerEndPoint(dialEndPoint, ServerProvenance.ClusterTopology, log);
if (!Equals(dialEndPoint, node.EndPoint)) RegisterServerIdentities(topology);
}
else
{
Expand Down Expand Up @@ -2320,18 +2320,44 @@ public EndPoint[] GetEndPoints(bool configuredOnly = false) =>
return null;
}

// an identity we already hold if there is one - otherwise the form the answering node advertised,
// which is what Identities is ordered by, and which is the form TLS can validate
EndPoint? SelectIdentity(ClusterTopologyNode node)
{
var preferred = node.PreferredDialEndPoint;
ServerEndPoint? preferredKnown = null, dnsKnown = null, anyKnown = null;

foreach (var identity in node.Identities)
{
if (TryResolveServerEndPoint(identity) is { } known) return known.EndPoint;
if (TryResolveServerEndPoint(identity) is not { } known) continue;
anyKnown ??= known;
if (preferred is not null && Equals(known.EndPoint, preferred)) preferredKnown = known;
if (known.EndPoint is DnsEndPoint) dnsKnown ??= known;
}

if (preferred is DnsEndPoint)
{
return (preferredKnown ?? dnsKnown)?.EndPoint ?? preferred;
}
return node.Identities.Count > 0 ? node.Identities[0] : null;

return (preferredKnown ?? anyKnown)?.EndPoint ?? preferred;
}
}

/// <summary>
/// Finds the SLOTS preferred dial endpoint for a node that also answers at <paramref name="identity"/>
/// (typically the IP form from <c>CLUSTER NODES</c>).
/// </summary>
private static EndPoint? FindPreferredDialEndPoint(ClusterTopology topology, EndPoint identity)
{
foreach (var node in topology.Nodes)
{
foreach (var id in node.Identities)
{
if (Equals(id, identity)) return node.PreferredDialEndPoint;
}
}
return null;
}

private void ResetAllNonConnected()
{
var snapshot = GetServerSnapshot();
Expand Down Expand Up @@ -2552,8 +2578,8 @@ internal void UpdateClusterRange(ClusterTopology topology)
{
if (node.IsReplica || node.Slots.Count == 0) continue;

// resolve by *any* identity the node answers to before falling back to the form this reply
// happened to use; that is what keeps one node from becoming two ServerEndPoints
// Resolve the preferred server first, and do not adopt a companion IP server when hostnames
// are preferred.
var server = ResolveOrCreate(node);
if (server is null) continue;

Expand All @@ -2565,15 +2591,36 @@ internal void UpdateClusterRange(ClusterTopology topology)

ServerEndPoint? ResolveOrCreate(ClusterTopologyNode node)
{
var preferred = node.PreferredDialEndPoint;
ServerEndPoint? preferredKnown = null, dnsKnown = null, anyKnown = null;

foreach (var identity in node.Identities)
{
if (TryResolveServerEndPoint(identity) is { } known) return known;
if (TryResolveServerEndPoint(identity) is not { } known) continue;
anyKnown ??= known;
if (preferred is not null && Equals(known.EndPoint, preferred)) preferredKnown = known;
if (known.EndPoint is DnsEndPoint) dnsKnown ??= known;
}

// unknown node: dial the form the answering node *advertised*, which Identities is ordered by.
// Not the address by preference: a certificate validates against a name, and where hostnames
// are preferred the advertised address may not even be routable (#2826)
return node.Identities.Count > 0 ? GetServerEndPoint(node.Identities[0], ServerProvenance.ClusterTopology) : null;
if (preferred is DnsEndPoint)
{
// Hostname-routed TLS/SNI clusters: never adopt a companion IP ServerEndPoint for
// the slot map. That IP is often the pod address with the ingress client port and
// does not survive pod restart.
if (preferredKnown is not null) return preferredKnown;
if (dnsKnown is not null) return dnsKnown;
return preferred is not null
? GetServerEndPoint(preferred, ServerProvenance.ClusterTopology)
: null;
}

if (preferredKnown is not null) return preferredKnown;
if (anyKnown is not null) return anyKnown;

// Unknown node: dial the form the answering node advertised (#2826).
return preferred is not null
? GetServerEndPoint(preferred, ServerProvenance.ClusterTopology)
: null;
}
}

Expand Down
29 changes: 26 additions & 3 deletions tests/StackExchange.Redis.Tests/ClusterTopologyUnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,23 @@ public async Task ShadowTopologyKnowsBothIdentities(ClusterEndpointType preferre
Assert.Contains(new DnsEndPoint(Hostname, port), node.Identities);
}

[Theory]
[InlineData(ClusterEndpointType.Ip)]
[InlineData(ClusterEndpointType.Hostname)]
public async Task PreferredDialEndPointFollowsAnnouncedForm(ClusterEndpointType preferred)
{
using var server = CreateServer(log, preferred);
await using var conn = await server.ConnectAsync(defaultOnly: true);

var node = Assert.Single((await GetShadowAsync(conn, server.DefaultEndPoint)).Nodes);
GetHost(server.DefaultEndPoint, out var port);
EndPoint expected = preferred == ClusterEndpointType.Hostname
? new DnsEndPoint(Hostname, port)
: new IPEndPoint(IPAddress.Loopback, port);
Assert.Equal(expected, node.PreferredDialEndPoint);
Assert.Equal(node.Identities[0], node.PreferredDialEndPoint);
}

[Fact]
public async Task ShadowTopologyAgreesWithTheRoutingView()
{
Expand Down Expand Up @@ -173,19 +190,25 @@ public async Task SlotMapIsDrivenByTheSlotsView()
[Fact]
public async Task HostnamePreferredClusterRoutesWithoutDuplicatingEndpoints()
{
// the case the flip exists for: SLOTS names every node by hostname while NODES names them by
// address, so a slot map fed from SLOTS must still resolve to the servers we already hold
// Discover the new shard from a single seed. SLOTS names it by hostname, while NODES reports its
// companion IP; routing and registration must use the hostname and retain the IP only as an alias.
using var server = CreateServer(log, ClusterEndpointType.Hostname);
GetHost(server.DefaultEndPoint, out var port);
var other = server.AddEmptyNode(new IPEndPoint(IPAddress.Loopback, port + 1));
server.SetHostname(other, "host-2.redis.example.com");
server.Migrate((RedisKey)"hostname-routed-key", other);

await using var conn = await server.ConnectAsync();
await using var conn = await server.ConnectAsync(defaultOnly: true);
var db = conn.GetDatabase();
await db.StringSetAsync("hostname-routed-key", "value");
Assert.Equal("value", await db.StringGetAsync("hostname-routed-key"));

var hostnameEndpoint = new DnsEndPoint("host-2.redis.example.com", port + 1);
var companionIp = new IPEndPoint(IPAddress.Loopback, port + 1);
Assert.Contains(hostnameEndpoint, conn.GetEndPoints());
Assert.DoesNotContain(companionIp, conn.GetEndPoints());
Assert.Equal(hostnameEndpoint, conn.GetServer(companionIp).EndPoint);

foreach (var ep in conn.GetEndPoints())
{
log.WriteLine($"endpoint: {ep}");
Expand Down
Loading