Handle role change error in cluster setslot when migrating the las…

…t slot away with allow-replica-migration enabled (valkey-io#466)
hwware · May 10, 2024 · 138a7d9 · 138a7d9
1 parent 8048abb
commit 138a7d9
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 8 deletions.
diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c
@@ -6320,7 +6320,7 @@ int clusterParseSetSlotCommand(client *c, int *slot_out, clusterNode **node_out,
     return 1;
 }
 
-void clusterSetSlotCommand(client *c) {
+void clusterCommandSetSlot(client *c) {
     int slot;
     int timeout_ms;
     clusterNode *n;
@@ -6575,7 +6575,7 @@ int clusterCommandSpecial(client *c) {
         /* SETSLOT 10 IMPORTING <node ID> */
         /* SETSLOT 10 STABLE */
         /* SETSLOT 10 NODE <node ID> */
-        clusterSetSlotCommand(c);
+        clusterCommandSetSlot(c);
     } else if (!strcasecmp(c->argv[1]->ptr,"bumpepoch") && c->argc == 2) {
         /* CLUSTER BUMPEPOCH */
         int retval = clusterBumpConfigEpochWithoutConsensus();

diff --git a/src/valkey-cli.c b/src/valkey-cli.c
@@ -5241,13 +5241,30 @@ static int clusterManagerMoveSlot(clusterManagerNode *source,
         /* Inform the source node. If the source node has just lost its last
          * slot and the target node has already informed the source node, the
          * source node has turned itself into a replica. This is not an error in
-         * this scenario so we ignore it. See issue #9223. */
+         * this scenario so we ignore it. See issue #9223.
+         *
+         * Another acceptable error can arise now that the primary pre-replicates
+         * `cluster setslot` commands to replicas while blocking the client on the
+         * primary. This change enhances the reliability of `cluster setslot` in
+         * the face of primary failures. However, while our client is blocked on
+         * the primary awaiting replication, the primary might become a replica
+         * for the same reason as mentioned above, resulting in the client being
+         * unblocked with the role change error. */
         success = clusterManagerSetSlot(source, target, slot, "node", err);
-        const char *acceptable = "ERR Please use SETSLOT only with masters.";
-        if (!success && err && !strncmp(*err, acceptable, strlen(acceptable))) {
-            zfree(*err);
-            *err = NULL;
-        } else if (!success && err) {
+        if (!success && err) {
+            const char *acceptable[] = {
+                "ERR Please use SETSLOT only with masters.",
+                "UNBLOCKED"};
+            for (size_t i = 0; i < sizeof(acceptable)/sizeof(acceptable[0]); i++) {
+                if (!strncmp(*err, acceptable[i], strlen(acceptable[i]))) {
+                    zfree(*err);
+                    *err = NULL;
+                    success = 1;
+                    break;
+                }
+            }
+        }
+        if (!success && err) {
             return 0;
         }