From b120366d48d9e488a406965773e64f29ba2946f7 Mon Sep 17 00:00:00 2001
From: Eran Liberty <eran.liberty@gmail.com>
Date: Wed, 9 Sep 2020 09:35:42 +0300
Subject: [PATCH] Allow exec with read commands on readonly replica in cluster
 (#7766)

There was a bug. Although cluster replicas would allow read commands,
they would not allow a MULTI-EXEC that's composed solely of read commands.
Adds tests for coverage.

Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Eran Liberty <eranl@amazon.com>
---
 src/cluster.c                                 |  4 +-
 .../tests/16-transactions-on-replica.tcl      | 48 +++++++++++++++++++
 tests/instances.tcl                           | 10 +++-
 3 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100644 tests/cluster/tests/16-transactions-on-replica.tcl

diff --git a/src/cluster.c b/src/cluster.c
index ba0ada04a..b9a0c8661 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -5769,8 +5769,10 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in
     /* Handle the read-only client case reading from a slave: if this
      * node is a slave and the request is about an hash slot our master
      * is serving, we can reply without redirection. */
+    int is_readonly_command = (c->cmd->flags & CMD_READONLY) ||
+                              (c->cmd->proc == execCommand && !(c->mstate.cmd_inv_flags & CMD_READONLY));
     if (c->flags & CLIENT_READONLY &&
-        (cmd->flags & CMD_READONLY || cmd->proc == evalCommand ||
+        (is_readonly_command || cmd->proc == evalCommand ||
          cmd->proc == evalShaCommand) &&
         nodeIsSlave(myself) &&
         myself->slaveof == n)
diff --git a/tests/cluster/tests/16-transactions-on-replica.tcl b/tests/cluster/tests/16-transactions-on-replica.tcl
new file mode 100644
index 000000000..da9dff1ca
--- /dev/null
+++ b/tests/cluster/tests/16-transactions-on-replica.tcl
@@ -0,0 +1,48 @@
+# Check basic transactions on a replica.
+
+source "../tests/includes/init-tests.tcl"
+
+test "Create a primary with a replica" {
+    create_cluster 1 1
+}
+
+test "Cluster should start ok" {
+    assert_cluster_state ok
+}
+
+set primary [Rn 0]
+set replica [Rn 1]
+
+test "Cant read from replica without READONLY" {
+    $primary SET a 1
+    catch {$replica GET a} err
+    assert {[string range $err 0 4] eq {MOVED}}
+}
+
+test "Can read from replica after READONLY" {
+    $replica READONLY
+    assert {[$replica GET a] eq {1}}
+}
+
+test "Can preform HSET primary and HGET from replica" {
+    $primary HSET h a 1
+    $primary HSET h b 2
+    $primary HSET h c 3
+    assert {[$replica HGET h a] eq {1}}
+    assert {[$replica HGET h b] eq {2}}
+    assert {[$replica HGET h c] eq {3}}
+}
+
+test "Can MULTI-EXEC transaction of HGET operations from replica" {
+    $replica MULTI
+    assert {[$replica HGET h a] eq {QUEUED}}
+    assert {[$replica HGET h b] eq {QUEUED}}
+    assert {[$replica HGET h c] eq {QUEUED}}
+    assert {[$replica EXEC] eq {1 2 3}}
+}
+
+test "MULTI-EXEC with write operations is MOVED" {
+    $replica MULTI
+    catch {$replica HSET h b 4} err
+    assert {[string range $err 0 4] eq {MOVED}}
+}
diff --git a/tests/instances.tcl b/tests/instances.tcl
index 82c35854b..2199cfcd4 100644
--- a/tests/instances.tcl
+++ b/tests/instances.tcl
@@ -422,10 +422,16 @@ proc S {n args} {
     [dict get $s link] {*}$args
 }
 
+# Returns a Redis instance by index.
+# Example:
+#     [Rn 0] info
+proc Rn {n} {
+    return [dict get [lindex $::redis_instances $n] link]
+}
+
 # Like R but to chat with Redis instances.
 proc R {n args} {
-    set r [lindex $::redis_instances $n]
-    [dict get $r link] {*}$args
+    [Rn $n] {*}$args
 }
 
 proc get_info_field {info field} {