From c55e33a99fa2efd5a9583964c45425ef0855182b Mon Sep 17 00:00:00 2001 From: Moti Cohen Date: Thu, 31 Jul 2025 15:44:25 +0300 Subject: [PATCH] KEYSIZES - Fix resolving key slot on modules (#14240) In cluster mode with modules, for a given key, the slot resolution for the KEYSIZES histogram update was incorrect. As a result, the histogram might gracefully ignored those keys instead or update the wrong slot histogram. --- src/module.c | 6 +++--- tests/unit/moduleapi/list.tcl | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/module.c b/src/module.c index 41e6d93d3..216030809 100644 --- a/src/module.c +++ b/src/module.c @@ -4743,7 +4743,7 @@ int RM_ListDelete(RedisModuleKey *key, long index) { if (moduleListIteratorSeek(key, index, REDISMODULE_WRITE)) { listTypeDelete(key->iter, &key->u.list.entry); int64_t l = (int64_t) listTypeLength(key->kv); - updateKeysizesHist(key->db, getKeySlot(key->kv->ptr), OBJ_LIST, l+1, l); + updateKeysizesHist(key->db, getKeySlot(key->key->ptr), OBJ_LIST, l+1, l); if (moduleDelKeyIfEmpty(key)) return REDISMODULE_OK; listTypeTryConversion(key->kv, LIST_CONV_SHRINKING, moduleFreeListIterator, key); if (!key->iter) return REDISMODULE_OK; /* Return ASAP if iterator has been freed */ @@ -4872,7 +4872,7 @@ int RM_ZsetIncrby(RedisModuleKey *key, double score, RedisModuleString *ele, int } if (out_flags & ZADD_OUT_ADDED) { int64_t l = (int64_t) zsetLength(key->kv); - updateKeysizesHist(key->db, getKeySlot(key->kv->ptr), OBJ_ZSET, l-1, l); + updateKeysizesHist(key->db, getKeySlot(key->key->ptr), OBJ_ZSET, l-1, l); } if (flagsptr) *flagsptr = moduleZsetAddFlagsFromCoreFlags(out_flags); return REDISMODULE_OK; @@ -4902,7 +4902,7 @@ int RM_ZsetRem(RedisModuleKey *key, RedisModuleString *ele, int *deleted) { if (key->kv != NULL && zsetDel(key->kv,ele->ptr)) { if (deleted) *deleted = 1; int64_t l = (int64_t) zsetLength(key->kv); - updateKeysizesHist(key->db, getKeySlot(key->kv->ptr), OBJ_ZSET, l+1, l); + updateKeysizesHist(key->db, getKeySlot(key->key->ptr), OBJ_ZSET, l+1, l); moduleDelKeyIfEmpty(key); } else { if (deleted) *deleted = 0; diff --git a/tests/unit/moduleapi/list.tcl b/tests/unit/moduleapi/list.tcl index 53f79ca4a..91793c54f 100644 --- a/tests/unit/moduleapi/list.tcl +++ b/tests/unit/moduleapi/list.tcl @@ -192,3 +192,34 @@ start_server {tags {"modules"}} { assert_equal {OK} [r module unload list] } } + +# A basic test that exercises a module's list commands under cluster mode. +# Currently, many module commands are never run even once in a clustered setup. +# This test helps ensure that basic module functionality works correctly and that +# the KEYSIZES histogram remains accurate and that insert & delete was tested. +set testmodule [file normalize tests/modules/list.so] +set modules [list loadmodule $testmodule] +start_cluster 2 2 [list config_lines [list loadmodule $testmodule enable-debug-command yes]] { + test "Module list - KEYSIZES is updated correctly in cluster mode" { + for {set srvid -2} {$srvid <= 0} {incr srvid} { + set instance [srv $srvid client] + # Assert consistency after each command + $instance DEBUG KEYSIZES-HIST-ASSERT 1 + + for {set i 0} {$i < 50} {incr i} { + for {set j 0} {$j < 4} {incr j} { + catch {$instance list.insert "list:$i" $j "item:$j"} e + if {![string match "OK" $e]} {assert_match "*MOVED*" $e} + } + } + for {set i 0} {$i < 50} {incr i} { + for {set j 0} {$j < 4} {incr j} { + catch {$instance list.delete "list:$i" 0} e + if {![string match "OK" $e]} {assert_match "*MOVED*" $e} + } + } + # Verify also that instance is responsive and didn't crash on assert + assert_equal [$instance dbsize] 0 + } + } +}