From d5ea320d0bb6fb986781fbbeb286fa1152000a9e Mon Sep 17 00:00:00 2001 From: Meir Shpilraien Date: Tue, 7 Nov 2023 11:00:05 +0200 Subject: [PATCH] Review fixes --- src/expire.c | 2 +- tests/modules/postnotifications.c | 12 ++++---- tests/unit/moduleapi/async_rm_call.tcl | 32 ++++++++++++++++++++++ tests/unit/moduleapi/postnotifications.tcl | 6 ++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/expire.c b/src/expire.c index d787d769cf1e..c9a4ad44d40d 100644 --- a/src/expire.c +++ b/src/expire.c @@ -54,7 +54,7 @@ int activeExpireCycleTryExpire(redisDb *db, dictEntry *de, long long now) { long long t = dictGetSignedIntegerVal(de); if (now > t) { - enterExecutionUnit(1, 0); + enterExecutionUnit(1, 0); sds key = dictGetKey(de); robj *keyobj = createStringObject(key,sdslen(key)); deleteExpiredKeyAndPropagate(db,keyobj); diff --git a/tests/modules/postnotifications.c b/tests/modules/postnotifications.c index 6c2b1eb096b9..095d5fb57ec5 100644 --- a/tests/modules/postnotifications.c +++ b/tests/modules/postnotifications.c @@ -206,13 +206,15 @@ static void KeySpace_ServerEventCallback(RedisModuleCtx *ctx, RedisModuleEvent e const RedisModuleString *key_name = RedisModule_GetKeyNameFromModuleKey(((RedisModuleKeyInfo*)data)->key); const char *key_str = RedisModule_StringPtrLen(key_name, NULL); - const char *event = events[subevent]; - size_t event_len = strlen(event); - if (strncmp(key_str, event , event_len) == 0) { - return; /* avoid loops */ + + for (int i = 0 ; i < 4 ; ++i) { + const char *event = events[i]; + if (strncmp(key_str, event , strlen(event)) == 0) { + return; /* don't log any event on our tracking keys */ + } } - RedisModuleString *new_key = RedisModule_CreateString(NULL, event, event_len); + RedisModuleString *new_key = RedisModule_CreateString(NULL, events[subevent], strlen(events[subevent])); RedisModule_AddPostNotificationJob(ctx, KeySpace_PostNotificationString, new_key, KeySpace_PostNotificationStringFreePD); } diff --git a/tests/unit/moduleapi/async_rm_call.tcl b/tests/unit/moduleapi/async_rm_call.tcl index e6209761517e..abe09376fb2b 100644 --- a/tests/unit/moduleapi/async_rm_call.tcl +++ b/tests/unit/moduleapi/async_rm_call.tcl @@ -314,6 +314,15 @@ start_server {tags {"modules"}} { r lpush l a assert_equal [$rd read] {OK} + # Explanation of the first multi exec block: + # {lpop l} - pop the value by our blocking command 'blpop_and_set_multiple_keys' + # {set string_foo 1} - the action of our blocking command 'blpop_and_set_multiple_keys' + # {set string_bar 2} - the action of our blocking command 'blpop_and_set_multiple_keys' + # {incr before_deleted} - post notificaiton job registered on the before deleted event of list l that got empty + # {incr string_changed{string_foo}} - post notification job that was registered when 'string_foo' changed + # {incr string_changed{string_bar}} - post notification job that was registered when 'string_bar' changed + # {incr string_total} - post notification job that was registered when string_changed{string_foo} changed + # {incr string_total} - post notification job that was registered when string_changed{string_bar} changed assert_replication_stream $repl { {select *} {lpush l a} @@ -356,6 +365,29 @@ start_server {tags {"modules"}} { r lpush l a assert_equal [$rd read] {OK} + # Explanation of the first multi exec block: + # {lpop l} - pop the value by our blocking command 'blpop_and_set_multiple_keys' + # {set string_foo 1} - the action of our blocking command 'blpop_and_set_multiple_keys' + # {set string_bar 2} - the action of our blocking command 'blpop_and_set_multiple_keys' + # {incr before_deleted} - post notificaiton job registered on the before deleted event of list l that got empty + # {incr string_changed{string_foo}} - post notification job that was registered when 'string_foo' changed + # {incr string_changed{string_bar}} - post notification job that was registered when 'string_bar' changed + # {incr string_total} - post notification job that was registered when string_changed{string_foo} changed + # {incr string_total} - post notification job that was registered when string_changed{string_bar} changed + # + # Explanation of the second multi exec block: + # {lpop l} - pop the value by our blocking command 'blpop_and_set_multiple_keys' + # {del string_foo} - lazy expiration of string_foo when 'blpop_and_set_multiple_keys' tries to write to it. + # {set string_foo 1} - the action of our blocking command 'blpop_and_set_multiple_keys' + # {set string_bar 2} - the action of our blocking command 'blpop_and_set_multiple_keys' + # {incr before_deleted} - the post notification job, registered when the list got empty and deleted + # {incr before_expired} - the post notification job, registered just before string_foo got expired + # {incr expired} - the post notification job, registered after string_foo got expired + # {incr string_changed{string_foo}} - post notification job triggered when we set string_foo + # {incr before_overwritten} - post notification job triggered before we overwrite string_bar + # {incr string_changed{string_bar}} - post notification job triggered when we set string_bar + # {incr string_total} - post notification job triggered when we incr 'string_changed{string_foo}' + # {incr string_total} - post notification job triggered when we incr 'string_changed{string_bar}' assert_replication_stream $repl { {select *} {lpush l a} diff --git a/tests/unit/moduleapi/postnotifications.tcl b/tests/unit/moduleapi/postnotifications.tcl index b281a66c5554..183fbd098643 100644 --- a/tests/unit/moduleapi/postnotifications.tcl +++ b/tests/unit/moduleapi/postnotifications.tcl @@ -14,6 +14,7 @@ tags "modules" { assert_equal {2} [r get string_changed{string_x}] assert_equal {2} [r get string_total] + # the {incr before_overwritten} is a post notification job registered when 'string_x' was overwritten assert_replication_stream $repl { {multi} {select *} @@ -64,6 +65,7 @@ tags "modules" { fail "Failed waiting for x to expired" } + # the {incr before_expired} is a post notification job registered before 'x' got expired assert_replication_stream $repl { {select *} {set x 1} @@ -87,6 +89,7 @@ tags "modules" { after 10 assert_equal {} [r get x] + # the {incr before_expired} is a post notification job registered before 'x' got expired assert_replication_stream $repl { {select *} {set x 1} @@ -111,6 +114,7 @@ tags "modules" { after 10 assert_equal {OK} [r set read_x 1] + # the {incr before_expired} is a post notification job registered before 'x' got expired assert_replication_stream $repl { {select *} {set x 1} @@ -152,6 +156,7 @@ tags "modules" { assert_error {OOM *} {r set y 1} + # the {incr before_evicted} is a post notification job registered before 'x' got evicted assert_replication_stream $repl { {select *} {set x 1} @@ -186,6 +191,7 @@ tags "modules" { assert_equal {1} [r get string_changed{string1_x}] assert_equal {3} [r get string_total] + # the {incr before_overwritten} is a post notification job registered before 'string_x' got overwritten assert_replication_stream $repl { {multi} {select *}