From 71560a2a4a1a73085dba9a8ea8f835c371358cfa Mon Sep 17 00:00:00 2001 From: Wen Hui Date: Thu, 5 Dec 2024 11:58:24 -0500 Subject: [PATCH] Add API UpdateRuntimeArgs for updating the module arguments during runtime (#1041) Before Redis OSS 7, if we load a module with some arguments during runtime, and run the command "config rewrite", the module information will not be saved into the config file. Since Redis OSS 7 and Valkey 7.2, if we load a module with some arguments during runtime, the module information (path, arguments number, and arguments value) can be saved into the config file after config rewrite command is called. Thus, the module will be loaded automatically when the server startup next time. Following is one example: bind 172.25.0.58 port 7000 protected-mode no enable-module-command yes Generated by CONFIG REWRITE latency-tracking-info-percentiles 50 99 99.9 dir "/home/ubuntu/valkey" save 3600 1 300 100 60 10000 user default on nopass sanitize-payload ~* &* +https://github.com/ALL loadmodule tests/modules/datatype.so 10 20 However, there is one problem. If developers write a module, and update the running arguments by someway, the updated arguments can not be saved into the config file even "config rewrite" is called. The reason comes from the following function rewriteConfigLoadmoduleOption (src/config.c) void rewriteConfigLoadmoduleOption(struct rewriteConfigState *state) { .......... struct ValkeyModule *module = dictGetVal(de); line = sdsnew("loadmodule "); line = sdscatsds(line, module->loadmod->path); for (int i = 0; i < module->loadmod->argc; i++) { line = sdscatlen(line, " ", 1); line = sdscatsds(line, module->loadmod->argv[i]->ptr); } rewriteConfigRewriteLine(state, "loadmodule", line, 1); ....... } The function only save the initial arguments information (module->loadmod) into the configfile. After core members discuss, ref https://github.com/valkey-io/valkey/issues/1177 We decide add the following API to implement this feature: Original proposal: int VM_UpdateRunTimeArgs(ValkeyModuleCtx *ctx, int index, char *value); Updated proposal: ValkeyModuleString **values VM_GetRuntimeArgs(ValkeyModuleCtx *ctx); **int VM_UpdateRuntimeArgs(ValkeyModuleCtx *ctx, int argc, ValkeyModuleString **values); Why we do not recommend the following way: MODULE UNLOAD Update module args in the conf file MODULE LOAD I think there are the following disadvantages: 1. Some modules can not be unloaded. Such as the example module datatype.so, which is tests/modules/datatype.so 2. it is not atomic operation for MODULE UNLOAD + MODULE LOAD 3. sometimes, if we just run the module unload, the client business could be interrupted --------- Signed-off-by: hwware --- src/module.c | 22 ++++++++++++++++++++ src/valkeymodule.h | 2 ++ tests/modules/Makefile | 1 + tests/modules/moduleparameter.c | 28 ++++++++++++++++++++++++++ tests/unit/moduleapi/moduleconfigs.tcl | 13 +++++++++++- 5 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/modules/moduleparameter.c diff --git a/src/module.c b/src/module.c index 4092ae6b06..5f9dff0402 100644 --- a/src/module.c +++ b/src/module.c @@ -2255,6 +2255,27 @@ int moduleIsModuleCommand(void *module_handle, struct serverCommand *cmd) { return (cp->module == module_handle); } +/* ValkeyModule_UpdateRuntimeArgs can be used to update the module argument values. + * The function parameter 'argc' indicates the number of updated arguments, and 'argv' + * represents the values of the updated arguments. + * Once 'CONFIG REWRITE' command is called, the updated argument values can be saved into conf file. + * + * The function always returns VALKEYMODULE_OK. */ +int VM_UpdateRuntimeArgs(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { + struct moduleLoadQueueEntry *loadmod = ctx->module->loadmod; + for (int i = 0; i < loadmod->argc; i++) { + decrRefCount(loadmod->argv[i]); + } + zfree(loadmod->argv); + loadmod->argv = argc - 1 ? zmalloc(sizeof(robj *) * (argc - 1)) : NULL; + loadmod->argc = argc - 1; + for (int i = 1; i < argc; i++) { + loadmod->argv[i - 1] = argv[i]; + incrRefCount(loadmod->argv[i - 1]); + } + return VALKEYMODULE_OK; +} + /* -------------------------------------------------------------------------- * ## Module information and time measurement * -------------------------------------------------------------------------- */ @@ -13560,6 +13581,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(SetModuleAttribs); REGISTER_API(IsModuleNameBusy); REGISTER_API(WrongArity); + REGISTER_API(UpdateRuntimeArgs); REGISTER_API(ReplyWithLongLong); REGISTER_API(ReplyWithError); REGISTER_API(ReplyWithErrorFormat); diff --git a/src/valkeymodule.h b/src/valkeymodule.h index c2cdb2f0e7..7c3adfd477 100644 --- a/src/valkeymodule.h +++ b/src/valkeymodule.h @@ -967,6 +967,7 @@ VALKEYMODULE_API void (*ValkeyModule_SetModuleAttribs)(ValkeyModuleCtx *ctx, con VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_IsModuleNameBusy)(const char *name) VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_WrongArity)(ValkeyModuleCtx *ctx) VALKEYMODULE_ATTR; +VALKEYMODULE_API int (*ValkeyModule_UpdateRuntimeArgs)(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_ReplyWithLongLong)(ValkeyModuleCtx *ctx, long long ll) VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_GetSelectedDb)(ValkeyModuleCtx *ctx) VALKEYMODULE_ATTR; VALKEYMODULE_API int (*ValkeyModule_SelectDb)(ValkeyModuleCtx *ctx, int newid) VALKEYMODULE_ATTR; @@ -1673,6 +1674,7 @@ static int ValkeyModule_Init(ValkeyModuleCtx *ctx, const char *name, int ver, in VALKEYMODULE_GET_API(SetModuleAttribs); VALKEYMODULE_GET_API(IsModuleNameBusy); VALKEYMODULE_GET_API(WrongArity); + VALKEYMODULE_GET_API(UpdateRuntimeArgs); VALKEYMODULE_GET_API(ReplyWithLongLong); VALKEYMODULE_GET_API(ReplyWithError); VALKEYMODULE_GET_API(ReplyWithErrorFormat); diff --git a/tests/modules/Makefile b/tests/modules/Makefile index 1690b9b627..82813bb6f7 100644 --- a/tests/modules/Makefile +++ b/tests/modules/Makefile @@ -58,6 +58,7 @@ TEST_MODULES = \ eventloop.so \ moduleconfigs.so \ moduleconfigstwo.so \ + moduleparameter.so \ publish.so \ usercall.so \ postnotifications.so \ diff --git a/tests/modules/moduleparameter.c b/tests/modules/moduleparameter.c new file mode 100644 index 0000000000..6c110f2cfb --- /dev/null +++ b/tests/modules/moduleparameter.c @@ -0,0 +1,28 @@ +#include "valkeymodule.h" +#include +#include +#include +#include + +int test_module_update_parameter(ValkeyModuleCtx *ctx, + ValkeyModuleString **argv, int argc) { + + ValkeyModule_UpdateRuntimeArgs(ctx, argv, argc); + return ValkeyModule_ReplyWithSimpleString(ctx, "OK"); +} + +int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { + VALKEYMODULE_NOT_USED(argv); + VALKEYMODULE_NOT_USED(argc); + + if (ValkeyModule_Init(ctx, "moduleparameter", 1, VALKEYMODULE_APIVER_1) == + VALKEYMODULE_ERR) + return VALKEYMODULE_ERR; + + if (ValkeyModule_CreateCommand(ctx, "testmoduleparameter.update.parameter", + test_module_update_parameter, "fast", 0, 0, + 0) == VALKEYMODULE_ERR) + return VALKEYMODULE_ERR; + + return VALKEYMODULE_OK; +} diff --git a/tests/unit/moduleapi/moduleconfigs.tcl b/tests/unit/moduleapi/moduleconfigs.tcl index 44f994d2d0..54de5f2611 100644 --- a/tests/unit/moduleapi/moduleconfigs.tcl +++ b/tests/unit/moduleapi/moduleconfigs.tcl @@ -1,5 +1,7 @@ set testmodule [file normalize tests/modules/moduleconfigs.so] set testmoduletwo [file normalize tests/modules/moduleconfigstwo.so] +set testmoduleparameter [file normalize tests/modules/moduleparameter.so] + start_server {tags {"modules"}} { r module load $testmodule @@ -243,5 +245,14 @@ start_server {tags {"modules"}} { assert_equal [r config get moduleconfigs.memory_numeric] "moduleconfigs.memory_numeric 1024" } } -} + test {Module Update Args} { + r module load $testmoduleparameter 10 20 30 + set t [r module list] + set modulename [lmap x [r module list] {dict get $x name}] + assert_not_equal [lsearch $modulename moduleparameter] -1 + assert_equal "{10 20 30}" [lmap x [r module list] {dict get $x args}] + assert_equal OK [r testmoduleparameter.update.parameter 40 50 60 70] + assert_equal "{40 50 60 70}" [lmap x [r module list] {dict get $x args}] + } +}