From 1c2a4725c860476b37027cb2b69b69aefb810b17 Mon Sep 17 00:00:00 2001 From: Gabriel Harnagea Date: Tue, 18 Aug 2026 22:06:09 +0200 Subject: [PATCH] Make fallback discovery and keep-alive probes cluster-slot aware On OSS cluster, the direct (NoRedirect) probe messages used during connection setup and keep-alive could target a hash slot the connected node doesn't own, so the server replies MOVED and the probe is dropped instead of following it. - Skip the replica_read_only SET fallback in AutoConfigureAsync once cluster topology already reports our role, since it's both redundant and slot-unsafe there. - Skip the tie-breaker GET fallback in AutoConfigureAsync on cluster, where a tie-breaker key isn't meaningful. - When the ECHO/PING/TIME tracer is unavailable, build the EXISTS fallback key with a hash-tag targeting a slot this endpoint actually owns, reusing the existing hash-tag cache. Fixes #2970. --- src/StackExchange.Redis/ServerEndPoint.cs | 27 ++++++-- .../ServerSelectionStrategy.HashTags.cs | 19 +++++- .../ServerSelectionStrategy.cs | 2 + .../HashTagUnitTests.cs | 16 ++++- .../ServerEndPointClusterProbeUnitTests.cs | 66 +++++++++++++++++++ 5 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 tests/StackExchange.Redis.Tests/ServerEndPointClusterProbeUnitTests.cs diff --git a/src/StackExchange.Redis/ServerEndPoint.cs b/src/StackExchange.Redis/ServerEndPoint.cs index 20968261b..278d98193 100644 --- a/src/StackExchange.Redis/ServerEndPoint.cs +++ b/src/StackExchange.Redis/ServerEndPoint.cs @@ -355,7 +355,7 @@ public void SetClusterConfiguration(ClusterConfiguration configuration) public void UpdateNodeRelations(ClusterConfiguration configuration) { - var thisNode = configuration.Nodes.FirstOrDefault(x => x.EndPoint?.Equals(EndPoint) == true); + var thisNode = GetClusterNode(configuration); if (thisNode != null) { Multiplexer.Trace($"Updating node relations for {Format.ToString(thisNode.EndPoint)}..."); @@ -379,6 +379,9 @@ public void UpdateNodeRelations(ClusterConfiguration configuration) } } + private ClusterNode? GetClusterNode(ClusterConfiguration? configuration) => + configuration?.Nodes.FirstOrDefault(x => x.EndPoint?.Equals(EndPoint) == true); + public void SetUnselectable(UnselectableFlags flags) { if (flags != 0) @@ -502,7 +505,9 @@ internal async Task AutoConfigureAsync(PhysicalConnection? connection, ILogger? await WriteDirectOrQueueFireAndForgetAsync(connection, msg, autoConfigProcessor).ForAwait(); } } - else if (commandMap.IsAvailable(RedisCommand.SET) && !(helloPending || RoleKnownFromHello)) + else if (commandMap.IsAvailable(RedisCommand.SET) + && !(helloPending || RoleKnownFromHello) + && !(ServerType == ServerType.Cluster && GetClusterNode(ClusterConfiguration) is not null)) { // This is a nasty way to find if we are a replica, and it will only work on up-level servers, but... // (note we only get here when HELLO isn't going to tell us: the HELLO reply carries "role", and @@ -535,7 +540,9 @@ internal async Task AutoConfigureAsync(PhysicalConnection? connection, ILogger? } // If we are going to fetch a tie breaker, do so last and we'll get it in before the tracer fires completing the connection // But if GETs are disabled on this, do not fail the connection - we just don't get tiebreaker benefits - if (Multiplexer.RawConfig.TryGetTieBreaker(out var tieBreakerKey) && Multiplexer.CommandMap.IsAvailable(RedisCommand.GET)) + if (ServerType != ServerType.Cluster + && Multiplexer.RawConfig.TryGetTieBreaker(out var tieBreakerKey) + && Multiplexer.CommandMap.IsAvailable(RedisCommand.GET)) { log?.LogInformationRequestingTieBreak(new(EndPoint), tieBreakerKey); msg = Message.Create(0, flags, RedisCommand.GET, tieBreakerKey); @@ -680,12 +687,24 @@ internal Message GetTracerMessage(bool checkResponse) else { map.AssertAvailable(RedisCommand.EXISTS); - msg = Message.Create(0, flags, RedisCommand.EXISTS, (RedisValue)Multiplexer.UniqueId); + msg = Message.Create(0, flags, RedisCommand.EXISTS, GetTracerKey()); } msg.SetInternalCall(); return msg; } + internal RedisKey GetTracerKey() + { + RedisKey key = Multiplexer.UniqueId; + if (ServerType == ServerType.Cluster + && GetClusterNode(ClusterConfiguration) is { } node + && node.Slots.Count > 0) + { + key = key.Prepend(ServerSelectionStrategy.GetHashTagPrefix(node.Slots[0].From)); + } + return key; + } + internal UnselectableFlags GetUnselectableFlags() => unselectableReasons; internal bool IsSelectable(RedisCommand command, bool allowDisconnected = false) diff --git a/src/StackExchange.Redis/ServerSelectionStrategy.HashTags.cs b/src/StackExchange.Redis/ServerSelectionStrategy.HashTags.cs index 4cbd4538c..a1751f50a 100644 --- a/src/StackExchange.Redis/ServerSelectionStrategy.HashTags.cs +++ b/src/StackExchange.Redis/ServerSelectionStrategy.HashTags.cs @@ -1,6 +1,7 @@ -using System; +using System; using System.Diagnostics; using System.Text; +using System.Threading; namespace StackExchange.Redis; @@ -10,9 +11,25 @@ internal sealed partial class ServerSelectionStrategy private static class HashTags { private static readonly string[] Cache = Populate(); + private static readonly byte[]?[] PrefixCache = new byte[TotalSlots][]; + private static readonly object PrefixCacheLock = new(); + public static ReadOnlySpan Tags => Cache; public static string Get(int slot) => Cache[slot]; + public static byte[] GetPrefix(int slot) + { + var prefix = Volatile.Read(ref PrefixCache[slot]); + if (prefix is null) + { + lock (PrefixCacheLock) + { + prefix = PrefixCache[slot] ??= Encoding.ASCII.GetBytes("{" + Get(slot) + "}"); + } + } + return prefix; + } + private static string[] Populate() { // Via testing, we know that 3 characters is sufficient to populate all slots diff --git a/src/StackExchange.Redis/ServerSelectionStrategy.cs b/src/StackExchange.Redis/ServerSelectionStrategy.cs index 80c9b9efe..8c0623e67 100644 --- a/src/StackExchange.Redis/ServerSelectionStrategy.cs +++ b/src/StackExchange.Redis/ServerSelectionStrategy.cs @@ -442,5 +442,7 @@ internal string GetHashTag(ServerEndPoint endpoint) /// Gets a string that can be used as a hash-tag to reference a specific slot. /// internal static string GetHashTag(int slot) => slot < 0 ? "" : HashTags.Get(slot); + + internal static RedisKey GetHashTagPrefix(int slot) => HashTags.GetPrefix(slot); } } diff --git a/tests/StackExchange.Redis.Tests/HashTagUnitTests.cs b/tests/StackExchange.Redis.Tests/HashTagUnitTests.cs index 91b63d980..6709da826 100644 --- a/tests/StackExchange.Redis.Tests/HashTagUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/HashTagUnitTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Text; using Xunit; @@ -26,4 +26,18 @@ public void TestHashTagCoverage() } Assert.Equal(ServerSelectionStrategy.TotalSlots, uniques.Count); } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(8191)] + [InlineData(16383)] + public void TestHashTagPrefixTargetsSlot(int slot) + { + var prefix = ServerSelectionStrategy.GetHashTagPrefix(slot); + RedisKey key = ((RedisKey)"probe-id").Prepend(prefix); + + Assert.Equal(slot, ServerSelectionStrategy.GetHashSlot(key)); + Assert.Same((byte[]?)prefix, (byte[]?)ServerSelectionStrategy.GetHashTagPrefix(slot)); + } } diff --git a/tests/StackExchange.Redis.Tests/ServerEndPointClusterProbeUnitTests.cs b/tests/StackExchange.Redis.Tests/ServerEndPointClusterProbeUnitTests.cs new file mode 100644 index 000000000..3ff088ceb --- /dev/null +++ b/tests/StackExchange.Redis.Tests/ServerEndPointClusterProbeUnitTests.cs @@ -0,0 +1,66 @@ +using System.Linq; +using System.Net; +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests; + +public class ServerEndPointClusterProbeUnitTests +{ + [Fact] + public async Task ExistsTracerUsesOwnedClusterSlot() + { + using var server = new InProcessTestServer { ServerType = ServerType.Cluster }; + var config = server.GetClientConfig(defaultOnly: true); + var commands = server.GetCommands(); + commands.Remove(nameof(RedisCommand.ECHO)); + commands.Remove(nameof(RedisCommand.PING)); + commands.Remove(nameof(RedisCommand.TIME)); + config.CommandMap = CommandMap.Create(commands); + + await using var connection = await ConnectionMultiplexer.ConnectAsync(config); + var endpoint = connection.GetServerEndPoint(server.DefaultEndPoint); + var node = endpoint.ClusterConfiguration?.Nodes.Single(x => x.EndPoint?.Equals(endpoint.EndPoint) == true); + Assert.NotNull(node); + var targetSlot = node.Slots[0].From; + + var message = endpoint.GetTracerMessage(checkResponse: true); + + Assert.Equal(RedisCommand.EXISTS, message.Command); + Assert.Equal(targetSlot, message.GetHashSlot(connection.ServerSelectionStrategy)); + var key = endpoint.GetTracerKey(); + var keyBytes = (byte[]?)key; + var prefixBytes = (byte[]?)ServerSelectionStrategy.GetHashTagPrefix(targetSlot); + Assert.NotNull(keyBytes); + Assert.NotNull(prefixBytes); + Assert.True(keyBytes.Take(prefixBytes.Length).SequenceEqual(prefixBytes)); + Assert.True(keyBytes.Skip(keyBytes.Length - connection.UniqueId.Length).SequenceEqual(connection.UniqueId)); + } + + [Theory] + [InlineData(ServerType.Standalone)] + [InlineData(ServerType.Cluster)] + public async Task ExistsTracerUsesPlainKeyWithoutKnownOwnedSlots(ServerType serverType) + { + using var server = new InProcessTestServer(); + var config = server.GetClientConfig(defaultOnly: true); + var commands = server.GetCommands(); + commands.Remove(nameof(RedisCommand.ECHO)); + commands.Remove(nameof(RedisCommand.PING)); + commands.Remove(nameof(RedisCommand.TIME)); + config.CommandMap = CommandMap.Create(commands); + + await using var connection = await ConnectionMultiplexer.ConnectAsync(config); + var endpoint = new ServerEndPoint(connection, new IPEndPoint(IPAddress.Loopback, 12345)) + { + ServerType = serverType, + }; + + var message = endpoint.GetTracerMessage(checkResponse: true); + var clusterStrategy = new ServerSelectionStrategy(null) { ServerType = ServerType.Cluster }; + + Assert.Equal(RedisCommand.EXISTS, message.Command); + Assert.Equal(ServerSelectionStrategy.GetHashSlot((RedisKey)connection.UniqueId), message.GetHashSlot(clusterStrategy)); + Assert.Equal(connection.UniqueId, (byte[]?)endpoint.GetTracerKey()); + } +}