From 9dcf878f1b735e4714a2f7bbea7ce567768605bf Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 31 Mar 2020 11:00:45 +0200 Subject: [PATCH] Fix module commands propagation double MULTI bug. 37a10cef introduced automatic wrapping of MULTI/EXEC for the alsoPropagate API. However this collides with the built-in mechanism already present in module.c. To avoid complex changes near Redis 6 GA this commit introduces the ability to exclude call() MUTLI/EXEC wrapping for also propagate in order to continue to use the old code paths in module.c. --- src/module.c | 7 +------ src/server.c | 8 ++++++-- src/server.h | 2 ++ tests/modules/propagate.c | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/module.c b/src/module.c index 7d9eb3719..61dc25169 100644 --- a/src/module.c +++ b/src/module.c @@ -3326,13 +3326,8 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch * a Lua script in the context of AOF and slaves. */ if (replicate) moduleReplicateMultiIfNeeded(ctx); - if (ctx->client->flags & CLIENT_MULTI || - ctx->flags & REDISMODULE_CTX_MULTI_EMITTED) { - c->flags |= CLIENT_MULTI; - } - /* Run the command */ - int call_flags = CMD_CALL_SLOWLOG | CMD_CALL_STATS; + int call_flags = CMD_CALL_SLOWLOG | CMD_CALL_STATS | CMD_CALL_NOWRAP; if (replicate) { if (!(flags & REDISMODULE_ARGV_NO_AOF)) call_flags |= CMD_CALL_PROPAGATE_AOF; diff --git a/src/server.c b/src/server.c index c1f7436a1..93bafbae8 100644 --- a/src/server.c +++ b/src/server.c @@ -3266,11 +3266,15 @@ void call(client *c, int flags) { int multi_emitted = 0; /* Wrap the commands in server.also_propagate array, * but don't wrap it if we are already in MULIT context, - * in case the nested MULIT/EXEC. + * in case the nested MULTI/EXEC. * * And if the array contains only one command, no need to * wrap it, since the single command is atomic. */ - if (server.also_propagate.numops > 1 && !(c->flags & CLIENT_MULTI)) { + if (server.also_propagate.numops > 1 && + !(c->cmd->flags & CMD_MODULE) && + !(c->flags & CLIENT_MULTI) && + !(flags & CMD_CALL_NOWRAP)) + { execCommandPropagateMulti(c); multi_emitted = 1; } diff --git a/src/server.h b/src/server.h index 691f47c48..d5be5ed29 100644 --- a/src/server.h +++ b/src/server.h @@ -395,6 +395,8 @@ typedef long long ustime_t; /* microsecond time type. */ #define CMD_CALL_PROPAGATE_REPL (1<<3) #define CMD_CALL_PROPAGATE (CMD_CALL_PROPAGATE_AOF|CMD_CALL_PROPAGATE_REPL) #define CMD_CALL_FULL (CMD_CALL_SLOWLOG | CMD_CALL_STATS | CMD_CALL_PROPAGATE) +#define CMD_CALL_NOWRAP (1<<4) /* Don't wrap also propagate array into + MULTI/EXEC: the caller will handle it. */ /* Command propagation flags, see propagate() function */ #define PROPAGATE_NONE 0 diff --git a/tests/modules/propagate.c b/tests/modules/propagate.c index f83af1799..2571ee43d 100644 --- a/tests/modules/propagate.c +++ b/tests/modules/propagate.c @@ -89,6 +89,16 @@ int propagateTestCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc return REDISMODULE_OK; } +int propagateTest2Command(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) +{ + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + + RedisModule_Replicate(ctx,"INCR","c","counter"); + RedisModule_ReplyWithSimpleString(ctx,"OK"); + return REDISMODULE_OK; +} + int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); @@ -100,5 +110,11 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) propagateTestCommand, "",1,1,1) == REDISMODULE_ERR) return REDISMODULE_ERR; + + if (RedisModule_CreateCommand(ctx,"propagate-test-2", + propagateTest2Command, + "",1,1,1) == REDISMODULE_ERR) + return REDISMODULE_ERR; + return REDISMODULE_OK; }