From 8d71f2c7188c8f1e1a02b4a547cef3d53c989980 Mon Sep 17 00:00:00 2001 From: Seungmin Lee Date: Tue, 14 Jan 2025 11:57:28 -0800 Subject: [PATCH] Address comments: renaming Signed-off-by: Seungmin Lee --- src/module.c | 72 +++++++++++++++++++-------------------- src/valkeymodule.h | 9 ++--- tests/modules/propagate.c | 18 +++++----- 3 files changed, 47 insertions(+), 52 deletions(-) diff --git a/src/module.c b/src/module.c index 2f80ed62bd..3fdd40d3f4 100644 --- a/src/module.c +++ b/src/module.c @@ -510,7 +510,6 @@ static void zsetKeyReset(ValkeyModuleKey *key); static void moduleInitKeyTypeSpecific(ValkeyModuleKey *key); void VM_FreeDict(ValkeyModuleCtx *ctx, ValkeyModuleDict *d); void VM_FreeServerInfo(ValkeyModuleCtx *ctx, ValkeyModuleServerInfoData *data); -int moduleReplicate(ValkeyModuleCtx *ctx, ValkeyModuleFlag flag, const char *cmdname, const char *fmt, va_list ap); /* Helpers for VM_SetCommandInfo. */ static int moduleValidateCommandInfo(const ValkeyModuleCommandInfo *info); @@ -3581,6 +3580,34 @@ int VM_ReplyWithLongDouble(ValkeyModuleCtx *ctx, long double ld) { return VALKEYMODULE_OK; } +/* Helper function for VM_Replicate and VM_ReplicateWithoutValidation to replicate the specified command + * and arguments to replicas and AOF, as effect of execution of the calling command implementation. + * Skip command validation if skip is set to true(non-zero). */ +int moduleReplicate(ValkeyModuleCtx *ctx, int skip, const char *cmdname, const char *fmt, va_list ap) { + struct serverCommand *cmd; + robj **argv = NULL; + int argc = 0, flags = 0, j; + if (!skip) { + cmd = lookupCommandByCString((char *)cmdname); + if (!cmd) return VALKEYMODULE_ERR; + } + /* Create the client and dispatch the command. */ + argv = moduleCreateArgvFromUserFormat(cmdname, fmt, &argc, &flags, ap); + if (argv == NULL) return VALKEYMODULE_ERR; + /* Select the propagation target. Usually is AOF + replicas, however + * the caller can exclude one or the other using the "A" or "R" + * modifiers. */ + int target = 0; + if (!(flags & VALKEYMODULE_ARGV_NO_AOF)) target |= PROPAGATE_AOF; + if (!(flags & VALKEYMODULE_ARGV_NO_REPLICAS)) target |= PROPAGATE_REPL; + alsoPropagate(ctx->client->db->id, argv, argc, target); + /* Release the argv. */ + for (j = 0; j < argc; j++) decrRefCount(argv[j]); + zfree(argv); + server.dirty++; + return VALKEYMODULE_OK; +} + /* -------------------------------------------------------------------------- * ## Commands replication API * -------------------------------------------------------------------------- */ @@ -3625,18 +3652,19 @@ int VM_ReplyWithLongDouble(ValkeyModuleCtx *ctx, long double ld) { int VM_Replicate(ValkeyModuleCtx *ctx, const char *cmdname, const char *fmt, ...) { va_list ap; va_start(ap, fmt); - int result = moduleReplicate(ctx, VALKEYMODULE_FLAG_DEFAULT, cmdname, fmt, ap); + int result = moduleReplicate(ctx, 0, cmdname, fmt, ap); va_end(ap); return result; } -/* Same as ValkeyModule_Replicate, but can take ValkeyModuleFlag - * Can be either VALKEYMODULE_FLAG_DEFAULT, which means default behavior - * (same as calling ValkeyModule_Replicate) */ -int VM_ReplicateWithFlag(ValkeyModuleCtx *ctx, ValkeyModuleFlag flag, const char *cmdname, const char *fmt, ...) { +/* Same as ValkeyModule_Replicate, but performing without command validation. + * Skipping validation can improve performance by reducing the overhead associated + * with command checking, especially in high-throughput scenarios where commands + * are already pre-validated or trusted. */ +int VM_ReplicateWithoutValidation(ValkeyModuleCtx *ctx, const char *cmdname, const char *fmt, ...) { va_list ap; va_start(ap, fmt); - int result = moduleReplicate(ctx, flag, cmdname, fmt, ap); + int result = moduleReplicate(ctx, 1, cmdname, fmt, ap); va_end(ap); return result; } @@ -13669,34 +13697,6 @@ void moduleDefragGlobals(void) { dictReleaseIterator(di); } -/* Helper function for VM_Replicate and VM_ReplicateWithFlag to replicate the specified command - * and arguments to replicas and AOF, as effect of execution of the calling command implementation. - * Skip command validation if the ValkeyModuleFlag is set to VALKEYMODULE_FLAG_SKIP_VALIDATION. */ -int moduleReplicate(ValkeyModuleCtx *ctx, ValkeyModuleFlag flag, const char *cmdname, const char *fmt, va_list ap) { - struct serverCommand *cmd; - robj **argv = NULL; - int argc = 0, flags = 0, j; - if (flag != VALKEYMODULE_FLAG_SKIP_VALIDATION) { - cmd = lookupCommandByCString((char *)cmdname); - if (!cmd) return VALKEYMODULE_ERR; - } - /* Create the client and dispatch the command. */ - argv = moduleCreateArgvFromUserFormat(cmdname, fmt, &argc, &flags, ap); - if (argv == NULL) return VALKEYMODULE_ERR; - /* Select the propagation target. Usually is AOF + replicas, however - * the caller can exclude one or the other using the "A" or "R" - * modifiers. */ - int target = 0; - if (!(flags & VALKEYMODULE_ARGV_NO_AOF)) target |= PROPAGATE_AOF; - if (!(flags & VALKEYMODULE_ARGV_NO_REPLICAS)) target |= PROPAGATE_REPL; - alsoPropagate(ctx->client->db->id, argv, argc, target); - /* Release the argv. */ - for (j = 0; j < argc; j++) decrRefCount(argv[j]); - zfree(argv); - server.dirty++; - return VALKEYMODULE_OK; -} - /* Returns the name of the key currently being processed. * There is no guarantee that the key name is always available, so this may return NULL. */ @@ -13810,7 +13810,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(StringPtrLen); REGISTER_API(AutoMemory); REGISTER_API(Replicate); - REGISTER_API(ReplicateWithFlag); + REGISTER_API(ReplicateWithoutValidation); REGISTER_API(ReplicateVerbatim); REGISTER_API(DeleteKey); REGISTER_API(UnlinkKey); diff --git a/src/valkeymodule.h b/src/valkeymodule.h index 30d2b0e9ea..c240f33788 100644 --- a/src/valkeymodule.h +++ b/src/valkeymodule.h @@ -782,11 +782,6 @@ typedef enum { VALKEYMODULE_ACL_LOG_CHANNEL /* Channel authorization failure */ } ValkeyModuleACLLogEntryReason; -typedef enum { - VALKEYMODULE_FLAG_DEFAULT = 0, /* Default behavior */ - VALKEYMODULE_FLAG_SKIP_VALIDATION, /* Skip validation */ -} ValkeyModuleFlag; - /* Incomplete structures needed by both the core and modules. */ typedef struct ValkeyModuleCtx ValkeyModuleCtx; typedef struct ValkeyModuleIO ValkeyModuleIO; @@ -1185,7 +1180,7 @@ VALKEYMODULE_API int (*ValkeyModule_StringToStreamID)(const ValkeyModuleString * VALKEYMODULE_API void (*ValkeyModule_AutoMemory)(ValkeyModuleCtx *ctx) VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_Replicate)(ValkeyModuleCtx *ctx, const char *cmdname, const char *fmt, ...) VALKEYMODULE_ATTR; -VALKEYMODULE_API int (*ValkeyModule_ReplicateWithFlag)(ValkeyModuleCtx *ctx, ValkeyModuleFlag flag, const char *cmdname, const char *fmt, ...) +VALKEYMODULE_API int (*ValkeyModule_ReplicateWithoutValidation)(ValkeyModuleCtx *ctx, const char *cmdname, const char *fmt, ...) VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_ReplicateVerbatim)(ValkeyModuleCtx *ctx) VALKEYMODULE_ATTR; VALKEYMODULE_API const char *(*ValkeyModule_CallReplyStringPtr)(ValkeyModuleCallReply *reply, @@ -1854,7 +1849,7 @@ static int ValkeyModule_Init(ValkeyModuleCtx *ctx, const char *name, int ver, in VALKEYMODULE_GET_API(StringPtrLen); VALKEYMODULE_GET_API(AutoMemory); VALKEYMODULE_GET_API(Replicate); - VALKEYMODULE_GET_API(ReplicateWithFlag); + VALKEYMODULE_GET_API(ReplicateWithoutValidation); VALKEYMODULE_GET_API(ReplicateVerbatim); VALKEYMODULE_GET_API(DeleteKey); VALKEYMODULE_GET_API(UnlinkKey); diff --git a/tests/modules/propagate.c b/tests/modules/propagate.c index 72684317aa..5f801c23fd 100644 --- a/tests/modules/propagate.c +++ b/tests/modules/propagate.c @@ -250,9 +250,9 @@ int propagateTestSimpleCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, /* Replicate two commands to test MULTI/EXEC wrapping. */ ValkeyModule_Replicate(ctx,"INCR","c","counter-1"); - ValkeyModule_ReplicateWithFlag(ctx, VALKEYMODULE_FLAG_SKIP_VALIDATION, "INCR", - "c", "counter-2"); - ValkeyModule_ReplyWithSimpleString(ctx,"OK"); + ValkeyModule_ReplicateWithoutValidation(ctx, "INCR", + "c", "counter-2"); + ValkeyModule_ReplyWithSimpleString(ctx, "OK"); return VALKEYMODULE_OK; } @@ -267,8 +267,8 @@ int propagateTestMixedCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, i ValkeyModule_FreeCallReply(reply); ValkeyModule_Replicate(ctx,"INCR","c","counter-1"); - ValkeyModule_ReplicateWithFlag(ctx, VALKEYMODULE_FLAG_SKIP_VALIDATION, "INCR", - "c", "counter-2"); + ValkeyModule_ReplicateWithoutValidation(ctx, "INCR", + "c", "counter-2"); reply = ValkeyModule_Call(ctx, "INCR", "c!", "after-call"); ValkeyModule_FreeCallReply(reply); @@ -281,10 +281,10 @@ int propagateTestInvalidCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, VALKEYMODULE_NOT_USED(argv); VALKEYMODULE_NOT_USED(argc); /* Replicate two commands to test MULTI/EXEC wrapping. */ - ValkeyModule_ReplicateWithFlag(ctx, VALKEYMODULE_FLAG_SKIP_VALIDATION, "INVALID", - "c", "counter-1"); - ValkeyModule_ReplicateWithFlag(ctx, VALKEYMODULE_FLAG_SKIP_VALIDATION, "INVALID", - "c", "counter-2"); + ValkeyModule_ReplicateWithoutValidation(ctx, "INVALID", + "c", "counter-1"); + ValkeyModule_ReplicateWithoutValidation(ctx, "INVALID", + "c", "counter-2"); ValkeyModule_ReplyWithSimpleString(ctx, "OK"); return VALKEYMODULE_OK; }