From 5944fa23f6c7d1b571964c87d64d56f1160a954d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 10:27:48 +0100 Subject: [PATCH] Sentinel soft-fail follow-ups: discovery failure as RedisConnectionException, late sentinel init, clear LastException Follow-ups to #3187 (AbortOnConnectFail=false for sentinel), prompted by the investigation of #3271: - GetSentinelMasterConnection with a config that has no endpoints of its own and nothing discovered threw AggregateException(InvalidOperationException "No nodes to consider") from the seed connect. It now throws the discovery RedisConnectionException, as it did before #3187. - A sentinel connection whose initial connect soft-failed never ran InitializeSentinel, because only the connect paths called it; such a connection never subscribed to +switch-master and tracked failovers only through the fallback polling. InitializeSentinel is now idempotent and is also reached from a non-first reconfigure that detects a sentinel. - The discovery failure recorded as LastException at connect time stayed there after recovery and was folded into every later "no connection available" message. It is cleared once the managed connection is restored. - Document what abortConnect means for sentinel connections. --- docs/Configuration.md | 5 +++ .../ConnectionMultiplexer.Sentinel.cs | 37 +++++++++++++++---- .../ConnectionMultiplexer.cs | 7 ++++ .../SentinelConfigTests.cs | 21 +++++++++++ .../SentinelTests.cs | 12 ++++++ 5 files changed, 74 insertions(+), 8 deletions(-) diff --git a/docs/Configuration.md b/docs/Configuration.md index 72bc9b2b9..2519c0e2e 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -36,6 +36,11 @@ pointing to that primary server that will automatically be updated if the primar var conn = ConnectionMultiplexer.Connect("localhost,serviceName=myprimary"); ``` +`abortConnect` applies to sentinel connections the same way it applies to everything else. With `abortConnect=true` (the default), `Connect` +throws if the sentinels cannot be reached or do not know the service. With `abortConnect=false`, `Connect` returns a multiplexer even then: +it is not connected, operations fail with a connection exception that names the problem, and it connects itself to the primary once the +sentinels become reachable and name one. If you want startup to fail when the sentinels are unavailable, keep `abortConnect=true`. + An overview of mapping between the `string` and `ConfigurationOptions` representation is shown below, but you can switch between them trivially: ```csharp diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs index 40d513a3f..a93f8bd95 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs @@ -15,14 +15,21 @@ public partial class ConnectionMultiplexer internal Timer? sentinelPrimaryReconnectTimer; internal readonly Dictionary sentinelConnectionChildren = new(); internal ConnectionMultiplexer? sentinelConnection; + private int _sentinelInitialized; /// /// Initializes the connection as a Sentinel connection and adds the necessary event handlers to track changes to the managed primaries. /// + /// + /// Runs once. It is reached from the connect paths, and also from the reconfigure loop when a connection + /// that soft-failed its initial connect ( is false) later + /// reaches a sentinel: without that, a connection that started while the sentinels were down would never subscribe + /// to +switch-master, and would track failovers only through the fallback polling. + /// /// The to log to, if any. internal void InitializeSentinel(ILogger? log) { - if (!_isSentinel) + if (!_isSentinel || Interlocked.Exchange(ref _sentinelInitialized, 1) != 0) { return; } @@ -253,8 +260,7 @@ public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions co } if (connection is not null) try { connection.Dispose(); } catch { } - endpoints = config.EndPoints.Clone(); - connection = ConnectImpl(config, log, endpoints: endpoints); + connection = SeedConnection(failure); break; } @@ -330,11 +336,7 @@ public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions co throw failure; } - if (connection is null) - { - endpoints = config.EndPoints.Clone(); - connection = ConnectImpl(config, log, endpoints: endpoints); - } + connection ??= SeedConnection(failure); connection.LastException = failure; connection.Logger?.LogErrorSyncConnectTimeout(failure, failure.Message); @@ -385,6 +387,20 @@ public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions co } throw; } + + // AbortOnConnectFail=false with no primary discovered: hand back a multiplexer seeded with whatever endpoints + // the caller gave us (the sentinel addresses, via Connect), for SwitchPrimary to rebuild once discovery works. + // A config with no endpoints of its own has nothing to seed with, and that is the discovery failure the caller + // should see - not the "No nodes to consider" that ConnectImpl would throw at an empty endpoint list. + ConnectionMultiplexer SeedConnection(RedisConnectionException discoveryFailure) + { + if (config.EndPoints.Count == 0) + { + throw discoveryFailure; + } + endpoints = config.EndPoints.Clone(); + return ConnectImpl(config, log, endpoints: endpoints); + } } [System.Diagnostics.CodeAnalysis.SuppressMessage("Roslynator", "RCS1075:Avoid empty catch clause that catches System.Exception.", Justification = "We don't care.")] @@ -423,6 +439,11 @@ internal void OnManagedConnectionRestored(object? sender, ConnectionFailedEventA // we are not aware of, so retry SwitchPrimary(e.EndPoint, connection); } + + // A soft-failed connect (AbortOnConnectFail=false) records the discovery failure as LastException, and + // the exception factory folds that into every "no connection available" message. We are connected to a + // sentinel-approved primary again, so stop reporting it. + connection.LastException = null; } catch (Exception) { diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs index 970561773..ee4105444 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.cs @@ -2038,6 +2038,13 @@ internal async Task ReconfigureAsync(bool first, bool reconfigureAll, ILog else if (standaloneCount == 0 && sentinelCount > 0) { ServerSelectionStrategy.ServerType = ServerType.Sentinel; + if (!first) + { + // A sentinel connection whose initial connect soft-failed (AbortOnConnectFail=false) is + // only now reaching a sentinel; the connect path that would have wired up the sentinel + // handlers has long since returned. (The first connect leaves that to ConnectImpl.) + InitializeSentinel(log); + } } else if (standaloneCount > 0) { diff --git a/tests/StackExchange.Redis.Tests/SentinelConfigTests.cs b/tests/StackExchange.Redis.Tests/SentinelConfigTests.cs index 814f99719..d1e58b37a 100644 --- a/tests/StackExchange.Redis.Tests/SentinelConfigTests.cs +++ b/tests/StackExchange.Redis.Tests/SentinelConfigTests.cs @@ -93,6 +93,27 @@ public void Connect_UnreachableSentinel_AbortEnabled_Throws() Assert.Throws(() => ConnectionMultiplexer.Connect(options)); } + [Fact] + public void GetSentinelMasterConnection_NothingToSeedWith_AbortDisabled_ThrowsTheDiscoveryFailure() + { + // the direct API: a long-lived SentinelConnect, then a per-service config that carries only the service name + // (the data endpoints are sentinel's to supply). With nothing discovered and nothing to seed a multiplexer + // with, the caller should see the discovery failure as a RedisConnectionException - not the + // InvalidOperationException("No nodes to consider") that connecting an empty endpoint list produces. + using var sentinel = ConnectionMultiplexer.SentinelConnect(GetUnreachableSentinelOptions(abortOnConnectFail: false)); + var config = new ConfigurationOptions + { + AbortOnConnectFail = false, + ConnectRetry = 0, + ConnectTimeout = 100, + ServiceName = "unreachable-primary", + }; + + var exception = Assert.Throws(() => sentinel.GetSentinelMasterConnection(config)); + Assert.Equal(ConnectionFailureType.UnableToConnect, exception.FailureType); + Assert.Equal("Sentinel: Failed connecting to configured primary for service: unreachable-primary", exception.Message); + } + private static ConfigurationOptions GetUnreachableSentinelOptions(bool abortOnConnectFail) { var options = new ConfigurationOptions diff --git a/tests/StackExchange.Redis.Tests/SentinelTests.cs b/tests/StackExchange.Redis.Tests/SentinelTests.cs index d99e47db7..ffe6d3705 100644 --- a/tests/StackExchange.Redis.Tests/SentinelTests.cs +++ b/tests/StackExchange.Redis.Tests/SentinelTests.cs @@ -565,6 +565,18 @@ await UntilConditionAsync( () => !conn.GetServerSnapshot().ToArray().Any(x => Equals(x.EndPoint, seedEndPoint))); Assert.DoesNotContain(seedEndPoint, conn.GetEndPoints()); + // the sentinel connection behind it soft-failed too, so the connect path never wired up its handlers; it + // must do that itself on reaching a sentinel, or this connection would never hear +switch-master + var sentinelConn = conn.sentinelConnection!; + var switchMaster = RedisChannel.Literal("+switch-master"); + await UntilConditionAsync( + TimeSpan.FromSeconds(10), + () => sentinelConn.GetSubscriber().SubscribedEndpoint(switchMaster) is not null && conn.LastException is null); + Assert.NotNull(sentinelConn.GetSubscriber().SubscribedEndpoint(switchMaster)); + + // and the discovery failure recorded at connect time is no longer reported now that we have a primary + Assert.Null(conn.LastException); + static bool TryPing(IDatabase db) { try