diff options
author | Oran Agra <oran@redislabs.com> | 2022-06-01 14:09:40 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-01 14:09:40 +0300 |
commit | df558618389a578b5eff7f60c9526e64952b74fb (patch) | |
tree | 949e7a2bd3a3f05643e68cc6f1ee03912d8a6c22 /src/script.c | |
parent | b2061de2e713d304ab8e845162888cca47ee32d3 (diff) | |
download | redis-df558618389a578b5eff7f60c9526e64952b74fb.tar.gz |
Expose script flags to processCommand for better handling (#10744)
The important part is that read-only scripts (not just EVAL_RO
and FCALL_RO, but also ones with `no-writes` executed by normal EVAL or
FCALL), will now be permitted to run during CLIENT PAUSE WRITE (unlike
before where only the _RO commands would be processed).
Other than that, some errors like OOM, READONLY, MASTERDOWN are now
handled by processCommand, rather than the command itself affects the
error string (and even error code in some cases), and command stats.
Besides that, now the `may-replicate` commands, PFCOUNT and PUBLISH, will
be considered `write` commands in scripts and will be blocked in all
read-only scripts just like other write commands.
They'll also be blocked in EVAL_RO (i.e. even for scripts without the
`no-writes` shebang flag.
This commit also hides the `may_replicate` flag from the COMMAND command
output. this is a **breaking change**.
background about may_replicate:
We don't want to expose a no-may-replicate flag or alike to scripts, since we
consider the may-replicate thing an internal concern of redis, that we may
some day get rid of.
In fact, the may-replicate flag was initially introduced to flag EVAL: since
we didn't know what it's gonna do ahead of execution, before function-flags
existed). PUBLISH and PFCOUNT, both of which because they have side effects
which may some day be fixed differently.
code changes:
The changes in eval.c are mostly code re-ordering:
- evalCalcFunctionName is extracted out of evalGenericCommand
- evalExtractShebangFlags is extracted luaCreateFunction
- evalGetCommandFlags is new code
Diffstat (limited to 'src/script.c')
-rw-r--r-- | src/script.c | 52 |
1 files changed, 32 insertions, 20 deletions
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; } |