Fix #2249: Handle fail on cluster node responses appropriately (#2288)

Right now we don't pay attention to fail state (PFAIL == FAIL) and continue trying to connect in the main loop. I don't believe this was intended looking at the code, we just weren't handling the flag appropriately. Added now.

Docs at: https://redis.io/commands/cluster-nodes/

Author: NickCraver

docs/ReleaseNotes.md

@@ -9,6 +9,7 @@ Current package versions:
 ## Unreleased
 
 - Fix [#1520](https://github.com/StackExchange/StackExchange.Redis/issues/1520) & [#1660](https://github.com/StackExchange/StackExchange.Redis/issues/1660): When `MOVED` is encountered from a cluster, a reconfigure will happen proactively to react to cluster changes ASAP ([#2286 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2286))
+- Fix [#2249](https://github.com/StackExchange/StackExchange.Redis/issues/2249): Properly handle a `fail` state (new `ClusterNode.IsFail` property) for `CLUSTER NODES` and expose `fail?` as a property (`IsPossiblyFail`) as well ([#2288 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2288))
 
 
 ## 2.6.80

src/StackExchange.Redis/ClusterConfiguration.cs

@@ -171,7 +171,7 @@ internal ClusterConfiguration(ServerSelectionStrategy serverSelectionStrategy, s
                     var node = new ClusterNode(this, line, origin);
 
                     // Be resilient to ":0 {primary,replica},fail,noaddr" nodes, and nodes where the endpoint doesn't parse
-                    if (node.IsNoAddr || node.EndPoint == null)
+                    if (node.IsNoAddr || node.IsFail || node.EndPoint == null)
                         continue;
 
                     // Override the origin value with the endpoint advertised with the target node to
@@ -301,6 +301,8 @@ internal ClusterNode(ClusterConfiguration configuration, string raw, EndPoint or
             }
 
             NodeId = parts[0];
+            IsFail = flags.Contains("fail");
+            IsPossiblyFail = flags.Contains("fail?");
             IsReplica = flags.Contains("slave") || flags.Contains("replica");
             IsNoAddr = flags.Contains("noaddr");
             ParentNodeId = string.IsNullOrWhiteSpace(parts[3]) ? null : parts[3];
@@ -345,6 +347,17 @@ public IList<ClusterNode> Children
         /// </summary>
         public EndPoint? EndPoint { get; }
 
+        /// <summary>
+        /// Gets whether this node is in a failed state.
+        /// </summary>
+        public bool IsFail { get; }
+
+        /// <summary>
+        /// Gets whether this node is possibly in a failed state.
+        /// Possibly here means the node we're getting status from can't communicate with it, but doesn't mean it's down for sure.
+        /// </summary>
+        public bool IsPossiblyFail { get; }
+
         /// <summary>
         /// Gets whether this is the node which responded to the CLUSTER NODES request.
         /// </summary>

src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt

@@ -140,8 +140,10 @@ StackExchange.Redis.ClusterNode.CompareTo(StackExchange.Redis.ClusterNode? other
 StackExchange.Redis.ClusterNode.EndPoint.get -> System.Net.EndPoint?
 StackExchange.Redis.ClusterNode.Equals(StackExchange.Redis.ClusterNode? other) -> bool
 StackExchange.Redis.ClusterNode.IsConnected.get -> bool
+StackExchange.Redis.ClusterNode.IsFail.get -> bool
 StackExchange.Redis.ClusterNode.IsMyself.get -> bool
 StackExchange.Redis.ClusterNode.IsNoAddr.get -> bool
+StackExchange.Redis.ClusterNode.IsPossiblyFail.get -> bool
 StackExchange.Redis.ClusterNode.IsReplica.get -> bool
 StackExchange.Redis.ClusterNode.IsSlave.get -> bool
 StackExchange.Redis.ClusterNode.NodeId.get -> string!

tests/StackExchange.Redis.Tests/CommandTimeoutTests.cs

@@ -10,7 +10,7 @@ public class CommandTimeoutTests : TestBase
 {
     public CommandTimeoutTests(ITestOutputHelper output) : base (output) { }
 
-    [Fact]
+    [FactLongRunning]
     public async Task DefaultHeartbeatTimeout()
     {
         var options = ConfigurationOptions.Parse(TestConfig.Current.PrimaryServerAndPort);
@@ -21,15 +21,15 @@ public async Task DefaultHeartbeatTimeout()
         using var conn = ConnectionMultiplexer.Connect(options);
 
         var pauseServer = GetServer(pauseConn);
-        var pauseTask = pauseServer.ExecuteAsync("CLIENT", "PAUSE", 2500);
+        var pauseTask = pauseServer.ExecuteAsync("CLIENT", "PAUSE", 5000);
 
         var key = Me();
         var db = conn.GetDatabase();
         var sw = ValueStopwatch.StartNew();
         var ex = await Assert.ThrowsAsync<RedisTimeoutException>(async () => await db.StringGetAsync(key));
         Log(ex.Message);
         var duration = sw.GetElapsedTime();
-        Assert.True(duration < TimeSpan.FromSeconds(2100), $"Duration ({duration.Milliseconds} ms) should be less than 2100ms");
+        Assert.True(duration < TimeSpan.FromSeconds(4000), $"Duration ({duration.Milliseconds} ms) should be less than 4000ms");
 
         // Await as to not bias the next test
         await pauseTask;

tests/StackExchange.Redis.Tests/ConfigTests.cs

@@ -414,7 +414,7 @@ public async Task TestAutomaticHeartbeat()
             await Task.Delay(8000).ForAwait();
 
             var after = innerConn.OperationCount;
-            Assert.True(after >= before + 2, $"after: {after}, before: {before}");
+            Assert.True(after >= before + 1, $"after: {after}, before: {before}");
         }
         finally
         {