summaryrefslogtreecommitdiff
path: root/src/script.c
diff options
context:
space:
mode:
authorOran Agra <oran@redislabs.com>2022-06-01 14:09:40 +0300
committerGitHub <noreply@github.com>2022-06-01 14:09:40 +0300
commitdf558618389a578b5eff7f60c9526e64952b74fb (patch)
tree949e7a2bd3a3f05643e68cc6f1ee03912d8a6c22 /src/script.c
parentb2061de2e713d304ab8e845162888cca47ee32d3 (diff)
downloadredis-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.c52
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;
}