From f121c10fad302ab1943e4cfc2125ca57dd3dead5 Mon Sep 17 00:00:00 2001 From: rebaseandpanic Date: Sun, 12 Jul 2026 20:58:41 +0300 Subject: [PATCH] fix(sentinel): re-elect primary on failover between already-known nodes SwitchPrimary only reconfigured when the new primary was an unknown endpoint. On an in-place failover (old master demoted, replica promoted), the new master is already in `servers`, so it no-op'd and the client kept writing to the demoted node until restart. Reconfigure with reconfigureAll:true when a known primary's cached role is stale, so connected nodes re-read their role before re-election. Also move the switch reconfigure out of the sentinelConnectionChildren lock. Refs #1891 --- .../ConnectionMultiplexer.Sentinel.cs | 100 +++++++++++++++--- 1 file changed, 88 insertions(+), 12 deletions(-) diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs index 35276347d..25412c277 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs @@ -41,6 +41,13 @@ internal void InitializeSentinel(ILogger? log) // We don't care about the result of this - we're just trying _ = Format.TryParseEndPoint(string.Format("{0}:{1}", messageParts[1], messageParts[2]), out var switchBlame); + // Resolve the child under the lock, but run the (potentially multi-second, + // blocking) SwitchPrimary reconfigure OUTSIDE it: otherwise a failover for one + // service would block +switch-master handling and GetSentinelMasterConnection + // for every other service sharing this sentinel connection. SwitchPrimary is + // already invoked without this lock from the reconnect timer and the restore + // handlers, so holding it here is not a required invariant. + ConnectionMultiplexer? switchTarget = null; lock (sentinelConnectionChildren) { // Switch the primary if we have connections for that service @@ -55,10 +62,15 @@ internal void InitializeSentinel(ILogger? log) } else { - SwitchPrimary(switchBlame, child); + switchTarget = child; } } } + + if (switchTarget is not null) + { + SwitchPrimary(switchBlame, switchTarget); + } }, CommandFlags.FireAndForget); } @@ -423,8 +435,30 @@ internal void SwitchPrimary(EndPoint? switchBlame, ConnectionMultiplexer connect connection.currentSentinelPrimaryEndPoint = newPrimaryEndPoint; + // Trigger a blocking reconfigure for the switch, then refresh the sentinel address list. + // reconfigureAll:true forces every already-connected node to re-read its role before the + // primary is re-elected (see IsStalePrimaryView); reconfigureAll:false matches the legacy + // behavior for the "unknown endpoint" rebuild path. + void TriggerReconfigure(bool reconfigureAll) + { + Trace($"Switching primary to {newPrimaryEndPoint}"); + connection.ReconfigureAsync( + first: false, + reconfigureAll: reconfigureAll, + log: logger, + blame: switchBlame, + cause: $"Primary switch {serviceName}", + publishReconfigure: false, + publishReconfigureFlags: CommandFlags.PreferMaster).Wait(); + + UpdateSentinelAddressList(serviceName); + } + if (!connection.servers.Contains(newPrimaryEndPoint)) { + // The sentinel-reported primary is an endpoint we have never seen (e.g. a + // recreated node that came back on a brand-new IP). Rebuild the endpoint set + // from scratch around it and reconfigure. EndPoint[]? replicaEndPoints = GetReplicasForService(serviceName) ?? GetReplicasForService(serviceName); @@ -438,19 +472,61 @@ internal void SwitchPrimary(EndPoint? switchBlame, ConnectionMultiplexer connect connection.EndPoints.TryAdd(replicaEndPoint); } } - Trace($"Switching primary to {newPrimaryEndPoint}"); - // Trigger a reconfigure - connection.ReconfigureAsync( - first: false, - reconfigureAll: false, - log: logger, - blame: switchBlame, - cause: $"Primary switch {serviceName}", - publishReconfigure: false, - publishReconfigureFlags: CommandFlags.PreferMaster).Wait(); - UpdateSentinelAddressList(serviceName); + TriggerReconfigure(reconfigureAll: false); + } + else if (IsStalePrimaryView(connection, newPrimaryEndPoint)) + { + // The sentinel-reported primary IS an endpoint we already know about, but our + // cached view of the topology is stale (failover between known nodes: the old + // primary was demoted to a replica in-place and a former replica was promoted). + // A plain reconfigure with reconfigureAll:false would NOT re-read the role of an + // already-connected node, so the primary election would re-pick the stale demoted + // node. Force a full reconfigure so every connected node's role is refreshed before + // the primary is re-elected. + TriggerReconfigure(reconfigureAll: true); + } + } + + /// + /// Determines whether the multiplexer's cached view of the topology disagrees with the + /// sentinel-reported primary for a known endpoint, and therefore needs a full reconfigure. + /// + /// + /// This is the self-clearing guard that keeps the per-second + /// timer from storming reconfigures: once a switch has succeeded, a full reconfigure has refreshed + /// the new primary's role ( becomes false, it is + /// connected, and no other node is still seen as a primary), so every subsequent tick observes a + /// healthy, correct primary and returns false. + /// + /// The managed connection whose view is being validated. + /// The primary endpoint reported by sentinel (already known to the connection). + private static bool IsStalePrimaryView(ConnectionMultiplexer connection, EndPoint newPrimaryEndPoint) + { + var newPrimaryServer = connection.GetServerEndPoint(newPrimaryEndPoint, activate: false); + + // We do not know this endpoint yet, or we still think the sentinel-reported primary is a + // replica, or we are not actually connected to it: our view is stale. + if (newPrimaryServer is null || newPrimaryServer.IsReplica || !newPrimaryServer.IsConnected) + { + return true; + } + + // The sentinel-reported primary looks correct locally, but double-check that no *other* + // node is still being treated as a connected primary (a demoted node whose role has not + // yet been refreshed). If one exists, our view is split and needs a full reconfigure. + foreach (var server in connection.GetServerSnapshot()) + { + if (server.ServerType != ServerType.Sentinel + && server.IsConnected + && !server.IsReplica + && !Equals(server.EndPoint, newPrimaryEndPoint)) + { + return true; + } } + + return false; } internal void UpdateSentinelAddressList(string serviceName)