Align to review comments

valkey-io · Dec 14, 2024 · 36adcad · 36adcad
1 parent 9325141
commit 36adcad
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 47 deletions.
diff --git a/src/server.c b/src/server.c
@@ -4259,10 +4259,10 @@ int processCommand(client *c) {
         return C_OK;
     }
 
-    /* Prevent a replica (but not a monitor client) from sending commands that access the keyspace.
+    /* Prevent a replica from sending commands that access the keyspace.
      * The main objective here is to prevent abuse of client pause check
      * from which replicas are exempt. */
-    if ((c->flag.replica && !c->flag.monitor) && (is_may_replicate_command || is_write_command || is_read_command)) {
+    if (c->flag.replica && (is_may_replicate_command || is_write_command || is_read_command)) {
         rejectCommandFormat(c, "Replica can't interact with the keyspace");
         return C_OK;
     }
@@ -6182,8 +6182,11 @@ void monitorCommand(client *c) {
         return;
     }
 
-    /* ignore MONITOR if already replica or in monitor mode */
-    if (c->flag.replica) return;
+    /* Gently notify the client that the monitor command has already been issued. */
+    if (c->flag.replica) {
+        addReplyError(c, "The connection is already in monitoring mode.");
+        return;
+    }
 
     c->flag.replica = 1;
     c->flag.monitor = 1;

diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl
@@ -317,52 +317,20 @@ start_server {tags {"introspection"}} {
         $rd close
     }
 
-    test {MONITOR and CLIENT TRACKING should work on the same connection with RESP3} {
-        set rd1 [valkey_deferring_client]
-        set rd2 [valkey_deferring_client]
-
-        $rd1 HELLO 3
-        $rd1 read ; # Consume the HELLO reply
-
-        $rd1 client tracking on
-        $rd1 read ; # Consume the TRACKING reply
-
-        $rd1 monitor
-        $rd1 read ; # Consume the MONITOR reply
-
-        $rd1 set foo bar
-        assert_equal "OK" [$rd1 read]
-        assert_match {monitor*"set"*"foo"*"bar"*} [$rd1 read]
-
-        # Because we need to verify exact RESP3 response correctness,
-        # we need to instruct valkey client to return raw, unparsed response.
-        $rd1 readraw 1 ;
-
-        $rd1 get foo
-        assert_equal "\$3" [$rd1 read]
-        assert_equal "bar" [$rd1 read]
-
-        assert_equal ">2" [$rd1 read]
-        assert_equal "\$7" [$rd1 read]
-        assert_equal "monitor" [$rd1 read]
-        assert_match {*"get"*"foo"*} [$rd1 read]
-
-        $rd2 set foo baz
+    test {multiple MONITOR commands should result in ERR} {
+        set rd [valkey_deferring_client]
+        $rd HELLO 3
+        $rd read ; # Consume the HELLO reply
 
-        assert_equal ">2" [$rd1 read]
-        assert_equal "\$10" [$rd1 read]
-        assert_equal "invalidate" [$rd1 read]
-        assert_equal "*1" [$rd1 read]
-        assert_equal "\$3" [$rd1 read]
-        assert_equal "foo" [$rd1 read]
+        $rd readraw 1 ;
 
-        assert_equal ">2" [$rd1 read]
-        assert_equal "\$7" [$rd1 read]
-        assert_equal "monitor" [$rd1 read]
-        assert_match {*"set"*"foo"*"baz"*} [$rd1 read]
+        $rd monitor
+        assert_equal "+OK" [$rd read]
 
-        $rd1 close
-        $rd2 close
+        $rd monitor
+        assert_equal "-ERR The connection is already in monitoring mode." [$rd read]
+
+        $rd close
     }
 
     test {MONITOR can log commands issued by the scripting engine} {