diff --git a/src/aof.c b/src/aof.c index 485fa1960..823759f24 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1502,7 +1502,10 @@ int loadSingleAppendOnlyFile(char *filename) { if (fakeClient->flags & CLIENT_MULTI && fakeClient->cmd->proc != execCommand) { - queueMultiCommand(fakeClient); + /* Note: we don't have to attempt calling evalGetCommandFlags, + * since this is AOF, the checks in processCommand are not made + * anyway.*/ + queueMultiCommand(fakeClient, cmd->flags); } else { cmd->proc(fakeClient); } diff --git a/src/eval.c b/src/eval.c index 332aec9ce..e2ee41cb5 100644 --- a/src/eval.c +++ b/src/eval.c @@ -285,6 +285,124 @@ void scriptingReset(int async) { * EVAL and SCRIPT commands implementation * ------------------------------------------------------------------------- */ +static void evalCalcFunctionName(int evalsha, sds script, char *out_funcname) { + /* We obtain the script SHA1, then check if this function is already + * defined into the Lua state */ + out_funcname[0] = 'f'; + out_funcname[1] = '_'; + if (!evalsha) { + /* Hash the code if this is an EVAL call */ + sha1hex(out_funcname+2,script,sdslen(script)); + } else { + /* We already have the SHA if it is an EVALSHA */ + int j; + char *sha = script; + + /* Convert to lowercase. We don't use tolower since the function + * managed to always show up in the profiler output consuming + * a non trivial amount of time. */ + for (j = 0; j < 40; j++) + out_funcname[j+2] = (sha[j] >= 'A' && sha[j] <= 'Z') ? + sha[j]+('a'-'A') : sha[j]; + out_funcname[42] = '\0'; + } +} + +/* Helper function to try and extract shebang flags from the script body. + * If no shebang is found, return with success and COMPAT mode flag. + * The err arg is optional, can be used to get a detailed error string. + * The out_shebang_len arg is optional, can be used to trim the shebang from the script. + * Returns C_OK on success, and C_ERR on error. */ +int evalExtractShebangFlags(sds body, uint64_t *out_flags, ssize_t *out_shebang_len, sds *err) { + ssize_t shebang_len = 0; + uint64_t script_flags = SCRIPT_FLAG_EVAL_COMPAT_MODE; + if (!strncmp(body, "#!", 2)) { + int numparts,j; + char *shebang_end = strchr(body, '\n'); + if (shebang_end == NULL) { + if (err) + *err = sdsnew("Invalid script shebang"); + return C_ERR; + } + shebang_len = shebang_end - body; + sds shebang = sdsnewlen(body, shebang_len); + sds *parts = sdssplitargs(shebang, &numparts); + sdsfree(shebang); + if (!parts || numparts == 0) { + if (err) + *err = sdsnew("Invalid engine in script shebang"); + sdsfreesplitres(parts, numparts); + return C_ERR; + } + /* Verify lua interpreter was specified */ + if (strcmp(parts[0], "#!lua")) { + if (err) + *err = sdscatfmt(sdsempty(), "Unexpected engine in script shebang: %s", parts[0]); + sdsfreesplitres(parts, numparts); + return C_ERR; + } + script_flags &= ~SCRIPT_FLAG_EVAL_COMPAT_MODE; + for (j = 1; j < numparts; j++) { + if (!strncmp(parts[j], "flags=", 6)) { + sdsrange(parts[j], 6, -1); + int numflags, jj; + sds *flags = sdssplitlen(parts[j], sdslen(parts[j]), ",", 1, &numflags); + for (jj = 0; jj < numflags; jj++) { + scriptFlag *sf; + for (sf = scripts_flags_def; sf->flag; sf++) { + if (!strcmp(flags[jj], sf->str)) break; + } + if (!sf->flag) { + if (err) + *err = sdscatfmt(sdsempty(), "Unexpected flag in script shebang: %s", flags[jj]); + sdsfreesplitres(flags, numflags); + sdsfreesplitres(parts, numparts); + return C_ERR; + } + script_flags |= sf->flag; + } + sdsfreesplitres(flags, numflags); + } else { + /* We only support function flags options for lua scripts */ + if (err) + *err = sdscatfmt(sdsempty(), "Unknown lua shebang option: %s", parts[j]); + sdsfreesplitres(parts, numparts); + return C_ERR; + } + } + sdsfreesplitres(parts, numparts); + } + if (out_shebang_len) + *out_shebang_len = shebang_len; + *out_flags = script_flags; + return C_OK; +} + +/* Try to extract command flags if we can, returns the modified flags. + * Note that it does not guarantee the command arguments are right. */ +uint64_t evalGetCommandFlags(client *c, uint64_t cmd_flags) { + char funcname[43]; + int evalsha = c->cmd->proc == evalShaCommand || c->cmd->proc == evalShaRoCommand; + if (evalsha && sdslen(c->argv[1]->ptr) != 40) + return cmd_flags; + evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname); + char *lua_cur_script = funcname + 2; + dictEntry *de = dictFind(lctx.lua_scripts, lua_cur_script); + uint64_t script_flags; + if (!de) { + if (evalsha) + return cmd_flags; + if (evalExtractShebangFlags(c->argv[1]->ptr, &script_flags, NULL, NULL) == C_ERR) + return cmd_flags; + } else { + luaScript *l = dictGetVal(de); + script_flags = l->flags; + } + if (script_flags & SCRIPT_FLAG_EVAL_COMPAT_MODE) + return cmd_flags; + return scriptFlagsToCmdFlags(cmd_flags, script_flags); +} + /* Define a Lua function with the specified body. * The function name will be generated in the following form: * @@ -305,7 +423,7 @@ void scriptingReset(int async) { sds luaCreateFunction(client *c, robj *body) { char funcname[43]; dictEntry *de; - uint64_t script_flags = SCRIPT_FLAG_EVAL_COMPAT_MODE; + uint64_t script_flags; funcname[0] = 'f'; funcname[1] = '_'; @@ -317,56 +435,10 @@ sds luaCreateFunction(client *c, robj *body) { /* Handle shebang header in script code */ ssize_t shebang_len = 0; - if (!strncmp(body->ptr, "#!", 2)) { - int numparts,j; - char *shebang_end = strchr(body->ptr, '\n'); - if (shebang_end == NULL) { - addReplyError(c,"Invalid script shebang"); - return NULL; - } - shebang_len = shebang_end - (char*)body->ptr; - sds shebang = sdsnewlen(body->ptr, shebang_len); - sds *parts = sdssplitargs(shebang, &numparts); - sdsfree(shebang); - if (!parts || numparts == 0) { - addReplyError(c,"Invalid engine in script shebang"); - sdsfreesplitres(parts, numparts); - return NULL; - } - /* Verify lua interpreter was specified */ - if (strcmp(parts[0], "#!lua")) { - addReplyErrorFormat(c,"Unexpected engine in script shebang: %s", parts[0]); - sdsfreesplitres(parts, numparts); - return NULL; - } - script_flags &= ~SCRIPT_FLAG_EVAL_COMPAT_MODE; - for (j = 1; j < numparts; j++) { - if (!strncmp(parts[j], "flags=", 6)) { - sdsrange(parts[j], 6, -1); - int numflags, jj; - sds *flags = sdssplitlen(parts[j], sdslen(parts[j]), ",", 1, &numflags); - for (jj = 0; jj < numflags; jj++) { - scriptFlag *sf; - for (sf = scripts_flags_def; sf->flag; sf++) { - if (!strcmp(flags[jj], sf->str)) break; - } - if (!sf->flag) { - addReplyErrorFormat(c,"Unexpected flag in script shebang: %s", flags[jj]); - sdsfreesplitres(flags, numflags); - sdsfreesplitres(parts, numparts); - return NULL; - } - script_flags |= sf->flag; - } - sdsfreesplitres(flags, numflags); - } else { - /* We only support function flags options for lua scripts */ - addReplyErrorFormat(c,"Unknown lua shebang option: %s", parts[j]); - sdsfreesplitres(parts, numparts); - return NULL; - } - } - sdsfreesplitres(parts, numparts); + sds err = NULL; + if (evalExtractShebangFlags(body->ptr, &script_flags, &shebang_len, &err) == C_ERR) { + addReplyErrorSds(c, err); + return NULL; } /* Note that in case of a shebang line we skip it but keep the line feed to conserve the user's line numbers */ @@ -430,26 +502,7 @@ void evalGenericCommand(client *c, int evalsha) { return; } - /* We obtain the script SHA1, then check if this function is already - * defined into the Lua state */ - funcname[0] = 'f'; - funcname[1] = '_'; - if (!evalsha) { - /* Hash the code if this is an EVAL call */ - sha1hex(funcname+2,c->argv[1]->ptr,sdslen(c->argv[1]->ptr)); - } else { - /* We already have the SHA if it is an EVALSHA */ - int j; - char *sha = c->argv[1]->ptr; - - /* Convert to lowercase. We don't use tolower since the function - * managed to always show up in the profiler output consuming - * a non trivial amount of time. */ - for (j = 0; j < 40; j++) - funcname[j+2] = (sha[j] >= 'A' && sha[j] <= 'Z') ? - sha[j]+('a'-'A') : sha[j]; - funcname[42] = '\0'; - } + evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname); /* Push the pcall error handler function on the stack. */ lua_getglobal(lua, "__redis__err__handler"); diff --git a/src/functions.c b/src/functions.c index 29fbfc0de..463a07311 100644 --- a/src/functions.c +++ b/src/functions.c @@ -604,6 +604,17 @@ void functionKillCommand(client *c) { scriptKill(c, 0); } +/* Try to extract command flags if we can, returns the modified flags. + * Note that it does not guarantee the command arguments are right. */ +uint64_t fcallGetCommandFlags(client *c, uint64_t cmd_flags) { + robj *function_name = c->argv[1]; + functionInfo *fi = dictFetchValue(curr_functions_lib_ctx->functions, function_name->ptr); + if (!fi) + return cmd_flags; + uint64_t script_flags = fi->f_flags; + return scriptFlagsToCmdFlags(cmd_flags, script_flags); +} + static void fcallCommandGeneric(client *c, int ro) { robj *function_name = c->argv[1]; functionInfo *fi = dictFetchValue(curr_functions_lib_ctx->functions, function_name->ptr); diff --git a/src/multi.c b/src/multi.c index 11f33f48f..78f08526a 100644 --- a/src/multi.c +++ b/src/multi.c @@ -56,7 +56,7 @@ void freeClientMultiState(client *c) { } /* Add a new command into the MULTI commands queue */ -void queueMultiCommand(client *c) { +void queueMultiCommand(client *c, uint64_t cmd_flags) { multiCmd *mc; /* No sense to waste memory if the transaction is already aborted. @@ -75,8 +75,8 @@ void queueMultiCommand(client *c) { mc->argv_len = c->argv_len; c->mstate.count++; - c->mstate.cmd_flags |= c->cmd->flags; - c->mstate.cmd_inv_flags |= ~c->cmd->flags; + c->mstate.cmd_flags |= cmd_flags; + c->mstate.cmd_inv_flags |= ~cmd_flags; c->mstate.argv_len_sums += c->argv_len_sum + sizeof(robj*)*c->argc; /* Reset the client's args since we copied them into the mstate and shouldn't diff --git a/src/script.c b/src/script.c index 4d9cb6b1f..f12f3c8c6 100644 --- a/src/script.c +++ b/src/script.c @@ -108,6 +108,25 @@ int scriptInterrupt(scriptRunCtx *run_ctx) { return (run_ctx->flags & SCRIPT_KILLED) ? SCRIPT_KILL : SCRIPT_CONTINUE; } +uint64_t scriptFlagsToCmdFlags(uint64_t cmd_flags, uint64_t script_flags) { + /* If the script declared flags, clear the ones from the command and use the ones it declared.*/ + cmd_flags &= ~(CMD_STALE | CMD_DENYOOM | CMD_WRITE); + + /* NO_WRITES implies ALLOW_OOM */ + if (!(script_flags & (SCRIPT_FLAG_ALLOW_OOM | SCRIPT_FLAG_NO_WRITES))) + cmd_flags |= CMD_DENYOOM; + if (!(script_flags & SCRIPT_FLAG_NO_WRITES)) + cmd_flags |= CMD_WRITE; + if (script_flags & SCRIPT_FLAG_ALLOW_STALE) + cmd_flags |= CMD_STALE; + + /* In addition the MAY_REPLICATE flag is set for these commands, but + * if we have flags we know if it's gonna do any writes or not. */ + cmd_flags &= ~CMD_MAY_REPLICATE; + + return cmd_flags; +} + /* Prepare the given run ctx for execution */ int scriptPrepareForRun(scriptRunCtx *run_ctx, client *engine_client, client *caller, const char *funcname, uint64_t script_flags, int ro) { serverAssert(!curr_run_ctx); @@ -136,7 +155,7 @@ int scriptPrepareForRun(scriptRunCtx *run_ctx, client *engine_client, client *ca * 2. no disk error detected * 3. command is not `fcall_ro`/`eval[sha]_ro` */ if (server.masterhost && server.repl_slave_ro && !obey_client) { - addReplyError(caller, "Can not run script with write flag on readonly replica"); + addReplyError(caller, "-READONLY Can not run script with write flag on readonly replica"); return C_ERR; } @@ -327,16 +346,23 @@ static int scriptVerifyACL(client *c, sds *err) { } static int scriptVerifyWriteCommandAllow(scriptRunCtx *run_ctx, char **err) { - if (!(run_ctx->c->cmd->flags & CMD_WRITE)) { - return C_OK; - } - if (run_ctx->flags & SCRIPT_READ_ONLY) { - /* We know its a write command, on a read only run we do not allow it. */ + /* A write command, on an RO command or an RO script is rejected ASAP. + * Note: For scripts, we consider may-replicate commands as write commands. + * This also makes it possible to allow read-only scripts to be run during + * CLIENT PAUSE WRITE. */ + if (run_ctx->flags & SCRIPT_READ_ONLY && + (run_ctx->c->cmd->flags & (CMD_WRITE|CMD_MAY_REPLICATE))) + { *err = sdsnew("Write commands are not allowed from read-only scripts."); return C_ERR; } + /* The other checks below are on the server state and are only relevant for + * write commands, return if this is not a write command. */ + if (!(run_ctx->c->cmd->flags & CMD_WRITE)) + return C_OK; + /* Write commands are forbidden against read-only slaves, or if a * command marked as non-deterministic was already called in the context * of this script. */ @@ -366,16 +392,6 @@ static int scriptVerifyWriteCommandAllow(scriptRunCtx *run_ctx, char **err) { return C_OK; } -static int scriptVerifyMayReplicate(scriptRunCtx *run_ctx, char **err) { - if (run_ctx->c->cmd->flags & CMD_MAY_REPLICATE && - server.client_pause_type == CLIENT_PAUSE_WRITE) { - *err = sdsnew("May-replicate commands are not allowed when client pause write."); - return C_ERR; - } - - return C_OK; -} - static int scriptVerifyOOM(scriptRunCtx *run_ctx, char **err) { if (run_ctx->flags & SCRIPT_ALLOW_OOM) { /* Allow running any command even if OOM reached */ @@ -530,10 +546,6 @@ void scriptCall(scriptRunCtx *run_ctx, robj* *argv, int argc, sds *err) { goto error; } - if (scriptVerifyMayReplicate(run_ctx, err) != C_OK) { - goto error; - } - if (scriptVerifyOOM(run_ctx, err) != C_OK) { goto error; } diff --git a/src/script.h b/src/script.h index d855c80e2..b601342c5 100644 --- a/src/script.h +++ b/src/script.h @@ -93,6 +93,7 @@ typedef struct scriptFlag { extern scriptFlag scripts_flags_def[]; +uint64_t scriptFlagsToCmdFlags(uint64_t cmd_flags, uint64_t script_flags); int scriptPrepareForRun(scriptRunCtx *r_ctx, client *engine_client, client *caller, const char *funcname, uint64_t script_flags, int ro); void scriptResetRun(scriptRunCtx *r_ctx); int scriptSetResp(scriptRunCtx *r_ctx, int resp); diff --git a/src/server.c b/src/server.c index e87e4ece5..a5454eded 100644 --- a/src/server.c +++ b/src/server.c @@ -3633,19 +3633,33 @@ int processCommand(client *c) { } } - int is_read_command = (c->cmd->flags & CMD_READONLY) || + /* If we're executing a script, try to extract a set of command flags from + * it, in case it declared them. Note this is just an attempt, we don't yet + * know the script command is well formed.*/ + uint64_t cmd_flags = c->cmd->flags; + if (c->cmd->proc == evalCommand || c->cmd->proc == evalShaCommand || + c->cmd->proc == evalRoCommand || c->cmd->proc == evalShaRoCommand || + c->cmd->proc == fcallCommand || c->cmd->proc == fcallroCommand) + { + if (c->cmd->proc == fcallCommand || c->cmd->proc == fcallroCommand) + cmd_flags = fcallGetCommandFlags(c, cmd_flags); + else + cmd_flags = evalGetCommandFlags(c, cmd_flags); + } + + int is_read_command = (cmd_flags & CMD_READONLY) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_READONLY)); - int is_write_command = (c->cmd->flags & CMD_WRITE) || + int is_write_command = (cmd_flags & CMD_WRITE) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_WRITE)); - int is_denyoom_command = (c->cmd->flags & CMD_DENYOOM) || + int is_denyoom_command = (cmd_flags & CMD_DENYOOM) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_DENYOOM)); - int is_denystale_command = !(c->cmd->flags & CMD_STALE) || + int is_denystale_command = !(cmd_flags & CMD_STALE) || (c->cmd->proc == execCommand && (c->mstate.cmd_inv_flags & CMD_STALE)); - int is_denyloading_command = !(c->cmd->flags & CMD_LOADING) || + int is_denyloading_command = !(cmd_flags & CMD_LOADING) || (c->cmd->proc == execCommand && (c->mstate.cmd_inv_flags & CMD_LOADING)); - int is_may_replicate_command = (c->cmd->flags & (CMD_WRITE | CMD_MAY_REPLICATE)) || + int is_may_replicate_command = (cmd_flags & (CMD_WRITE | CMD_MAY_REPLICATE)) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & (CMD_WRITE | CMD_MAY_REPLICATE))); - int is_deny_async_loading_command = (c->cmd->flags & CMD_NO_ASYNC_LOADING) || + int is_deny_async_loading_command = (cmd_flags & CMD_NO_ASYNC_LOADING) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_NO_ASYNC_LOADING)); int obey_client = mustObeyClient(c); @@ -3913,7 +3927,7 @@ int processCommand(client *c) { c->cmd->proc != quitCommand && c->cmd->proc != resetCommand) { - queueMultiCommand(c); + queueMultiCommand(c, cmd_flags); addReply(c,shared.queued); } else { call(c,CMD_CALL_FULL); @@ -4325,7 +4339,7 @@ void addReplyFlagsForCommand(client *c, struct redisCommand *cmd) { {CMD_ASKING, "asking"}, {CMD_FAST, "fast"}, {CMD_NO_AUTH, "no_auth"}, - {CMD_MAY_REPLICATE, "may_replicate"}, + /* {CMD_MAY_REPLICATE, "may_replicate"},, Hidden on purpose */ /* {CMD_SENTINEL, "sentinel"}, Hidden on purpose */ /* {CMD_ONLY_SENTINEL, "only_sentinel"}, Hidden on purpose */ {CMD_NO_MANDATORY_KEYS, "no_mandatory_keys"}, diff --git a/src/server.h b/src/server.h index 81fd9af19..0fd9de028 100644 --- a/src/server.h +++ b/src/server.h @@ -2592,7 +2592,7 @@ void listElementsRemoved(client *c, robj *key, int where, robj *o, long count, i void unwatchAllKeys(client *c); void initClientMultiState(client *c); void freeClientMultiState(client *c); -void queueMultiCommand(client *c); +void queueMultiCommand(client *c, uint64_t cmd_flags); size_t multiStateMemOverhead(client *c); void touchWatchedKey(redisDb *db, robj *key); int isWatchedKeyExpired(client *c); @@ -3201,6 +3201,8 @@ void sha1hex(char *digest, char *script, size_t len); unsigned long evalMemory(); dict* evalScriptsDict(); unsigned long evalScriptsMemory(); +uint64_t evalGetCommandFlags(client *c, uint64_t orig_flags); +uint64_t fcallGetCommandFlags(client *c, uint64_t orig_flags); int isInsideYieldingLongCommand(); typedef struct luaScript { diff --git a/tests/unit/functions.tcl b/tests/unit/functions.tcl index 0accb4958..a1f6acdff 100644 --- a/tests/unit/functions.tcl +++ b/tests/unit/functions.tcl @@ -424,7 +424,7 @@ start_server {tags {"scripting repl external:skip"}} { r -1 fcall test 0 } e set _ $e - } {*Can not run script with write flag on readonly replica*} + } {READONLY You can't write against a read only replica.} } } @@ -1052,7 +1052,7 @@ start_server {tags {"scripting"}} { r config set maxmemory 1 catch {[r fcall f1 1 k]} e - assert_match {*can not run it when used memory > 'maxmemory'*} $e + assert_match {OOM *when used memory > 'maxmemory'*} $e r config set maxmemory 0 } {OK} {needs:config-maxmemory} @@ -1082,12 +1082,12 @@ start_server {tags {"scripting"}} { r replicaof 127.0.0.1 1 catch {[r fcall f1 0]} e - assert_match {*'allow-stale' flag is not set on the script*} $e + assert_match {MASTERDOWN *} $e assert_equal {hello} [r fcall f2 0] catch {[r fcall f3 0]} e - assert_match {*Can not execute the command on a stale replica*} $e + assert_match {ERR *Can not execute the command on a stale replica*} $e assert_match {*redis_version*} [r fcall f4 0] diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index 492974c67..c8a5107f9 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -262,7 +262,7 @@ start_server {tags {"modules"}} { return 2 } 1 x - assert_error {ERR Can not run script with write flag on readonly replica} { + assert_error {READONLY Can not run script with write flag on readonly replica*} { r test.rm_call eval {#!lua redis.call('get','x') return 3 diff --git a/tests/unit/pause.tcl b/tests/unit/pause.tcl index f7ade2a10..3440c5f9b 100644 --- a/tests/unit/pause.tcl +++ b/tests/unit/pause.tcl @@ -47,18 +47,14 @@ start_server {tags {"pause network"}} { test "Test read/admin mutli-execs are not blocked by pause RO" { r SET FOO BAR r client PAUSE 100000 WRITE - set rd [redis_deferring_client] - $rd MULTI - assert_equal [$rd read] "OK" - $rd PING - assert_equal [$rd read] "QUEUED" - $rd GET FOO - assert_equal [$rd read] "QUEUED" - $rd EXEC + set rr [redis_client] + assert_equal [$rr MULTI] "OK" + assert_equal [$rr PING] "QUEUED" + assert_equal [$rr GET FOO] "QUEUED" + assert_match "PONG BAR" [$rr EXEC] assert_equal [s 0 blocked_clients] 0 r client unpause - assert_match "PONG BAR" [$rd read] - $rd close + $rr close } test "Test write mutli-execs are blocked by pause RO" { @@ -78,20 +74,129 @@ start_server {tags {"pause network"}} { test "Test scripts are blocked by pause RO" { r client PAUSE 60000 WRITE set rd [redis_deferring_client] + set rd2 [redis_deferring_client] $rd EVAL "return 1" 0 - wait_for_blocked_clients_count 1 50 100 + # test a script with a shebang and no flags for coverage + $rd2 EVAL {#!lua + return 1 + } 0 + + wait_for_blocked_clients_count 2 50 100 r client unpause assert_match "1" [$rd read] + assert_match "1" [$rd2 read] $rd close + $rd2 close } - test "Test may-replicate commands are rejected in ro script by pause RO" { + test "Test RO scripts are not blocked by pause RO" { + r set x y + # create a function for later + r FUNCTION load replace {#!lua name=f1 + redis.register_function{ + function_name='f1', + callback=function() return "hello" end, + flags={'no-writes'} + } + } + + r client PAUSE 6000000 WRITE + set rr [redis_client] + + # test an eval that's for sure not in the script cache + assert_equal [$rr EVAL {#!lua flags=no-writes + return 'unique script' + } 0 + ] "unique script" + + # for sanity, repeat that EVAL on a script that's already cached + assert_equal [$rr EVAL {#!lua flags=no-writes + return 'unique script' + } 0 + ] "unique script" + + # test EVAL_RO on a unique script that's for sure not in the cache + assert_equal [$rr EVAL_RO { + return redis.call('GeT', 'x')..' unique script' + } 1 x + ] "y unique script" + + # test with evalsha + set sha [$rr script load {#!lua flags=no-writes + return 2 + }] + assert_equal [$rr EVALSHA $sha 0] 2 + + # test with function + assert_equal [$rr fcall f1 0] hello + + r client unpause + $rr close + } + + test "Test read-only scripts in mutli-exec are not blocked by pause RO" { + r SET FOO BAR + r client PAUSE 100000 WRITE + set rr [redis_client] + assert_equal [$rr MULTI] "OK" + assert_equal [$rr EVAL {#!lua flags=no-writes + return 12 + } 0 + ] QUEUED + assert_equal [$rr EVAL {#!lua flags=no-writes + return 13 + } 0 + ] QUEUED + assert_match "12 13" [$rr EXEC] + assert_equal [s 0 blocked_clients] 0 + r client unpause + $rr close + } + + test "Test write scripts in mutli-exec are blocked by pause RO" { + set rd [redis_deferring_client] + set rd2 [redis_deferring_client] + + # one with a shebang + $rd MULTI + assert_equal [$rd read] "OK" + $rd EVAL {#!lua + return 12 + } 0 + assert_equal [$rd read] "QUEUED" + + # one without a shebang + $rd2 MULTI + assert_equal [$rd2 read] "OK" + $rd2 EVAL {#!lua + return 13 + } 0 + assert_equal [$rd2 read] "QUEUED" + r client PAUSE 60000 WRITE - assert_error {ERR May-replicate commands are not allowed when client pause write*} { + $rd EXEC + $rd2 EXEC + wait_for_blocked_clients_count 2 50 100 + r client unpause + assert_match "12" [$rd read] + assert_match "13" [$rd2 read] + $rd close + $rd2 close + } + + test "Test may-replicate commands are rejected in RO scripts" { + # that's specifically important for CLIENT PAUSE WRITE + assert_error {ERR Write commands are not allowed from read-only scripts. script:*} { r EVAL_RO "return redis.call('publish','ch','msg')" 0 } - r client unpause + assert_error {ERR Write commands are not allowed from read-only scripts. script:*} { + r EVAL {#!lua flags=no-writes + return redis.call('publish','ch','msg') + } 0 + } + # make sure that publish isn't blocked from a non-RO script + assert_equal [r EVAL "return redis.call('publish','ch','msg')" 0] 0 } test "Test multiple clients can be queued up and unblocked" { diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index ea7b17054..6e43ca264 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -1416,7 +1416,7 @@ start_server {tags {"scripting"}} { ] {123} # Fail to execute regardless of script content when we use default flags in OOM condition - assert_error {OOM allow-oom flag is not set on the script, can not run it when used memory > 'maxmemory'} { + assert_error {OOM *} { r eval {#!lua flags= return 1 } 0 @@ -1489,14 +1489,53 @@ start_server {tags {"scripting"}} { } 1 x ] "some value" - assert_error {ERR Can not run script with write flag on readonly replica} { + assert_error {READONLY You can't write against a read only replica.} { r eval {#!lua return redis.call('get','x') } 1 x } - # sanity check on protocol after error reply - assert_equal [r ping] PONG + # test no-write inside multi-exec + r multi + r eval {#!lua flags=no-writes + redis.call('get','x') + return 1 + } 1 x + assert_equal [r exec] 1 + + # test no shebang without write inside multi-exec + r multi + r eval { + redis.call('get','x') + return 1 + } 1 x + assert_equal [r exec] 1 + + # temporarily set the server to master, so it doesn't block the queuing + # and we can test the evaluation of the flags on exec + r replicaof no one + set rr [redis_client] + set rr2 [redis_client] + $rr multi + $rr2 multi + + # test write inside multi-exec + # we don't need to do any actual write + $rr eval {#!lua + return 1 + } 0 + + # test no shebang with write inside multi-exec + $rr2 eval { + redis.call('set','x',1) + return 1 + } 1 x + + r replicaof [srv -1 host] [srv -1 port] + assert_error {EXECABORT Transaction discarded because of: READONLY *} {$rr exec} + assert_error {READONLY You can't write against a read only replica. script: *} {$rr2 exec} + $rr close + $rr2 close } } @@ -1541,7 +1580,7 @@ start_server {tags {"scripting"}} { } 1 x } - assert_error {*'allow-stale' flag is not set on the script*} { + assert_error {MASTERDOWN Link with MASTER is down and replica-serve-stale-data is set to 'no'.} { r eval {#!lua flags=no-writes return 1 } 0 @@ -1582,7 +1621,7 @@ start_server {tags {"scripting"}} { test "reject script do not cause a Lua stack leak" { r config set maxmemory 1 for {set i 0} {$i < 50} {incr i} { - assert_error {OOM allow-oom flag is not set on the script, can not run it when used memory > 'maxmemory'} {r eval {#!lua + assert_error {OOM *} {r eval {#!lua return 1 } 0} }