From 907da0580b57013c9e5c38b27f351597c72e4e25 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Sun, 11 Oct 2020 16:11:31 +0300 Subject: [PATCH] Modules: Add RM_GetDetachedThreadSafeContext(). (#7886) The main motivation here is to provide a way for modules to create a single, global context that can be used for logging. Currently, it is possible to obtain a thread-safe context that is not attached to any blocked client by using `RM_GetThreadSafeContext`. However, the attached context is not linked to the module identity so log messages produced are not tagged with the module name. Ideally we'd fix this in `RM_GetThreadSafeContext` itself but as it doesn't accept the current context as an argument there's no way to do that in a backwards compatible manner. --- src/module.c | 21 +++++++++++++++++++-- src/redismodule.h | 2 ++ tests/modules/misc.c | 26 ++++++++++++++++++++++++++ tests/unit/moduleapi/misc.tcl | 5 +++++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/module.c b/src/module.c index 1e18f3c46..ef39d5d3a 100644 --- a/src/module.c +++ b/src/module.c @@ -4901,8 +4901,9 @@ int RM_BlockedClientDisconnected(RedisModuleCtx *ctx) { * that a blocked client was used when the context was created, otherwise * no RedisModule_Reply* call should be made at all. * - * TODO: thread safe contexts do not inherit the blocked client - * selected database. */ + * NOTE: If you're creating a detached thread safe context (bc is NULL), + * consider using `RM_GetDetachedThreadSafeContext` which will also retain + * the module ID and thus be more useful for logging. */ RedisModuleCtx *RM_GetThreadSafeContext(RedisModuleBlockedClient *bc) { RedisModuleCtx *ctx = zmalloc(sizeof(*ctx)); RedisModuleCtx empty = REDISMODULE_CTX_INIT; @@ -4924,6 +4925,21 @@ RedisModuleCtx *RM_GetThreadSafeContext(RedisModuleBlockedClient *bc) { return ctx; } +/* Return a detached thread safe context that is not associated with any + * specific blocked client, but is associated with the module's context. + * + * This is useful for modules that wish to hold a global context over + * a long term, for purposes such as logging. */ +RedisModuleCtx *RM_GetDetachedThreadSafeContext(RedisModuleCtx *ctx) { + RedisModuleCtx *new_ctx = zmalloc(sizeof(*new_ctx)); + RedisModuleCtx empty = REDISMODULE_CTX_INIT; + memcpy(new_ctx,&empty,sizeof(empty)); + new_ctx->module = ctx->module; + new_ctx->flags |= REDISMODULE_CTX_THREAD_SAFE; + new_ctx->client = createClient(NULL); + return new_ctx; +} + /* Release a thread safe context. */ void RM_FreeThreadSafeContext(RedisModuleCtx *ctx) { moduleFreeContext(ctx); @@ -8086,6 +8102,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(AbortBlock); REGISTER_API(Milliseconds); REGISTER_API(GetThreadSafeContext); + REGISTER_API(GetDetachedThreadSafeContext); REGISTER_API(FreeThreadSafeContext); REGISTER_API(ThreadSafeContextLock); REGISTER_API(ThreadSafeContextTryLock); diff --git a/src/redismodule.h b/src/redismodule.h index e05ce65ee..092ad3914 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -684,6 +684,7 @@ REDISMODULE_API void * (*RedisModule_GetBlockedClientPrivateData)(RedisModuleCtx REDISMODULE_API RedisModuleBlockedClient * (*RedisModule_GetBlockedClientHandle)(RedisModuleCtx *ctx) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_AbortBlock)(RedisModuleBlockedClient *bc) REDISMODULE_ATTR; REDISMODULE_API RedisModuleCtx * (*RedisModule_GetThreadSafeContext)(RedisModuleBlockedClient *bc) REDISMODULE_ATTR; +REDISMODULE_API RedisModuleCtx * (*RedisModule_GetDetachedThreadSafeContext)(RedisModuleCtx *ctx) REDISMODULE_ATTR; REDISMODULE_API void (*RedisModule_FreeThreadSafeContext)(RedisModuleCtx *ctx) REDISMODULE_ATTR; REDISMODULE_API void (*RedisModule_ThreadSafeContextLock)(RedisModuleCtx *ctx) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_ThreadSafeContextTryLock)(RedisModuleCtx *ctx) REDISMODULE_ATTR; @@ -919,6 +920,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int #ifdef REDISMODULE_EXPERIMENTAL_API REDISMODULE_GET_API(GetThreadSafeContext); + REDISMODULE_GET_API(GetDetachedThreadSafeContext); REDISMODULE_GET_API(FreeThreadSafeContext); REDISMODULE_GET_API(ThreadSafeContextLock); REDISMODULE_GET_API(ThreadSafeContextTryLock); diff --git a/tests/modules/misc.c b/tests/modules/misc.c index 1f9cb1932..a43bb84ac 100644 --- a/tests/modules/misc.c +++ b/tests/modules/misc.c @@ -231,6 +231,30 @@ int test_clientinfo(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_OK; } +int test_log_tsctx(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) +{ + RedisModuleCtx *tsctx = RedisModule_GetDetachedThreadSafeContext(ctx); + + if (argc != 3) { + RedisModule_WrongArity(ctx); + return REDISMODULE_OK; + } + + char level[50]; + size_t level_len; + const char *level_str = RedisModule_StringPtrLen(argv[1], &level_len); + snprintf(level, sizeof(level) - 1, "%.*s", (int) level_len, level_str); + + size_t msg_len; + const char *msg_str = RedisModule_StringPtrLen(argv[2], &msg_len); + + RedisModule_Log(tsctx, level, "%.*s", (int) msg_len, msg_str); + RedisModule_FreeThreadSafeContext(tsctx); + + RedisModule_ReplyWithSimpleString(ctx, "OK"); + return REDISMODULE_OK; +} + int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); @@ -259,6 +283,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"test.clientinfo", test_clientinfo,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx,"test.log_tsctx", test_log_tsctx,"",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; return REDISMODULE_OK; } diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index b57a94f6a..2c9ac4fd0 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -86,4 +86,9 @@ start_server {tags {"modules"}} { set info [r test.clientinfo] assert { [dict get $info flags] == "${ssl_flag}::tracking::" } } + + test {test detached thread safe cnotext} { + r test.log_tsctx "info" "Test message" + verify_log_message 0 "* Test message*" 0 + } }