Sentinel: re-elect primary on failover between already-known nodes

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);
+        }
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// <remarks>
+    /// This is the self-clearing guard that keeps the per-second <see cref="OnManagedConnectionFailed"/>
+    /// timer from storming reconfigures: once a switch has succeeded, a full reconfigure has refreshed
+    /// the new primary's role (<see cref="ServerEndPoint.IsReplica"/> becomes <c>false</c>, it is
+    /// connected, and no other node is still seen as a primary), so every subsequent tick observes a
+    /// healthy, correct primary and returns <c>false</c>.
+    /// </remarks>
+    /// <param name="connection">The managed connection whose view is being validated.</param>
+    /// <param name="newPrimaryEndPoint">The primary endpoint reported by sentinel (already known to the connection).</param>
+    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)