Add server log when module load fails with busy name (#1084)

Currently when module loading fails due to busy name, we
don't have a clean way to assist to troubleshooting.

Case 1: when loading the same module multiple times, we can
not detemine the cause of its failure without referring to
the module list or the earliest module load log. The log
may not exist and sometimes it is difficult for people
to associate module list.

Case 2: when multiple modules use the same module name,
we can not quickly associate the busy name without referring
to the module list and the earliest module load log.
Different people wrote modules with the same module name,
they don't easily associate module name.

So in this PR, when doing module onload, we will try to
print a busy name log if this happen. Currently we check
ctx.module since if it is NULL it means the Init call
failed, and Init currently only fails with busy name.

It's kind of ugly. It would have been nice if we could have had a
better way for onload to signal why the load failed.

Signed-off-by: Binbin <binloveplay1314@qq.com>
This commit is contained in:
Binbin 2024-10-09 16:10:29 +08:00 committed by GitHub
parent cba8eaf4c9
commit 1892f8a731
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 11 additions and 2 deletions

View File

@ -12194,11 +12194,15 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa
ValkeyModuleCtx ctx;
moduleCreateContext(&ctx, NULL, VALKEYMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */
if (onload((void *)&ctx, module_argv, module_argc) == VALKEYMODULE_ERR) {
serverLog(LL_WARNING, "Module %s initialization failed. Module not loaded", path);
if (ctx.module) {
serverLog(LL_WARNING, "Module %s initialization failed. Module not loaded.", path);
moduleUnregisterCleanup(ctx.module);
moduleRemoveCateogires(ctx.module);
moduleFreeModuleStructure(ctx.module);
} else {
/* If there is no ctx.module, this means that our ValkeyModule_Init call failed,
* and currently init will only fail on busy name. */
serverLog(LL_WARNING, "Module %s initialization failed. Module name is busy.", path);
}
moduleFreeContext(&ctx);
dlclose(handle);

View File

@ -34,7 +34,12 @@ start_server {tags {"modules"}} {
}
}
test "Unload the module - test" {
test "Busy module name" {
assert_error {ERR Error loading the extension. Please check the server logs.} {r module load $testmodule}
verify_log_message 0 "*Module name is busy*" 0
}
test "Unload the module - basics" {
assert_equal {OK} [r module unload test]
}
}