Skip to content

Commit

Permalink
Add API UpdateRuntimeArgs for updating the module arguments during ru…
Browse files Browse the repository at this point in the history
…ntime (valkey-io#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
valkey-io#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 <[email protected]>
  • Loading branch information
hwware authored Dec 5, 2024
1 parent a401e37 commit 71560a2
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 1 deletion.
22 changes: 22 additions & 0 deletions src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
* -------------------------------------------------------------------------- */
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/valkeymodule.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions tests/modules/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ TEST_MODULES = \
eventloop.so \
moduleconfigs.so \
moduleconfigstwo.so \
moduleparameter.so \
publish.so \
usercall.so \
postnotifications.so \
Expand Down
28 changes: 28 additions & 0 deletions tests/modules/moduleparameter.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#include "valkeymodule.h"
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

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;
}
13 changes: 12 additions & 1 deletion tests/unit/moduleapi/moduleconfigs.tcl
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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}]
}
}

0 comments on commit 71560a2

Please sign in to comment.