fix: inconsistency moved readonly client

Signed-off-by: proost <[email protected]>
proost · Nov 9, 2024 · 52b82fa · 52b82fa
1 parent 247a8f2
commit 52b82fa
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 0 deletions.
diff --git a/src/script.c b/src/script.c
@@ -373,6 +373,33 @@ static int scriptVerifyWriteCommandAllow(scriptRunCtx *run_ctx, char **err) {
      * of this script. */
     int deny_write_type = writeCommandsDeniedByDiskError();
 
+    /*
+     * If client is readonly and server is a replica, we should not allow write-commands.
+     * we should redirect the client.
+     */
+    if (run_ctx->original_client->flag.readonly && server.primary_host) {
+        client *c = run_ctx->c;
+        client *original_c = run_ctx->original_client;
+
+        /*
+         * Duplicate relevant flags in the script client.
+         */
+        c->flag.readonly = original_c->flag.readonly;
+        c->flag.asking = original_c->flag.asking;
+
+        int error_code;
+        int hashslot = -1;
+        clusterNode *n = getNodeByQuery(c, c->cmd, c->argv, c->argc, &hashslot, &error_code);
+        if (n == NULL || !clusterNodeIsMyself(n)) {
+            if (error_code == CLUSTER_REDIR_MOVED || error_code == CLUSTER_REDIR_ASK) {
+                int port = clusterNodeClientPort(n, connIsTLS(original_c->conn));
+                *err = sdscatprintf(sdsempty(), "-%s %d %s:%d", (error_code == CLUSTER_REDIR_ASK) ? "ASK" : "MOVED",
+                                    hashslot, clusterNodePreferredEndpoint(n, c), port);
+                return C_ERR;
+            }
+        }
+    }
+
     if (server.primary_host && server.repl_replica_ro && !mustObeyClient(run_ctx->original_client)) {
         *err = sdsdup(shared.roreplicaerr->ptr);
         return C_ERR;

diff --git a/tests/unit/cluster/scripting.tcl b/tests/unit/cluster/scripting.tcl
@@ -97,3 +97,39 @@ start_cluster 1 0 {tags {external:skip cluster}} {
         assert_equal [lsort [r 0 cluster shards]] [lsort [r 0 eval "return redis.call('cluster', 'shards')" 0]]
     }
 }
+
+start_cluster 2 2 {tags {external:skip cluster}} {
+    test "Cluster should start ok" {
+        wait_for_cluster_state ok
+    }
+
+    set primary1 [srv 0 "client"]
+    set primary2 [srv -1 "client"]
+    set replica1 [srv -2 "client"]
+    set replica2 [srv -3 "client"]
+
+    set slot_for_foo [$primary2 cluster keyslot foo]
+
+    test "Read-only client that sends lua script which has write command on replica get MOVED error" {
+        assert_equal {OK} [$replica2 READONLY]
+
+        catch {
+            $replica2 eval "redis.call('set', 'foo', 'bar')" 0
+        } e
+
+        assert_match {*MOVED*} $e
+    }
+
+    test "Read-only client that sends lua script which has write command on replica get ASK error during migration" {
+        assert_equal {OK} [$primary1 cluster setslot $slot_for_foo importing [$primary2 cluster myid]]
+        assert_equal {OK} [$primary2 cluster setslot $slot_for_foo migrating [$primary1 cluster myid]]
+
+        assert_equal {OK} [$replica2 READONLY]
+
+        catch {
+            $replica2 eval "redis.call('set', 'foo', 'bar')" 0
+        } e
+
+        assert_match {*ASK*} $e
+    }
+}