RM_Call - only enforce OOM on scripts if 'M' flag is sent (#11425)

RM_Call is designed to let modules call redis commands disregarding the
OOM state (the module is responsible to declare its command flags to redis,
or perform the necessary checks).
The other (new) alternative is to pass the "M" flag to RM_Call so that redis can
OOM reject commands implicitly.

However, Currently, RM_Call enforces OOM on scripts (excluding scripts that
declared `allow-oom`) in all cases, regardless of the RM_Call "M" flag being present.

This PR fixes scripts to be consistent with other commands being executed by RM_Call.
It modifies the flow in effect treats scripts as if they if they have the ALLOW_OOM script
flag, if the "M" flag is not passed (i.e. no OOM checking is being performed by RM_Call,
so no OOM checking should be done on script).

Co-authored-by: Oran Agra <oran@redislabs.com>

1 files changed, 43 insertions, 7 deletions

diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl
index 15e4f4c6c..0f076c6b5 100644
--- a/tests/unit/moduleapi/misc.tcl
+++ b/tests/unit/moduleapi/misc.tcl
@@ -245,29 +245,40 @@ start_server {tags {"modules"}} {
         }
     }
 
-    test {rm_call EVAL - OOM} {
+    # Note: each script is unique, to check that flags are extracted correctly
+    test {rm_call EVAL - OOM - with M flag} {
         r config set maxmemory 1
 
-        assert_error {OOM command not allowed when used memory > 'maxmemory'. script*} {
-            r test.rm_call eval {
+        # script without shebang, but uses SET, so fails
+        assert_error {*OOM command not allowed when used memory > 'maxmemory'*} {
+            r test.rm_call_flags M eval {
                 redis.call('set','x',1)
                 return 1
             } 1 x
         }
 
-        r test.rm_call eval {#!lua flags=no-writes
+        # script with an allow-oom flag, succeeds despite using SET
+        r test.rm_call_flags M eval {#!lua flags=allow-oom
+            redis.call('set','x', 1)
+            return 2
+        } 1 x
+
+        # script with no-writes flag, implies allow-oom, succeeds
+        r test.rm_call_flags M eval {#!lua flags=no-writes
             redis.call('get','x')
             return 2
         } 1 x
 
-        assert_error {OOM allow-oom flag is not set on the script,*} {
-            r test.rm_call eval {#!lua
+        # script with shebang using default flags, so fails regardless of using only GET
+        assert_error {*OOM command not allowed when used memory > 'maxmemory'*} {
+            r test.rm_call_flags M eval {#!lua
                 redis.call('get','x')
                 return 3
             } 1 x
         }
 
-        r test.rm_call eval {
+        # script without shebang, but uses GET, so succeeds
+        r test.rm_call_flags M eval {
             redis.call('get','x')
             return 4
         } 1 x
@@ -275,6 +286,31 @@ start_server {tags {"modules"}} {
         r config set maxmemory 0
     } {OK} {needs:config-maxmemory}
 
+    # All RM_Call for script succeeds in OOM state without using the M flag
+    test {rm_call EVAL - OOM - without M flag} {
+        r config set maxmemory 1
+
+        # no shebang at all
+        r test.rm_call eval {
+            redis.call('set','x',1)
+            return 6
+        } 1 x
+
+        # Shebang without flags
+        r test.rm_call eval {#!lua
+            redis.call('set','x', 1)
+            return 7
+        } 1 x
+
+        # with allow-oom flag
+        r test.rm_call eval {#!lua flags=allow-oom
+            redis.call('set','x', 1)
+            return 8
+        } 1 x
+
+        r config set maxmemory 0
+    } {OK} {needs:config-maxmemory}
+
     test "not enough good replicas" {
         r set x "some value"
         r config set min-replicas-to-write 1