Return re-connection failures immediately

hircluster.c

@@ -1902,7 +1902,10 @@ redisContext *ctx_get_by_node(redisClusterContext *cc, redisClusterNode *node) {
     c = node->con;
     if (c != NULL) {
         if (c->err) {
-            redisReconnect(c);
+            if (redisReconnect(c) != REDIS_OK) {
+                __redisClusterSetError(cc, c->err, c->errstr);
+                return NULL;
+            }
 
             if (cc->ssl && cc->ssl_init_fn(c, cc->ssl) != REDIS_OK) {
                 __redisClusterSetError(cc, c->err, c->errstr);

tests/CMakeLists.txt

@@ -220,6 +220,10 @@ add_test(NAME redirect-with-hostname-test
          COMMAND "${CMAKE_SOURCE_DIR}/tests/scripts/redirect-with-hostname-test.sh"
                  "$<TARGET_FILE:clusterclient>"
          WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/tests/scripts/")
+add_test(NAME reconnect-failure-test
+         COMMAND "${CMAKE_SOURCE_DIR}/tests/scripts/reconnect-failure-test.sh"
+                 "$<TARGET_FILE:clusterclient>"
+         WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/tests/scripts/")
 # This test can't be run on hiredis v1.1.0 due to hiredis issue #1171.
 # Disabling the testcase if hiredis contains the issue or if the version is unknown.
 add_test(NAME redirect-with-hostname-test-async

tests/clusterclient.c

@@ -22,23 +22,30 @@ int main(int argc, char **argv) {
     redisClusterSetOptionAddNodes(cc, initnode);
     redisClusterSetOptionConnectTimeout(cc, timeout);
     redisClusterSetOptionRouteUseSlots(cc);
+    redisClusterSetOptionMaxRetry(cc, 2);
 
     if (redisClusterConnect2(cc) != REDIS_OK) {
         fprintf(stderr, "Connect error: %s\n", cc->errstr);
         exit(100);
     }
 
-    char command[256];
-    while (fgets(command, 256, stdin)) {
-        size_t len = strlen(command);
-        if (command[len - 1] == '\n') // Chop trailing line break
-            command[len - 1] = '\0';
-        redisReply *reply = (redisReply *)redisClusterCommand(cc, command);
+    char cmd[256];
+    while (fgets(cmd, 256, stdin)) {
+        size_t len = strlen(cmd);
+        if (cmd[len - 1] == '\n') // Chop trailing line break
+            cmd[len - 1] = '\0';
+
+        if (cmd[0] == '\0') /* Skip empty lines */
+            continue;
+        if (cmd[0] == '#') /* Skip comments */
+            continue;
+
+        redisReply *reply = (redisReply *)redisClusterCommand(cc, cmd);
         if (cc->err) {
-            fprintf(stderr, "redisClusterCommand error: %s\n", cc->errstr);
-            exit(101);
+            printf("error: %s\n", cc->errstr);
+        } else {
+            printf("%s\n", reply->str);
         }
-        printf("%s\n", reply->str);
         freeReplyObject(reply);
     }
 

tests/scripts/reconnect-failure-test.sh

@@ -0,0 +1,85 @@
+#!/bin/bash
+#
+# Simulate a 2 node cluster.
+# - Node 1 will handle slotmap updates and redirects.
+# - Node 2 will simulate a short and temporary network issue
+#   that trigger the client to reconnect.
+#
+# Usage: $0 /path/to/clusterclient-binary
+
+clientprog=${1:-./clusterclient}
+testname=reconnect-failure-test
+
+# Sync process just waiting for server to be ready to accept connection.
+perl -we 'use sigtrap "handler", sub{exit}, "CONT"; sleep 1; die "timeout"' &
+syncpid1=$!
+perl -we 'use sigtrap "handler", sub{exit}, "CONT"; sleep 1; die "timeout"' &
+syncpid2=$!
+
+# Start simulated redis node #1
+timeout 5s ./simulated-redis.pl -p 7401 -d --sigcont $syncpid1 <<'EOF' &
+# Inital slotmap
+EXPECT CONNECT
+EXPECT ["CLUSTER", "SLOTS"]
+SEND [[0, 6000, ["127.0.0.1", 7401, "nodeid1"]],[6001, 16383, ["127.0.0.1", 7402, "nodeid2"]]]
+EXPECT CLOSE
+
+EXPECT CONNECT
+EXPECT ["SET", "bar", "initial"]
+SEND +OK
+
+EXPECT CLOSE
+EOF
+server1=$!
+
+# Start simulated redis node #2
+timeout 5s ./simulated-redis.pl -p 7402 -d --sigcont $syncpid2 <<'EOF' &
+EXPECT CONNECT
+EXPECT ["SET", "foo", "initial"]
+SEND +OK
+CLOSE
+EOF
+server2=$!
+
+# Wait until both nodes are ready to accept client connections
+wait $syncpid1 $syncpid2;
+
+# Run client
+timeout 4s "$clientprog" 127.0.0.1:7401 > "$testname.out" <<'EOF'
+SET foo initial
+SET bar initial
+
+SET foo first
+SET foo second
+
+EOF
+clientexit=$?
+
+# Wait for servers to exit
+wait $server1; server1exit=$?
+wait $server2; server2exit=$?
+
+# Check exit statuses
+if [ $server1exit -ne 0 ]; then
+    echo "Simulated server #1 exited with status $server1exit"
+    exit $server1exit
+fi
+if [ $server2exit -ne 0 ]; then
+    echo "Simulated server #2 exited with status $server2exit"
+    exit $server2exit
+fi
+if [ $clientexit -ne 0 ]; then
+    echo "$clientprog exited with status $clientexit"
+    exit $clientexit
+fi
+
+# Check the output from clusterclient
+expected="OK
+OK
+error: Server closed the connection
+error: Connection refused"
+
+cmp "$testname.out" <(echo "$expected") || exit 99
+
+# Clean up
+rm "$testname.out"