From e91d9a6fffcac20adfb4fdf2d8fb365ec0816261 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Wed, 24 Jul 2019 12:58:15 +0300 Subject: [PATCH 1/4] Extend modules API to allow modules report to redis INFO this implements #6012 --- runtest-moduleapi | 2 +- src/debug.c | 6 ++ src/module.c | 130 ++++++++++++++++++++++++++++++ src/redismodule.h | 16 ++++ src/server.c | 16 +++- src/server.h | 1 + tests/modules/Makefile | 7 +- tests/modules/infotest.c | 32 ++++++++ tests/unit/moduleapi/infotest.tcl | 53 ++++++++++++ 9 files changed, 260 insertions(+), 3 deletions(-) create mode 100644 tests/modules/infotest.c create mode 100644 tests/unit/moduleapi/infotest.tcl diff --git a/runtest-moduleapi b/runtest-moduleapi index 84cdb9bb8..9bf12677d 100755 --- a/runtest-moduleapi +++ b/runtest-moduleapi @@ -13,4 +13,4 @@ then fi make -C tests/modules && \ -$TCLSH tests/test_helper.tcl --single unit/moduleapi/commandfilter "${@}" +$TCLSH tests/test_helper.tcl --single unit/moduleapi/commandfilter --single unit/moduleapi/infotest "${@}" diff --git a/src/debug.c b/src/debug.c index 1f1157d4a..daf00b14c 100644 --- a/src/debug.c +++ b/src/debug.c @@ -1337,6 +1337,12 @@ void sigsegvHandler(int sig, siginfo_t *info, void *secret) { /* Log dump of processor registers */ logRegisters(uc); + /* Log Modules INFO */ + serverLogRaw(LL_WARNING|LL_RAW, "\n------ MODULES INFO OUTPUT ------\n"); + infostring = modulesCollectInfo(sdsempty(), NULL, 1, 0); + serverLogRaw(LL_WARNING|LL_RAW, infostring); + sdsfree(infostring); + #if defined(HAVE_PROC_MAPS) /* Test memory */ serverLogRaw(LL_WARNING|LL_RAW, "\n------ FAST MEMORY TEST ------\n"); diff --git a/src/module.c b/src/module.c index f4f753c00..0e19c11e2 100644 --- a/src/module.c +++ b/src/module.c @@ -40,6 +40,16 @@ * pointers that have an API the module can call with them) * -------------------------------------------------------------------------- */ +typedef struct RedisModuleInfoCtx { + struct RedisModule *module; + sds requested_section; + sds info; /* info string we collected so far */ + int sections; /* number of sections we collected so far */ + int in_section; /* indication if we're in an active section or not */ +} RedisModuleInfoCtx; + +typedef void (*RedisModuleInfoFunc)(RedisModuleInfoCtx *ctx, int for_crash_report); + /* This structure represents a module inside the system. */ struct RedisModule { void *handle; /* Module dlopen() handle. */ @@ -51,6 +61,7 @@ struct RedisModule { list *using; /* List of modules we use some APIs of. */ list *filters; /* List of filters the module has registered. */ int in_call; /* RM_Call() nesting level */ + RedisModuleInfoFunc info_cb; /* callback for module to add INFO fields. */ }; typedef struct RedisModule RedisModule; @@ -4686,6 +4697,118 @@ int RM_DictCompare(RedisModuleDictIter *di, const char *op, RedisModuleString *k return res ? REDISMODULE_OK : REDISMODULE_ERR; } + + + +/* -------------------------------------------------------------------------- + * Modules Info fields + * -------------------------------------------------------------------------- */ + +/* Used to start a new section, before adding any fields. the section name will + * be prefixed by "_" and must only include A-Z,a-z,0-9. + * When return value is REDISMODULE_ERR, the section should and will be skipped. */ +int RM_AddInfoSection(RedisModuleInfoCtx *ctx, char *name) { + sds full_name = sdscatprintf(sdsdup(ctx->module->name), "_%s", name); + + /* proceed only if: + * 1) no section was requested (emit all) + * 2) the module name was requested (emit all) + * 3) this specific section was requested. */ + if (ctx->requested_section) { + if (strcasecmp(ctx->requested_section, full_name) && + strcasecmp(ctx->requested_section, ctx->module->name)) { + sdsfree(full_name); + ctx->in_section = 0; + return REDISMODULE_ERR; + } + } + if (ctx->sections++) ctx->info = sdscat(ctx->info,"\r\n"); + ctx->info = sdscatprintf(ctx->info, "# %s\r\n", full_name); + ctx->in_section = 1; + sdsfree(full_name); + return REDISMODULE_OK; +} + +/* Used by RedisModuleInfoFunc to add info fields. + * Each field will be automatically prefixed by "_". + * Field names or values must not include \r\n of ":" */ +int RM_AddInfoFieldString(RedisModuleInfoCtx *ctx, char *field, RedisModuleString *value) { + if (!ctx->in_section) + return REDISMODULE_ERR; + ctx->info = sdscatprintf(ctx->info, + "%s_%s:%s\r\n", + ctx->module->name, + field, + (sds)value->ptr); + return REDISMODULE_OK; +} + +int RM_AddInfoFieldCString(RedisModuleInfoCtx *ctx, char *field, char *value) { + if (!ctx->in_section) + return REDISMODULE_ERR; + ctx->info = sdscatprintf(ctx->info, + "%s_%s:%s\r\n", + ctx->module->name, + field, + value); + return REDISMODULE_OK; +} + +int RM_AddInfoFieldDouble(RedisModuleInfoCtx *ctx, char *field, double value) { + if (!ctx->in_section) + return REDISMODULE_ERR; + ctx->info = sdscatprintf(ctx->info, + "%s_%s:%.17g\r\n", + ctx->module->name, + field, + value); + return REDISMODULE_OK; +} + +int RM_AddInfoFieldLongLong(RedisModuleInfoCtx *ctx, char *field, long long value) { + if (!ctx->in_section) + return REDISMODULE_ERR; + ctx->info = sdscatprintf(ctx->info, + "%s_%s:%lld\r\n", + ctx->module->name, + field, + value); + return REDISMODULE_OK; +} + +int RM_AddInfoFieldULongLong(RedisModuleInfoCtx *ctx, char *field, unsigned long long value) { + if (!ctx->in_section) + return REDISMODULE_ERR; + ctx->info = sdscatprintf(ctx->info, + "%s_%s:%llu\r\n", + ctx->module->name, + field, + value); + return REDISMODULE_OK; +} + +int RM_RegisterInfoFunc(RedisModuleCtx *ctx, RedisModuleInfoFunc cb) { + ctx->module->info_cb = cb; + return REDISMODULE_OK; +} + +sds modulesCollectInfo(sds info, sds section, int for_crash_report, int sections) { + dictIterator *di = dictGetIterator(modules); + dictEntry *de; + + while ((de = dictNext(di)) != NULL) { + struct RedisModule *module = dictGetVal(de); + if (!module->info_cb) + continue; + RedisModuleInfoCtx info_ctx = {module, section, info, sections, 0}; + module->info_cb(&info_ctx, for_crash_report); + info = info_ctx.info; + sections = info_ctx.sections; + } + dictReleaseIterator(di); + return info; +} + /* -------------------------------------------------------------------------- * Modules utility APIs * -------------------------------------------------------------------------- */ @@ -5490,4 +5613,11 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(CommandFilterArgInsert); REGISTER_API(CommandFilterArgReplace); REGISTER_API(CommandFilterArgDelete); + REGISTER_API(RegisterInfoFunc); + REGISTER_API(AddInfoSection); + REGISTER_API(AddInfoFieldString); + REGISTER_API(AddInfoFieldCString); + REGISTER_API(AddInfoFieldDouble); + REGISTER_API(AddInfoFieldLongLong); + REGISTER_API(AddInfoFieldULongLong); } diff --git a/src/redismodule.h b/src/redismodule.h index b9c73957b..1feb14a68 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -160,6 +160,7 @@ typedef struct RedisModuleDict RedisModuleDict; typedef struct RedisModuleDictIter RedisModuleDictIter; typedef struct RedisModuleCommandFilterCtx RedisModuleCommandFilterCtx; typedef struct RedisModuleCommandFilter RedisModuleCommandFilter; +typedef struct RedisModuleInfoCtx RedisModuleInfoCtx; typedef int (*RedisModuleCmdFunc)(RedisModuleCtx *ctx, RedisModuleString **argv, int argc); typedef void (*RedisModuleDisconnectFunc)(RedisModuleCtx *ctx, RedisModuleBlockedClient *bc); @@ -173,6 +174,7 @@ typedef void (*RedisModuleTypeFreeFunc)(void *value); typedef void (*RedisModuleClusterMessageReceiver)(RedisModuleCtx *ctx, const char *sender_id, uint8_t type, const unsigned char *payload, uint32_t len); typedef void (*RedisModuleTimerProc)(RedisModuleCtx *ctx, void *data); typedef void (*RedisModuleCommandFilterFunc) (RedisModuleCommandFilterCtx *filter); +typedef void (*RedisModuleInfoFunc)(RedisModuleInfoCtx *ctx, int for_crash_report); #define REDISMODULE_TYPE_METHOD_VERSION 1 typedef struct RedisModuleTypeMethods { @@ -317,6 +319,13 @@ RedisModuleString *REDISMODULE_API_FUNC(RedisModule_DictNext)(RedisModuleCtx *ct RedisModuleString *REDISMODULE_API_FUNC(RedisModule_DictPrev)(RedisModuleCtx *ctx, RedisModuleDictIter *di, void **dataptr); int REDISMODULE_API_FUNC(RedisModule_DictCompareC)(RedisModuleDictIter *di, const char *op, void *key, size_t keylen); int REDISMODULE_API_FUNC(RedisModule_DictCompare)(RedisModuleDictIter *di, const char *op, RedisModuleString *key); +int REDISMODULE_API_FUNC(RedisModule_RegisterInfoFunc)(RedisModuleCtx *ctx, RedisModuleInfoFunc cb); +int REDISMODULE_API_FUNC(RedisModule_AddInfoSection)(RedisModuleInfoCtx *ctx, char *name); +int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldString)(RedisModuleInfoCtx *ctx, char *field, RedisModuleString *value); +int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldCString)(RedisModuleInfoCtx *ctx, char *field, char *value); +int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldDouble)(RedisModuleInfoCtx *ctx, char *field, double value); +int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldLongLong)(RedisModuleInfoCtx *ctx, char *field, long long value); +int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldULongLong)(RedisModuleInfoCtx *ctx, char *field, unsigned long long value); /* Experimental APIs */ #ifdef REDISMODULE_EXPERIMENTAL_API @@ -490,6 +499,13 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(DictPrev); REDISMODULE_GET_API(DictCompare); REDISMODULE_GET_API(DictCompareC); + REDISMODULE_GET_API(RegisterInfoFunc); + REDISMODULE_GET_API(AddInfoSection); + REDISMODULE_GET_API(AddInfoFieldString); + REDISMODULE_GET_API(AddInfoFieldCString); + REDISMODULE_GET_API(AddInfoFieldDouble); + REDISMODULE_GET_API(AddInfoFieldLongLong); + REDISMODULE_GET_API(AddInfoFieldULongLong); #ifdef REDISMODULE_EXPERIMENTAL_API REDISMODULE_GET_API(GetThreadSafeContext); diff --git a/src/server.c b/src/server.c index 4337b8f01..c671e375d 100644 --- a/src/server.c +++ b/src/server.c @@ -3809,12 +3809,15 @@ sds genRedisInfoString(char *section) { time_t uptime = server.unixtime-server.stat_starttime; int j; struct rusage self_ru, c_ru; - int allsections = 0, defsections = 0; + int allsections = 0, defsections = 0, everything = 0, modules = 0; int sections = 0; if (section == NULL) section = "default"; allsections = strcasecmp(section,"all") == 0; defsections = strcasecmp(section,"default") == 0; + everything = strcasecmp(section,"everything") == 0; + modules = strcasecmp(section,"modules") == 0; + if (everything) allsections = 1; getrusage(RUSAGE_SELF, &self_ru); getrusage(RUSAGE_CHILDREN, &c_ru); @@ -4357,6 +4360,17 @@ sds genRedisInfoString(char *section) { } } } + + /* Get info from modules. + * if user asked for "everything" or "modules", or a specific section + * that's not found yet. */ + if (everything || modules || + (!allsections && !defsections && sections==0)) { + info = modulesCollectInfo(info, + everything || modules ? NULL: section, + 0, /* not a crash report */ + sections); + } return info; } diff --git a/src/server.h b/src/server.h index b200a6696..ddddd84f0 100644 --- a/src/server.h +++ b/src/server.h @@ -1528,6 +1528,7 @@ void moduleAcquireGIL(void); void moduleReleaseGIL(void); void moduleNotifyKeyspaceEvent(int type, const char *event, robj *key, int dbid); void moduleCallCommandFilters(client *c); +sds modulesCollectInfo(sds info, sds section, int for_crash_report, int sections); /* Utils */ long long ustime(void); diff --git a/tests/modules/Makefile b/tests/modules/Makefile index 014d20afa..66bf6de35 100644 --- a/tests/modules/Makefile +++ b/tests/modules/Makefile @@ -13,12 +13,17 @@ endif .SUFFIXES: .c .so .xo .o -all: commandfilter.so +all: commandfilter.so infotest.so .c.xo: $(CC) -I../../src $(CFLAGS) $(SHOBJ_CFLAGS) -fPIC -c $< -o $@ commandfilter.xo: ../../src/redismodule.h +infotest.xo: ../../src/redismodule.h commandfilter.so: commandfilter.xo $(LD) -o $@ $< $(SHOBJ_LDFLAGS) $(LIBS) -lc + +infotest.so: infotest.xo + $(LD) -o $@ $< $(SHOBJ_LDFLAGS) $(LIBS) -lc + diff --git a/tests/modules/infotest.c b/tests/modules/infotest.c new file mode 100644 index 000000000..d53ea2126 --- /dev/null +++ b/tests/modules/infotest.c @@ -0,0 +1,32 @@ +#include "redismodule.h" + +#include + +void InfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) { + RedisModule_AddInfoSection(ctx, "Spanish"); + RedisModule_AddInfoFieldCString(ctx, "uno", "one"); + RedisModule_AddInfoFieldLongLong(ctx, "dos", 2); + + RedisModule_AddInfoSection(ctx, "Italian"); + RedisModule_AddInfoFieldLongLong(ctx, "due", 2); + RedisModule_AddInfoFieldDouble(ctx, "tre", 3.3); + + if (for_crash_report) { + RedisModule_AddInfoSection(ctx, "Klingon"); + RedisModule_AddInfoFieldCString(ctx, "one", "wa’"); + RedisModule_AddInfoFieldCString(ctx, "two", "cha’"); + RedisModule_AddInfoFieldCString(ctx, "three", "wej"); + } + +} + +int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + if (RedisModule_Init(ctx,"infotest",1,REDISMODULE_APIVER_1) + == REDISMODULE_ERR) return REDISMODULE_ERR; + + if (RedisModule_RegisterInfoFunc(ctx, InfoFunc) == REDISMODULE_ERR) return REDISMODULE_ERR; + + return REDISMODULE_OK; +} diff --git a/tests/unit/moduleapi/infotest.tcl b/tests/unit/moduleapi/infotest.tcl new file mode 100644 index 000000000..143f9050a --- /dev/null +++ b/tests/unit/moduleapi/infotest.tcl @@ -0,0 +1,53 @@ +set testmodule [file normalize tests/modules/infotest.so] + +# Return value for INFO property +proc field {info property} { + if {[regexp "\r\n$property:(.*?)\r\n" $info _ value]} { + set _ $value + } +} + +start_server {tags {"modules"}} { + r module load $testmodule log-key 0 + + test {module info all} { + set info [r info all] + # info all does not contain modules + assert { ![string match "*Spanish*" $info] } + assert { [string match "*used_memory*" $info] } + } + + test {module info everything} { + set info [r info everything] + # info everything contains all default sections, but not ones for crash report + assert { [string match "*Spanish*" $info] } + assert { [string match "*Italian*" $info] } + assert { [string match "*used_memory*" $info] } + assert { ![string match "*Klingon*" $info] } + field $info infotest_dos + } {2} + + test {module info modules} { + set info [r info modules] + # info all does not contain modules + assert { [string match "*Spanish*" $info] } + assert { ![string match "*used_memory*" $info] } + } + + test {module info one module} { + set info [r info INFOTEST] + # info all does not contain modules + assert { [string match "*Spanish*" $info] } + assert { ![string match "*used_memory*" $info] } + } + + test {module info one section} { + set info [r info INFOTEST_SPANISH] + assert { ![string match "*used_memory*" $info] } + assert { ![string match "*Italian*" $info] } + field $info infotest_uno + } {one} + + # TODO: test crash report. + +} From 1d6e5dc4dcacb1af4698d16e695494f62129071e Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 18 Aug 2019 09:41:45 +0300 Subject: [PATCH 2/4] Module INFO, add support for dict fields, rename API to have common prefix --- src/module.c | 106 +++++++++++++++++++++++++----- src/redismodule.h | 28 ++++---- tests/modules/infotest.c | 26 +++++--- tests/unit/moduleapi/infotest.tcl | 7 +- 4 files changed, 129 insertions(+), 38 deletions(-) diff --git a/src/module.c b/src/module.c index 0e19c11e2..c48b2f045 100644 --- a/src/module.c +++ b/src/module.c @@ -43,9 +43,10 @@ typedef struct RedisModuleInfoCtx { struct RedisModule *module; sds requested_section; - sds info; /* info string we collected so far */ - int sections; /* number of sections we collected so far */ - int in_section; /* indication if we're in an active section or not */ + sds info; /* info string we collected so far */ + int sections; /* number of sections we collected so far */ + int in_section; /* indication if we're in an active section or not */ + int in_dict_field; /* indication that we're curreintly appending to a dict */ } RedisModuleInfoCtx; typedef void (*RedisModuleInfoFunc)(RedisModuleInfoCtx *ctx, int for_crash_report); @@ -4704,12 +4705,18 @@ int RM_DictCompare(RedisModuleDictIter *di, const char *op, RedisModuleString *k * Modules Info fields * -------------------------------------------------------------------------- */ +int RM_InfoEndDictField(RedisModuleInfoCtx *ctx); + /* Used to start a new section, before adding any fields. the section name will * be prefixed by "_" and must only include A-Z,a-z,0-9. * When return value is REDISMODULE_ERR, the section should and will be skipped. */ -int RM_AddInfoSection(RedisModuleInfoCtx *ctx, char *name) { +int RM_InfoAddSection(RedisModuleInfoCtx *ctx, char *name) { sds full_name = sdscatprintf(sdsdup(ctx->module->name), "_%s", name); + /* Implicitly end dicts, instead of returning an error which is likely un checked. */ + if (ctx->in_dict_field) + RM_InfoEndDictField(ctx); + /* proceed only if: * 1) no section was requested (emit all) * 2) the module name was requested (emit all) @@ -4729,12 +4736,48 @@ int RM_AddInfoSection(RedisModuleInfoCtx *ctx, char *name) { return REDISMODULE_OK; } +/* Starts a dict field, similar to the ones in INFO KEYSPACE. Use normal + * RedisModule_InfoAddField* functions to add the items to this field, and + * terminate with RedisModule_InfoEndDictField. */ +int RM_InfoBeginDictField(RedisModuleInfoCtx *ctx, char *name) { + if (!ctx->in_section) + return REDISMODULE_ERR; + /* Implicitly end dicts, instead of returning an error which is likely un checked. */ + if (ctx->in_dict_field) + RM_InfoEndDictField(ctx); + ctx->info = sdscatprintf(ctx->info, + "%s_%s:", + ctx->module->name, + name); + ctx->in_dict_field = 1; + return REDISMODULE_OK; +} + +/* Ends a dict field, see RedisModule_InfoBeginDictField */ +int RM_InfoEndDictField(RedisModuleInfoCtx *ctx) { + if (!ctx->in_dict_field) + return REDISMODULE_ERR; + /* trim the last ',' if found. */ + if (ctx->info[sdslen(ctx->info)-1]==',') + sdsIncrLen(ctx->info, -1); + ctx->info = sdscatprintf(ctx->info, "\r\n"); + ctx->in_dict_field = 0; + return REDISMODULE_OK; +} + /* Used by RedisModuleInfoFunc to add info fields. * Each field will be automatically prefixed by "_". * Field names or values must not include \r\n of ":" */ -int RM_AddInfoFieldString(RedisModuleInfoCtx *ctx, char *field, RedisModuleString *value) { +int RM_InfoAddFieldString(RedisModuleInfoCtx *ctx, char *field, RedisModuleString *value) { if (!ctx->in_section) return REDISMODULE_ERR; + if (ctx->in_dict_field) { + ctx->info = sdscatprintf(ctx->info, + "%s=%s,", + field, + (sds)value->ptr); + return REDISMODULE_OK; + } ctx->info = sdscatprintf(ctx->info, "%s_%s:%s\r\n", ctx->module->name, @@ -4743,9 +4786,16 @@ int RM_AddInfoFieldString(RedisModuleInfoCtx *ctx, char *field, RedisModuleStrin return REDISMODULE_OK; } -int RM_AddInfoFieldCString(RedisModuleInfoCtx *ctx, char *field, char *value) { +int RM_InfoAddFieldCString(RedisModuleInfoCtx *ctx, char *field, char *value) { if (!ctx->in_section) return REDISMODULE_ERR; + if (ctx->in_dict_field) { + ctx->info = sdscatprintf(ctx->info, + "%s=%s,", + field, + value); + return REDISMODULE_OK; + } ctx->info = sdscatprintf(ctx->info, "%s_%s:%s\r\n", ctx->module->name, @@ -4754,9 +4804,16 @@ int RM_AddInfoFieldCString(RedisModuleInfoCtx *ctx, char *field, char *value) { return REDISMODULE_OK; } -int RM_AddInfoFieldDouble(RedisModuleInfoCtx *ctx, char *field, double value) { +int RM_InfoAddFieldDouble(RedisModuleInfoCtx *ctx, char *field, double value) { if (!ctx->in_section) return REDISMODULE_ERR; + if (ctx->in_dict_field) { + ctx->info = sdscatprintf(ctx->info, + "%s=%.17g,", + field, + value); + return REDISMODULE_OK; + } ctx->info = sdscatprintf(ctx->info, "%s_%s:%.17g\r\n", ctx->module->name, @@ -4765,9 +4822,16 @@ int RM_AddInfoFieldDouble(RedisModuleInfoCtx *ctx, char *field, double value) { return REDISMODULE_OK; } -int RM_AddInfoFieldLongLong(RedisModuleInfoCtx *ctx, char *field, long long value) { +int RM_InfoAddFieldLongLong(RedisModuleInfoCtx *ctx, char *field, long long value) { if (!ctx->in_section) return REDISMODULE_ERR; + if (ctx->in_dict_field) { + ctx->info = sdscatprintf(ctx->info, + "%s=%lld,", + field, + value); + return REDISMODULE_OK; + } ctx->info = sdscatprintf(ctx->info, "%s_%s:%lld\r\n", ctx->module->name, @@ -4776,9 +4840,16 @@ int RM_AddInfoFieldLongLong(RedisModuleInfoCtx *ctx, char *field, long long valu return REDISMODULE_OK; } -int RM_AddInfoFieldULongLong(RedisModuleInfoCtx *ctx, char *field, unsigned long long value) { +int RM_InfoAddFieldULongLong(RedisModuleInfoCtx *ctx, char *field, unsigned long long value) { if (!ctx->in_section) return REDISMODULE_ERR; + if (ctx->in_dict_field) { + ctx->info = sdscatprintf(ctx->info, + "%s=%llu,", + field, + value); + return REDISMODULE_OK; + } ctx->info = sdscatprintf(ctx->info, "%s_%s:%llu\r\n", ctx->module->name, @@ -4802,6 +4873,9 @@ sds modulesCollectInfo(sds info, sds section, int for_crash_report, int sections continue; RedisModuleInfoCtx info_ctx = {module, section, info, sections, 0}; module->info_cb(&info_ctx, for_crash_report); + /* Implicitly end dicts (no way to handle errors, and we must add the newline). */ + if (info_ctx.in_dict_field) + RM_InfoEndDictField(&info_ctx); info = info_ctx.info; sections = info_ctx.sections; } @@ -5614,10 +5688,12 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(CommandFilterArgReplace); REGISTER_API(CommandFilterArgDelete); REGISTER_API(RegisterInfoFunc); - REGISTER_API(AddInfoSection); - REGISTER_API(AddInfoFieldString); - REGISTER_API(AddInfoFieldCString); - REGISTER_API(AddInfoFieldDouble); - REGISTER_API(AddInfoFieldLongLong); - REGISTER_API(AddInfoFieldULongLong); + REGISTER_API(InfoAddSection); + REGISTER_API(InfoBeginDictField); + REGISTER_API(InfoEndDictField); + REGISTER_API(InfoAddFieldString); + REGISTER_API(InfoAddFieldCString); + REGISTER_API(InfoAddFieldDouble); + REGISTER_API(InfoAddFieldLongLong); + REGISTER_API(InfoAddFieldULongLong); } diff --git a/src/redismodule.h b/src/redismodule.h index 1feb14a68..26690c17c 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -320,12 +320,14 @@ RedisModuleString *REDISMODULE_API_FUNC(RedisModule_DictPrev)(RedisModuleCtx *ct int REDISMODULE_API_FUNC(RedisModule_DictCompareC)(RedisModuleDictIter *di, const char *op, void *key, size_t keylen); int REDISMODULE_API_FUNC(RedisModule_DictCompare)(RedisModuleDictIter *di, const char *op, RedisModuleString *key); int REDISMODULE_API_FUNC(RedisModule_RegisterInfoFunc)(RedisModuleCtx *ctx, RedisModuleInfoFunc cb); -int REDISMODULE_API_FUNC(RedisModule_AddInfoSection)(RedisModuleInfoCtx *ctx, char *name); -int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldString)(RedisModuleInfoCtx *ctx, char *field, RedisModuleString *value); -int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldCString)(RedisModuleInfoCtx *ctx, char *field, char *value); -int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldDouble)(RedisModuleInfoCtx *ctx, char *field, double value); -int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldLongLong)(RedisModuleInfoCtx *ctx, char *field, long long value); -int REDISMODULE_API_FUNC(RedisModule_AddInfoFieldULongLong)(RedisModuleInfoCtx *ctx, char *field, unsigned long long value); +int REDISMODULE_API_FUNC(RedisModule_InfoAddSection)(RedisModuleInfoCtx *ctx, char *name); +int REDISMODULE_API_FUNC(RedisModule_InfoBeginDictField)(RedisModuleInfoCtx *ctx, char *name); +int REDISMODULE_API_FUNC(RedisModule_InfoEndDictField)(RedisModuleInfoCtx *ctx); +int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldString)(RedisModuleInfoCtx *ctx, char *field, RedisModuleString *value); +int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldCString)(RedisModuleInfoCtx *ctx, char *field, char *value); +int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldDouble)(RedisModuleInfoCtx *ctx, char *field, double value); +int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldLongLong)(RedisModuleInfoCtx *ctx, char *field, long long value); +int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldULongLong)(RedisModuleInfoCtx *ctx, char *field, unsigned long long value); /* Experimental APIs */ #ifdef REDISMODULE_EXPERIMENTAL_API @@ -500,12 +502,14 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(DictCompare); REDISMODULE_GET_API(DictCompareC); REDISMODULE_GET_API(RegisterInfoFunc); - REDISMODULE_GET_API(AddInfoSection); - REDISMODULE_GET_API(AddInfoFieldString); - REDISMODULE_GET_API(AddInfoFieldCString); - REDISMODULE_GET_API(AddInfoFieldDouble); - REDISMODULE_GET_API(AddInfoFieldLongLong); - REDISMODULE_GET_API(AddInfoFieldULongLong); + REDISMODULE_GET_API(InfoAddSection); + REDISMODULE_GET_API(InfoBeginDictField); + REDISMODULE_GET_API(InfoEndDictField); + REDISMODULE_GET_API(InfoAddFieldString); + REDISMODULE_GET_API(InfoAddFieldCString); + REDISMODULE_GET_API(InfoAddFieldDouble); + REDISMODULE_GET_API(InfoAddFieldLongLong); + REDISMODULE_GET_API(InfoAddFieldULongLong); #ifdef REDISMODULE_EXPERIMENTAL_API REDISMODULE_GET_API(GetThreadSafeContext); diff --git a/tests/modules/infotest.c b/tests/modules/infotest.c index d53ea2126..1a75d1606 100644 --- a/tests/modules/infotest.c +++ b/tests/modules/infotest.c @@ -3,19 +3,25 @@ #include void InfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) { - RedisModule_AddInfoSection(ctx, "Spanish"); - RedisModule_AddInfoFieldCString(ctx, "uno", "one"); - RedisModule_AddInfoFieldLongLong(ctx, "dos", 2); + RedisModule_InfoAddSection(ctx, "Spanish"); + RedisModule_InfoAddFieldCString(ctx, "uno", "one"); + RedisModule_InfoAddFieldLongLong(ctx, "dos", 2); - RedisModule_AddInfoSection(ctx, "Italian"); - RedisModule_AddInfoFieldLongLong(ctx, "due", 2); - RedisModule_AddInfoFieldDouble(ctx, "tre", 3.3); + RedisModule_InfoAddSection(ctx, "Italian"); + RedisModule_InfoAddFieldLongLong(ctx, "due", 2); + RedisModule_InfoAddFieldDouble(ctx, "tre", 3.3); + + RedisModule_InfoAddSection(ctx, "keyspace"); + RedisModule_InfoBeginDictField(ctx, "db0"); + RedisModule_InfoAddFieldLongLong(ctx, "keys", 3); + RedisModule_InfoAddFieldLongLong(ctx, "expires", 1); + RedisModule_InfoEndDictField(ctx); if (for_crash_report) { - RedisModule_AddInfoSection(ctx, "Klingon"); - RedisModule_AddInfoFieldCString(ctx, "one", "wa’"); - RedisModule_AddInfoFieldCString(ctx, "two", "cha’"); - RedisModule_AddInfoFieldCString(ctx, "three", "wej"); + RedisModule_InfoAddSection(ctx, "Klingon"); + RedisModule_InfoAddFieldCString(ctx, "one", "wa’"); + RedisModule_InfoAddFieldCString(ctx, "two", "cha’"); + RedisModule_InfoAddFieldCString(ctx, "three", "wej"); } } diff --git a/tests/unit/moduleapi/infotest.tcl b/tests/unit/moduleapi/infotest.tcl index 143f9050a..7f756f0e6 100644 --- a/tests/unit/moduleapi/infotest.tcl +++ b/tests/unit/moduleapi/infotest.tcl @@ -48,6 +48,11 @@ start_server {tags {"modules"}} { field $info infotest_uno } {one} - # TODO: test crash report. + test {module info dict} { + set info [r info infotest_keyspace] + set keyspace [field $info infotest_db0] + set keys [scan [regexp -inline {keys\=([\d]*)} $keyspace] keys=%d] + } {3} + # TODO: test crash report. } From 61853ad8dea0a3ddd8da77a257405452a114bd65 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 18 Aug 2019 10:01:57 +0300 Subject: [PATCH 3/4] Module INFO, support default section for simple modules --- src/module.c | 5 ++++- tests/modules/infotest.c | 3 +++ tests/unit/moduleapi/infotest.tcl | 7 ++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/module.c b/src/module.c index c48b2f045..046b9bc86 100644 --- a/src/module.c +++ b/src/module.c @@ -4709,9 +4709,12 @@ int RM_InfoEndDictField(RedisModuleInfoCtx *ctx); /* Used to start a new section, before adding any fields. the section name will * be prefixed by "_" and must only include A-Z,a-z,0-9. + * NULL or empty string indicates the default section (only ) is used. * When return value is REDISMODULE_ERR, the section should and will be skipped. */ int RM_InfoAddSection(RedisModuleInfoCtx *ctx, char *name) { - sds full_name = sdscatprintf(sdsdup(ctx->module->name), "_%s", name); + sds full_name = sdsdup(ctx->module->name); + if (name != NULL && strlen(name) > 0) + full_name = sdscatprintf(full_name, "_%s", name); /* Implicitly end dicts, instead of returning an error which is likely un checked. */ if (ctx->in_dict_field) diff --git a/tests/modules/infotest.c b/tests/modules/infotest.c index 1a75d1606..d28410932 100644 --- a/tests/modules/infotest.c +++ b/tests/modules/infotest.c @@ -3,6 +3,9 @@ #include void InfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) { + RedisModule_InfoAddSection(ctx, ""); + RedisModule_InfoAddFieldLongLong(ctx, "global", -2); + RedisModule_InfoAddSection(ctx, "Spanish"); RedisModule_InfoAddFieldCString(ctx, "uno", "one"); RedisModule_InfoAddFieldLongLong(ctx, "dos", 2); diff --git a/tests/unit/moduleapi/infotest.tcl b/tests/unit/moduleapi/infotest.tcl index 7f756f0e6..a64d729f3 100644 --- a/tests/unit/moduleapi/infotest.tcl +++ b/tests/unit/moduleapi/infotest.tcl @@ -14,12 +14,14 @@ start_server {tags {"modules"}} { set info [r info all] # info all does not contain modules assert { ![string match "*Spanish*" $info] } + assert { ![string match "*infotest*" $info] } assert { [string match "*used_memory*" $info] } } test {module info everything} { set info [r info everything] # info everything contains all default sections, but not ones for crash report + assert { [string match "*infotest_global*" $info] } assert { [string match "*Spanish*" $info] } assert { [string match "*Italian*" $info] } assert { [string match "*used_memory*" $info] } @@ -31,6 +33,7 @@ start_server {tags {"modules"}} { set info [r info modules] # info all does not contain modules assert { [string match "*Spanish*" $info] } + assert { [string match "*infotest_global*" $info] } assert { ![string match "*used_memory*" $info] } } @@ -39,12 +42,14 @@ start_server {tags {"modules"}} { # info all does not contain modules assert { [string match "*Spanish*" $info] } assert { ![string match "*used_memory*" $info] } - } + field $info infotest_global + } {-2} test {module info one section} { set info [r info INFOTEST_SPANISH] assert { ![string match "*used_memory*" $info] } assert { ![string match "*Italian*" $info] } + assert { ![string match "*infotest_global*" $info] } field $info infotest_uno } {one} From 1b4f888109b90b8982d16402268c6dbb90225430 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Mon, 30 Sep 2019 21:13:13 +0300 Subject: [PATCH 4/4] Use sdscatfmt instead of sdscatprintf in module info sdscatfmt is faster --- src/module.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/module.c b/src/module.c index efbf8eb83..18622bd6b 100644 --- a/src/module.c +++ b/src/module.c @@ -4824,7 +4824,7 @@ int RM_InfoEndDictField(RedisModuleInfoCtx *ctx); int RM_InfoAddSection(RedisModuleInfoCtx *ctx, char *name) { sds full_name = sdsdup(ctx->module->name); if (name != NULL && strlen(name) > 0) - full_name = sdscatprintf(full_name, "_%s", name); + full_name = sdscatfmt(full_name, "_%s", name); /* Implicitly end dicts, instead of returning an error which is likely un checked. */ if (ctx->in_dict_field) @@ -4843,7 +4843,7 @@ int RM_InfoAddSection(RedisModuleInfoCtx *ctx, char *name) { } } if (ctx->sections++) ctx->info = sdscat(ctx->info,"\r\n"); - ctx->info = sdscatprintf(ctx->info, "# %s\r\n", full_name); + ctx->info = sdscatfmt(ctx->info, "# %S\r\n", full_name); ctx->in_section = 1; sdsfree(full_name); return REDISMODULE_OK; @@ -4858,7 +4858,7 @@ int RM_InfoBeginDictField(RedisModuleInfoCtx *ctx, char *name) { /* Implicitly end dicts, instead of returning an error which is likely un checked. */ if (ctx->in_dict_field) RM_InfoEndDictField(ctx); - ctx->info = sdscatprintf(ctx->info, + ctx->info = sdscatfmt(ctx->info, "%s_%s:", ctx->module->name, name); @@ -4873,7 +4873,7 @@ int RM_InfoEndDictField(RedisModuleInfoCtx *ctx) { /* trim the last ',' if found. */ if (ctx->info[sdslen(ctx->info)-1]==',') sdsIncrLen(ctx->info, -1); - ctx->info = sdscatprintf(ctx->info, "\r\n"); + ctx->info = sdscat(ctx->info, "\r\n"); ctx->in_dict_field = 0; return REDISMODULE_OK; } @@ -4885,14 +4885,14 @@ int RM_InfoAddFieldString(RedisModuleInfoCtx *ctx, char *field, RedisModuleStrin if (!ctx->in_section) return REDISMODULE_ERR; if (ctx->in_dict_field) { - ctx->info = sdscatprintf(ctx->info, - "%s=%s,", + ctx->info = sdscatfmt(ctx->info, + "%s=%S,", field, (sds)value->ptr); return REDISMODULE_OK; } - ctx->info = sdscatprintf(ctx->info, - "%s_%s:%s\r\n", + ctx->info = sdscatfmt(ctx->info, + "%s_%s:%S\r\n", ctx->module->name, field, (sds)value->ptr); @@ -4903,13 +4903,13 @@ int RM_InfoAddFieldCString(RedisModuleInfoCtx *ctx, char *field, char *value) { if (!ctx->in_section) return REDISMODULE_ERR; if (ctx->in_dict_field) { - ctx->info = sdscatprintf(ctx->info, + ctx->info = sdscatfmt(ctx->info, "%s=%s,", field, value); return REDISMODULE_OK; } - ctx->info = sdscatprintf(ctx->info, + ctx->info = sdscatfmt(ctx->info, "%s_%s:%s\r\n", ctx->module->name, field, @@ -4939,14 +4939,14 @@ int RM_InfoAddFieldLongLong(RedisModuleInfoCtx *ctx, char *field, long long valu if (!ctx->in_section) return REDISMODULE_ERR; if (ctx->in_dict_field) { - ctx->info = sdscatprintf(ctx->info, - "%s=%lld,", + ctx->info = sdscatfmt(ctx->info, + "%s=%I,", field, value); return REDISMODULE_OK; } - ctx->info = sdscatprintf(ctx->info, - "%s_%s:%lld\r\n", + ctx->info = sdscatfmt(ctx->info, + "%s_%s:%I\r\n", ctx->module->name, field, value); @@ -4957,14 +4957,14 @@ int RM_InfoAddFieldULongLong(RedisModuleInfoCtx *ctx, char *field, unsigned long if (!ctx->in_section) return REDISMODULE_ERR; if (ctx->in_dict_field) { - ctx->info = sdscatprintf(ctx->info, - "%s=%llu,", + ctx->info = sdscatfmt(ctx->info, + "%s=%U,", field, value); return REDISMODULE_OK; } - ctx->info = sdscatprintf(ctx->info, - "%s_%s:%llu\r\n", + ctx->info = sdscatfmt(ctx->info, + "%s_%s:%U\r\n", ctx->module->name, field, value); @@ -5712,9 +5712,9 @@ sds genModulesInfoString(sds info) { sds usedby = genModulesInfoStringRenderModulesList(module->usedby); sds using = genModulesInfoStringRenderModulesList(module->using); sds options = genModulesInfoStringRenderModuleOptions(module); - info = sdscatprintf(info, - "module:name=%s,ver=%d,api=%d,filters=%d," - "usedby=%s,using=%s,options=%s\r\n", + info = sdscatfmt(info, + "module:name=%S,ver=%i,api=%i,filters=%i," + "usedby=%S,using=%S,options=%S\r\n", name, module->ver, module->apiver, (int)listLength(module->filters), usedby, using, options); sdsfree(usedby);