From adc3183cd2b54238424895e4298548df4526f8c3 Mon Sep 17 00:00:00 2001 From: "Meir Shpilraien (Spielrein)" Date: Sun, 11 Oct 2020 17:21:58 +0300 Subject: [PATCH] Add Module API for version and compatibility checks (#7865) * Introduce a new API's: RM_GetContextFlagsAll, and RM_GetKeyspaceNotificationFlagsAll that will return the full flags mask of each feature. The module writer can check base on this value if the Flags he needs are supported or not. * For each flag, introduce a new value on redismodule.h, this value represents the LAST value and should be there as a reminder to update it when a new value is added, also it will be used in the code to calculate the full flags mask (assuming flags are incrementally increasing). In addition, stated that the module writer should not use the LAST flag directly and he should use the GetFlagAll API's. * Introduce a new API: RM_IsSubEventSupported, that returns for a given event and subevent, whether or not the subevent supported. * Introduce a new macro RMAPI_FUNC_SUPPORTED(func) that returns whether or not a function API is supported by comparing it to NULL. * Introduce a new API: int RM_GetServerVersion();, that will return the current Redis version in the format 0x00MMmmpp; e.g. 0x00060008; * Changed unstable version from 999.999.999 to 255.255.255 Co-authored-by: Oran Agra Co-authored-by: Yossi Gottlieb --- src/module.c | 85 +++++++++++++++++++++++++- src/redismodule.h | 42 ++++++++++++- src/version.h | 3 +- tests/modules/blockedclient.c | 8 +++ tests/modules/hooks.c | 11 ++++ tests/modules/keyspace_events.c | 7 +++ tests/modules/misc.c | 18 ++++++ tests/unit/moduleapi/blockedclient.tcl | 6 ++ tests/unit/moduleapi/misc.tcl | 5 ++ 9 files changed, 181 insertions(+), 4 deletions(-) diff --git a/src/module.c b/src/module.c index 7b4b761fe..ca99d1951 100644 --- a/src/module.c +++ b/src/module.c @@ -7267,13 +7267,14 @@ void ModuleForkDoneHandler(int exitcode, int bysignal) { * * * The function returns REDISMODULE_OK if the module was successfully subscribed - * for the specified event. If the API is called from a wrong context then - * REDISMODULE_ERR is returned. */ + * for the specified event. If the API is called from a wrong context or unsupported event + * is given then REDISMODULE_ERR is returned. */ int RM_SubscribeToServerEvent(RedisModuleCtx *ctx, RedisModuleEvent event, RedisModuleEventCallback callback) { RedisModuleEventListener *el; /* Protect in case of calls from contexts without a module reference. */ if (ctx->module == NULL) return REDISMODULE_ERR; + if (event.id >= _REDISMODULE_EVENT_NEXT) return REDISMODULE_ERR; /* Search an event matching this module and event ID. */ listIter li; @@ -7305,6 +7306,42 @@ int RM_SubscribeToServerEvent(RedisModuleCtx *ctx, RedisModuleEvent event, Redis return REDISMODULE_OK; } +/** + * For a given server event and subevent, return zero if the + * subevent is not supported and non-zero otherwise. + */ +int RM_IsSubEventSupported(RedisModuleEvent event, int64_t subevent) { + switch (event.id) { + case REDISMODULE_EVENT_REPLICATION_ROLE_CHANGED: + return subevent < _REDISMODULE_EVENT_REPLROLECHANGED_NEXT; + case REDISMODULE_EVENT_PERSISTENCE: + return subevent < _REDISMODULE_SUBEVENT_PERSISTENCE_NEXT; + case REDISMODULE_EVENT_FLUSHDB: + return subevent < _REDISMODULE_SUBEVENT_FLUSHDB_NEXT; + case REDISMODULE_EVENT_LOADING: + return subevent < _REDISMODULE_SUBEVENT_LOADING_NEXT; + case REDISMODULE_EVENT_CLIENT_CHANGE: + return subevent < _REDISMODULE_SUBEVENT_CLIENT_CHANGE_NEXT; + case REDISMODULE_EVENT_SHUTDOWN: + return subevent < _REDISMODULE_SUBEVENT_SHUTDOWN_NEXT; + case REDISMODULE_EVENT_REPLICA_CHANGE: + return subevent < _REDISMODULE_EVENT_REPLROLECHANGED_NEXT; + case REDISMODULE_EVENT_MASTER_LINK_CHANGE: + return subevent < _REDISMODULE_SUBEVENT_MASTER_NEXT; + case REDISMODULE_EVENT_CRON_LOOP: + return subevent < _REDISMODULE_SUBEVENT_CRON_LOOP_NEXT; + case REDISMODULE_EVENT_MODULE_CHANGE: + return subevent < _REDISMODULE_SUBEVENT_MODULE_NEXT; + case REDISMODULE_EVENT_LOADING_PROGRESS: + return subevent < _REDISMODULE_SUBEVENT_LOADING_PROGRESS_NEXT; + case REDISMODULE_EVENT_SWAPDB: + return subevent < _REDISMODULE_SUBEVENT_SWAPDB_NEXT; + default: + break; + } + return 0; +} + /* This is called by the Redis internals every time we want to fire an * event that can be interceppted by some module. The pointer 'data' is useful * in order to populate the event-specific structure when needed, in order @@ -7894,6 +7931,46 @@ int RM_GetLFU(RedisModuleKey *key, long long *lfu_freq) { return REDISMODULE_OK; } +/** + * Returns the full ContextFlags mask, using the return value + * the module can check if a certain set of flags are supported + * by the redis server version in use. + * Example: + * int supportedFlags = RM_GetContextFlagsAll() + * if (supportedFlags & REDISMODULE_CTX_FLAGS_MULTI) { + * // REDISMODULE_CTX_FLAGS_MULTI is supported + * } else{ + * // REDISMODULE_CTX_FLAGS_MULTI is not supported + * } + */ +int RM_GetContextFlagsAll() { + return _REDISMODULE_CTX_FLAGS_NEXT - 1; +} + +/** + * Returns the full KeyspaceNotification mask, using the return value + * the module can check if a certain set of flags are supported + * by the redis server version in use. + * Example: + * int supportedFlags = RM_GetKeyspaceNotificationFlagsAll() + * if (supportedFlags & REDISMODULE_NOTIFY_LOADED) { + * // REDISMODULE_NOTIFY_LOADED is supported + * } else{ + * // REDISMODULE_NOTIFY_LOADED is not supported + * } + */ +int RM_GetKeyspaceNotificationFlagsAll() { + return _REDISMODULE_NOTIFY_NEXT - 1; +} + +/** + * Return the redis version in format of 0x00MMmmpp. + * Example for 6.0.7 the return value will be 0x00060007. + */ +int RM_GetServerVersion() { + return REDIS_VERSION_NUM; +} + /* Replace the value assigned to a module type. * * The key must be open for writing, have an existing value, and have a moduleType @@ -8227,6 +8304,10 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(DeauthenticateAndCloseClient); REGISTER_API(AuthenticateClientWithACLUser); REGISTER_API(AuthenticateClientWithUser); + REGISTER_API(GetContextFlagsAll); + REGISTER_API(GetKeyspaceNotificationFlagsAll); + REGISTER_API(IsSubEventSupported); + REGISTER_API(GetServerVersion); REGISTER_API(GetClientCertificate); REGISTER_API(GetCommandKeys); } diff --git a/src/redismodule.h b/src/redismodule.h index 7b3545585..ed9a73172 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -117,6 +117,11 @@ /* Redis is currently running inside background child process. */ #define REDISMODULE_CTX_FLAGS_IS_CHILD (1<<20) +/* Next context flag, must be updated when adding new flags above! +This flag should not be used directly by the module. + * Use RedisModule_GetContextFlagsAll instead. */ +#define _REDISMODULE_CTX_FLAGS_NEXT (1<<21) + /* Keyspace changes notification classes. Every class is associated with a * character for configuration purposes. * NOTE: These have to be in sync with NOTIFY_* in server.h */ @@ -133,6 +138,12 @@ #define REDISMODULE_NOTIFY_STREAM (1<<10) /* t */ #define REDISMODULE_NOTIFY_KEY_MISS (1<<11) /* m (Note: This one is excluded from REDISMODULE_NOTIFY_ALL on purpose) */ #define REDISMODULE_NOTIFY_LOADED (1<<12) /* module only key space notification, indicate a key loaded from rdb */ + +/* Next notification flag, must be updated when adding new flags above! +This flag should not be used directly by the module. + * Use RedisModule_GetKeyspaceNotificationFlagsAll instead. */ +#define _REDISMODULE_NOTIFY_NEXT (1<<13) + #define REDISMODULE_NOTIFY_ALL (REDISMODULE_NOTIFY_GENERIC | REDISMODULE_NOTIFY_STRING | REDISMODULE_NOTIFY_LIST | REDISMODULE_NOTIFY_SET | REDISMODULE_NOTIFY_HASH | REDISMODULE_NOTIFY_ZSET | REDISMODULE_NOTIFY_EXPIRED | REDISMODULE_NOTIFY_EVICTED | REDISMODULE_NOTIFY_STREAM) /* A */ /* A special pointer that we can use between the core and the module to signal @@ -182,7 +193,9 @@ typedef uint64_t RedisModuleTimerID; * are modified from the user's sperspective, to invalidate WATCH. */ #define REDISMODULE_OPTION_NO_IMPLICIT_SIGNAL_MODIFIED (1<<1) -/* Server events definitions. */ +/* Server events definitions. + * Those flags should not be used directly by the module, instead + * the module should use RedisModuleEvent_* variables */ #define REDISMODULE_EVENT_REPLICATION_ROLE_CHANGED 0 #define REDISMODULE_EVENT_PERSISTENCE 1 #define REDISMODULE_EVENT_FLUSHDB 2 @@ -196,6 +209,9 @@ typedef uint64_t RedisModuleTimerID; #define REDISMODULE_EVENT_LOADING_PROGRESS 10 #define REDISMODULE_EVENT_SWAPDB 11 +/* Next event flag, should be updated if a new event added. */ +#define _REDISMODULE_EVENT_NEXT 12 + typedef struct RedisModuleEvent { uint64_t id; /* REDISMODULE_EVENT_... defines. */ uint64_t dataver; /* Version of the structure we pass as 'data'. */ @@ -260,33 +276,47 @@ static const RedisModuleEvent #define REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_RDB_START 2 #define REDISMODULE_SUBEVENT_PERSISTENCE_ENDED 3 #define REDISMODULE_SUBEVENT_PERSISTENCE_FAILED 4 +#define _REDISMODULE_SUBEVENT_PERSISTENCE_NEXT 5 #define REDISMODULE_SUBEVENT_LOADING_RDB_START 0 #define REDISMODULE_SUBEVENT_LOADING_AOF_START 1 #define REDISMODULE_SUBEVENT_LOADING_REPL_START 2 #define REDISMODULE_SUBEVENT_LOADING_ENDED 3 #define REDISMODULE_SUBEVENT_LOADING_FAILED 4 +#define _REDISMODULE_SUBEVENT_LOADING_NEXT 5 #define REDISMODULE_SUBEVENT_CLIENT_CHANGE_CONNECTED 0 #define REDISMODULE_SUBEVENT_CLIENT_CHANGE_DISCONNECTED 1 +#define _REDISMODULE_SUBEVENT_CLIENT_CHANGE_NEXT 2 #define REDISMODULE_SUBEVENT_MASTER_LINK_UP 0 #define REDISMODULE_SUBEVENT_MASTER_LINK_DOWN 1 +#define _REDISMODULE_SUBEVENT_MASTER_NEXT 2 #define REDISMODULE_SUBEVENT_REPLICA_CHANGE_ONLINE 0 #define REDISMODULE_SUBEVENT_REPLICA_CHANGE_OFFLINE 1 +#define _REDISMODULE_SUBEVENT_REPLICA_CHANGE_NEXT 2 #define REDISMODULE_EVENT_REPLROLECHANGED_NOW_MASTER 0 #define REDISMODULE_EVENT_REPLROLECHANGED_NOW_REPLICA 1 +#define _REDISMODULE_EVENT_REPLROLECHANGED_NEXT 2 #define REDISMODULE_SUBEVENT_FLUSHDB_START 0 #define REDISMODULE_SUBEVENT_FLUSHDB_END 1 +#define _REDISMODULE_SUBEVENT_FLUSHDB_NEXT 2 #define REDISMODULE_SUBEVENT_MODULE_LOADED 0 #define REDISMODULE_SUBEVENT_MODULE_UNLOADED 1 +#define _REDISMODULE_SUBEVENT_MODULE_NEXT 2 + #define REDISMODULE_SUBEVENT_LOADING_PROGRESS_RDB 0 #define REDISMODULE_SUBEVENT_LOADING_PROGRESS_AOF 1 +#define _REDISMODULE_SUBEVENT_LOADING_PROGRESS_NEXT 2 + +#define _REDISMODULE_SUBEVENT_SHUTDOWN_NEXT 0 +#define _REDISMODULE_SUBEVENT_CRON_LOOP_NEXT 0 +#define _REDISMODULE_SUBEVENT_SWAPDB_NEXT 0 /* RedisModuleClientInfo flags. */ #define REDISMODULE_CLIENTINFO_FLAG_SSL (1<<0) @@ -672,6 +702,10 @@ REDISMODULE_API void (*RedisModule_ScanCursorRestart)(RedisModuleScanCursor *cur REDISMODULE_API void (*RedisModule_ScanCursorDestroy)(RedisModuleScanCursor *cursor) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_Scan)(RedisModuleCtx *ctx, RedisModuleScanCursor *cursor, RedisModuleScanCB fn, void *privdata) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_ScanKey)(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleScanKeyCB fn, void *privdata) REDISMODULE_ATTR; +REDISMODULE_API int (*RedisModule_GetContextFlagsAll)() REDISMODULE_ATTR; +REDISMODULE_API int (*RedisModule_GetKeyspaceNotificationFlagsAll)() REDISMODULE_ATTR; +REDISMODULE_API int (*RedisModule_IsSubEventSupported)(RedisModuleEvent event, uint64_t subevent) REDISMODULE_ATTR; +REDISMODULE_API int (*RedisModule_GetServerVersion)() REDISMODULE_ATTR; /* Experimental APIs */ #ifdef REDISMODULE_EXPERIMENTAL_API @@ -918,6 +952,10 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(ScanCursorDestroy); REDISMODULE_GET_API(Scan); REDISMODULE_GET_API(ScanKey); + REDISMODULE_GET_API(GetContextFlagsAll); + REDISMODULE_GET_API(GetKeyspaceNotificationFlagsAll); + REDISMODULE_GET_API(IsSubEventSupported); + REDISMODULE_GET_API(GetServerVersion); #ifdef REDISMODULE_EXPERIMENTAL_API REDISMODULE_GET_API(GetThreadSafeContext); @@ -988,5 +1026,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int * including this file. */ #define RedisModuleString robj +#define RMAPI_FUNC_SUPPORTED(func) (func != NULL) + #endif /* REDISMODULE_CORE */ #endif /* REDISMODULE_H */ diff --git a/src/version.h b/src/version.h index eb65e9bbd..89aef53fc 100644 --- a/src/version.h +++ b/src/version.h @@ -1 +1,2 @@ -#define REDIS_VERSION "999.999.999" +#define REDIS_VERSION "255.255.255" +#define REDIS_VERSION_NUM 0x00ffffff diff --git a/tests/modules/blockedclient.c b/tests/modules/blockedclient.c index ca98281a4..558e06502 100644 --- a/tests/modules/blockedclient.c +++ b/tests/modules/blockedclient.c @@ -57,6 +57,14 @@ int acquire_gil(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) UNUSED(argv); UNUSED(argc); + int flags = RedisModule_GetContextFlags(ctx); + int allFlags = RedisModule_GetContextFlagsAll(); + if ((allFlags & REDISMODULE_CTX_FLAGS_MULTI) && + (flags & REDISMODULE_CTX_FLAGS_MULTI)) { + RedisModule_ReplyWithSimpleString(ctx, "Blocked client is not supported inside multi"); + return REDISMODULE_OK; + } + /* This command handler tries to acquire the GIL twice * once in the worker thread using "RedisModule_ThreadSafeContextLock" * second in the sub-worker thread diff --git a/tests/modules/hooks.c b/tests/modules/hooks.c index 54f84aa23..6fb3d2325 100644 --- a/tests/modules/hooks.c +++ b/tests/modules/hooks.c @@ -266,12 +266,22 @@ void swapDbCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void /* This function must be present on each Redis module. It is used in order to * register the commands into the Redis server. */ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { +#define VerifySubEventSupported(e, s) \ + if (!RedisModule_IsSubEventSupported(e, s)) { \ + return REDISMODULE_ERR; \ + } + REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); if (RedisModule_Init(ctx,"testhook",1,REDISMODULE_APIVER_1) == REDISMODULE_ERR) return REDISMODULE_ERR; + /* Example on how to check if a server sub event is supported */ + if (!RedisModule_IsSubEventSupported(RedisModuleEvent_ReplicationRoleChanged, REDISMODULE_EVENT_REPLROLECHANGED_NOW_MASTER)) { + return REDISMODULE_ERR; + } + /* replication related hooks */ RedisModule_SubscribeToServerEvent(ctx, RedisModuleEvent_ReplicationRoleChanged, roleChangeCallback); @@ -297,6 +307,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) RedisModuleEvent_Shutdown, shutdownCallback); RedisModule_SubscribeToServerEvent(ctx, RedisModuleEvent_CronLoop, cronLoopCallback); + RedisModule_SubscribeToServerEvent(ctx, RedisModuleEvent_ModuleChange, moduleChangeCallback); RedisModule_SubscribeToServerEvent(ctx, diff --git a/tests/modules/keyspace_events.c b/tests/modules/keyspace_events.c index db3977be1..fac7edd1f 100644 --- a/tests/modules/keyspace_events.c +++ b/tests/modules/keyspace_events.c @@ -87,6 +87,13 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) loaded_event_log = RedisModule_CreateDict(ctx); + int keySpaceAll = RedisModule_GetKeyspaceNotificationFlagsAll(); + + if (!(keySpaceAll & REDISMODULE_NOTIFY_LOADED)) { + // REDISMODULE_NOTIFY_LOADED event are not supported we can not start + return REDISMODULE_ERR; + } + if(RedisModule_SubscribeToKeyspaceEvents(ctx, REDISMODULE_NOTIFY_LOADED, KeySpace_Notification) != REDISMODULE_OK){ return REDISMODULE_ERR; } diff --git a/tests/modules/misc.c b/tests/modules/misc.c index 621b92b2d..1c7c47a90 100644 --- a/tests/modules/misc.c +++ b/tests/modules/misc.c @@ -195,6 +195,22 @@ int test_setlfu(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_OK; } +int test_redisversion(RedisModuleCtx *ctx, RedisModuleString **argv, int argc){ + (void) argv; + (void) argc; + + int version = RedisModule_GetServerVersion(); + int patch = version & 0x000000ff; + int minor = (version & 0x0000ff00) >> 8; + int major = (version & 0x00ff0000) >> 16; + + RedisModuleString* vStr = RedisModule_CreateStringPrintf(ctx, "%d.%d.%d", major, minor, patch); + RedisModule_ReplyWithString(ctx, vStr); + RedisModule_FreeString(ctx, vStr); + + return REDISMODULE_OK; +} + int test_getclientcert(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { (void) argv; @@ -300,6 +316,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.redisversion", test_redisversion,"",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"test.getclientcert", test_getclientcert,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"test.log_tsctx", test_log_tsctx,"",0,0,0) == REDISMODULE_ERR) diff --git a/tests/unit/moduleapi/blockedclient.tcl b/tests/unit/moduleapi/blockedclient.tcl index d093a0297..5a541d138 100644 --- a/tests/unit/moduleapi/blockedclient.tcl +++ b/tests/unit/moduleapi/blockedclient.tcl @@ -8,4 +8,10 @@ start_server {tags {"modules"}} { test {Locked GIL acquisition} { assert_match "OK" [r acquire_gil] } + + test {Locked GIL acquisition during multi} { + r multi + r acquire_gil + assert_equal {{Blocked client is not supported inside multi}} [r exec] + } } diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index e825f33ff..d60ccca8f 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -16,6 +16,11 @@ start_server {tags {"modules"}} { assert { [string match "*cmdstat_module*" $info] } } + test {test redis version} { + set version [s redis_version] + assert_equal $version [r test.redisversion] + } + test {test long double conversions} { set ld [r test.ld_conversion] assert {[string match $ld "0.00000000000000001"]}