From 6b6f4f3dce26d130ef3859363b5b7ded641b8f1c Mon Sep 17 00:00:00 2001 From: Evgeny Barskiy Date: Thu, 13 Nov 2025 22:06:15 +0000 Subject: [PATCH 1/6] Added cache for COMMAND Signed-off-by: Evgeny Barskiy --- src/module.c | 12 +++++ src/server.c | 129 ++++++++++++++++++++++++++++++++++++++++++--------- src/server.h | 5 ++ 3 files changed, 125 insertions(+), 21 deletions(-) diff --git a/src/module.c b/src/module.c index 5c770141b5..e80659316b 100644 --- a/src/module.c +++ b/src/module.c @@ -1385,6 +1385,8 @@ int VM_CreateCommand(ValkeyModuleCtx *ctx, serverAssert(hashtableAdd(server.commands, cp->serverCmd)); serverAssert(hashtableAdd(server.orig_commands, cp->serverCmd)); cp->serverCmd->id = ACLGetCommandID(declared_name); /* ID used for ACL. */ + /* Invalidate COMMAND response cache since we added a new command */ + invalidateCommandCache(); return VALKEYMODULE_OK; } @@ -12530,6 +12532,14 @@ int moduleFreeCommand(struct ValkeyModule *module, struct serverCommand *cmd) { hdr_close(cmd->latency_histogram); cmd->latency_histogram = NULL; } + if (cmd->info_cache_resp2) { + sdsfree(cmd->info_cache_resp2); + cmd->info_cache_resp2 = NULL; + } + if (cmd->info_cache_resp3) { + sdsfree(cmd->info_cache_resp3); + cmd->info_cache_resp3 = NULL; + } moduleFreeArgs(cmd->args, cmd->num_args); zfree(cp); @@ -12571,6 +12581,8 @@ void moduleUnregisterCommands(struct ValkeyModule *module) { zfree(cmd); } hashtableResetIterator(&iter); + /* Invalidate COMMAND response cache since we removed commands */ + invalidateCommandCache(); } /* We parse argv to add sds "NAME VALUE" pairs to the server.module_configs_queue list of configs. diff --git a/src/server.c b/src/server.c index eeaec5d9d9..258ea88019 100644 --- a/src/server.c +++ b/src/server.c @@ -2342,6 +2342,8 @@ void initServerConfig(void) { * valkey.conf using the rename-command directive. */ server.commands = hashtableCreate(&commandSetType); server.orig_commands = hashtableCreate(&originalCommandSetType); + server.command_response_cache_resp2 = NULL; + server.command_response_cache_resp3 = NULL; populateCommandTable(); /* Debugging */ @@ -3282,6 +3284,10 @@ int populateCommandStructure(struct serverCommand *c) { * has been issued for the first time */ c->latency_histogram = NULL; + /* Initialize command info cache */ + c->info_cache_resp2 = NULL; + c->info_cache_resp3 = NULL; + /* Handle the legacy range spec and the "movablekeys" flag (must be done after populating all key specs). */ populateCommandLegacyRangeSpec(c); @@ -5222,30 +5228,77 @@ void addReplyCommandSubCommands(client *c, hashtableResetIterator(&iter); } +/* Forward declaration */ +void addReplyCommandInfo(client *c, struct serverCommand *cmd); + +/* Collect all output from a caching client (both buffer and reply list) */ +static sds collectCachedResponse(client *c) { + sds response = sdsempty(); + + /* First, collect from the fixed buffer if any */ + if (c->bufpos > 0) { + response = sdscatlen(response, c->buf, c->bufpos); + } + + /* Then, collect from the reply list */ + listIter li; + listNode *ln; + clientReplyBlock *val_block; + listRewind(c->reply, &li); + while ((ln = listNext(&li)) != NULL) { + val_block = (clientReplyBlock *)listNodeValue(ln); + response = sdscatlen(response, val_block->buf, val_block->used); + } + + return response; +} + +/* Generate and cache the command info response for a given protocol version */ +static void cacheCommandInfo(struct serverCommand *cmd, int resp) { + client *caching_client = createCachedResponseClient(resp); + + int firstkey = 0, lastkey = 0, keystep = 0; + if (cmd->legacy_range_key_spec.begin_search_type != KSPEC_BS_INVALID) { + firstkey = cmd->legacy_range_key_spec.bs.index.pos; + lastkey = cmd->legacy_range_key_spec.fk.range.lastkey; + if (lastkey >= 0) lastkey += firstkey; + keystep = cmd->legacy_range_key_spec.fk.range.keystep; + } + + addReplyArrayLen(caching_client, 10); + addReplyBulkCBuffer(caching_client, cmd->fullname, sdslen(cmd->fullname)); + addReplyLongLong(caching_client, cmd->arity); + addReplyFlagsForCommand(caching_client, cmd); + addReplyLongLong(caching_client, firstkey); + addReplyLongLong(caching_client, lastkey); + addReplyLongLong(caching_client, keystep); + addReplyCommandCategories(caching_client, cmd); + addReplyCommandTips(caching_client, cmd); + addReplyCommandKeySpecs(caching_client, cmd); + addReplyCommandSubCommands(caching_client, cmd, addReplyCommandInfo, 0); + + if (resp == 2) { + cmd->info_cache_resp2 = collectCachedResponse(caching_client); + } else { + cmd->info_cache_resp3 = collectCachedResponse(caching_client); + } + + deleteCachedResponseClient(caching_client); +} + /* Output the representation of a server command. Used by the COMMAND command and COMMAND INFO. */ void addReplyCommandInfo(client *c, struct serverCommand *cmd) { if (!cmd) { addReplyNull(c); } else { - int firstkey = 0, lastkey = 0, keystep = 0; - if (cmd->legacy_range_key_spec.begin_search_type != KSPEC_BS_INVALID) { - firstkey = cmd->legacy_range_key_spec.bs.index.pos; - lastkey = cmd->legacy_range_key_spec.fk.range.lastkey; - if (lastkey >= 0) lastkey += firstkey; - keystep = cmd->legacy_range_key_spec.fk.range.keystep; + /* Use cached response if available for the client's protocol version */ + sds *cache = (c->resp == 2) ? &cmd->info_cache_resp2 : &cmd->info_cache_resp3; + + if (*cache == NULL) { + cacheCommandInfo(cmd, c->resp); } - - addReplyArrayLen(c, 10); - addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); - addReplyLongLong(c, cmd->arity); - addReplyFlagsForCommand(c, cmd); - addReplyLongLong(c, firstkey); - addReplyLongLong(c, lastkey); - addReplyLongLong(c, keystep); - addReplyCommandCategories(c, cmd); - addReplyCommandTips(c, cmd); - addReplyCommandKeySpecs(c, cmd); - addReplyCommandSubCommands(c, cmd, addReplyCommandInfo, 0); + + addReplyProto(c, *cache, sdslen(*cache)); } } @@ -5371,16 +5424,50 @@ void getKeysSubcommand(client *c) { } /* COMMAND (no args) */ -void commandCommand(client *c) { +/* Invalidate the cached COMMAND response when command table changes */ +void invalidateCommandCache(void) { + if (server.command_response_cache_resp2) { + sdsfree(server.command_response_cache_resp2); + server.command_response_cache_resp2 = NULL; + } + if (server.command_response_cache_resp3) { + sdsfree(server.command_response_cache_resp3); + server.command_response_cache_resp3 = NULL; + } +} + +/* Generate and cache the full COMMAND response */ +static void cacheCommandResponse(int resp) { + client *caching_client = createCachedResponseClient(resp); + hashtableIterator iter; void *next; - addReplyArrayLen(c, hashtableSize(server.commands)); + addReplyArrayLen(caching_client, hashtableSize(server.commands)); hashtableInitIterator(&iter, server.commands, 0); while (hashtableNext(&iter, &next)) { struct serverCommand *cmd = next; - addReplyCommandInfo(c, cmd); + addReplyCommandInfo(caching_client, cmd); } hashtableResetIterator(&iter); + + if (resp == 2) { + server.command_response_cache_resp2 = collectCachedResponse(caching_client); + } else { + server.command_response_cache_resp3 = collectCachedResponse(caching_client); + } + + deleteCachedResponseClient(caching_client); +} + +void commandCommand(client *c) { + /* Use cached response if available for the client's protocol version */ + sds *cache = (c->resp == 2) ? &server.command_response_cache_resp2 : &server.command_response_cache_resp3; + + if (*cache == NULL) { + cacheCommandResponse(c->resp); + } + + addReplyProto(c, *cache, sdslen(*cache)); } /* COMMAND COUNT */ diff --git a/src/server.h b/src/server.h index e1fdce4cfc..6bc2579681 100644 --- a/src/server.h +++ b/src/server.h @@ -1677,6 +1677,8 @@ struct valkeyServer { serverDb **db; /* each db created when it's first used */ hashtable *commands; /* Command table */ hashtable *orig_commands; /* Command table before command renaming. */ + sds command_response_cache_resp2; /* Cached COMMAND response for RESP2 */ + sds command_response_cache_resp3; /* Cached COMMAND response for RESP3 */ aeEventLoop *el; _Atomic AeIoState io_poll_state; /* Indicates the state of the IO polling. */ int io_ae_fired_events; /* Number of poll events received by the IO thread. */ @@ -2626,6 +2628,8 @@ struct serverCommand { * (not the fullname), and the value is the serverCommand structure pointer. */ struct serverCommand *parent; struct ValkeyModuleCommand *module_cmd; /* A pointer to the module command data (NULL if native command) */ + sds info_cache_resp2; /* Cached COMMAND INFO response for RESP2 */ + sds info_cache_resp3; /* Cached COMMAND INFO response for RESP3 */ }; struct serverError { @@ -3783,6 +3787,7 @@ void commandCommand(client *c); void commandCountCommand(client *c); void commandListCommand(client *c); void commandInfoCommand(client *c); +void invalidateCommandCache(void); void commandGetKeysCommand(client *c); void commandGetKeysAndFlagsCommand(client *c); void commandHelpCommand(client *c); From 4bf34f34ffdce0e03f86aeaac12dfa6ffbaf3a65 Mon Sep 17 00:00:00 2001 From: Evgeny Barskiy Date: Thu, 13 Nov 2025 22:20:55 +0000 Subject: [PATCH 2/6] Added cache for COMMAND Signed-off-by: Evgeny Barskiy --- src/server.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server.c b/src/server.c index 258ea88019..7c8b1be4ed 100644 --- a/src/server.c +++ b/src/server.c @@ -5228,9 +5228,6 @@ void addReplyCommandSubCommands(client *c, hashtableResetIterator(&iter); } -/* Forward declaration */ -void addReplyCommandInfo(client *c, struct serverCommand *cmd); - /* Collect all output from a caching client (both buffer and reply list) */ static sds collectCachedResponse(client *c) { sds response = sdsempty(); @@ -5253,6 +5250,9 @@ static sds collectCachedResponse(client *c) { return response; } +/* Forward declaration */ +void addReplyCommandInfo(client *c, struct serverCommand *cmd); + /* Generate and cache the command info response for a given protocol version */ static void cacheCommandInfo(struct serverCommand *cmd, int resp) { client *caching_client = createCachedResponseClient(resp); @@ -5423,7 +5423,6 @@ void getKeysSubcommand(client *c) { getKeysSubcommandImpl(c, 0); } -/* COMMAND (no args) */ /* Invalidate the cached COMMAND response when command table changes */ void invalidateCommandCache(void) { if (server.command_response_cache_resp2) { @@ -5459,6 +5458,7 @@ static void cacheCommandResponse(int resp) { deleteCachedResponseClient(caching_client); } +/* COMMAND (no args) */ void commandCommand(client *c) { /* Use cached response if available for the client's protocol version */ sds *cache = (c->resp == 2) ? &server.command_response_cache_resp2 : &server.command_response_cache_resp3; From 06a7a71dc4097bbadd99ed970943555cd8f2e7c4 Mon Sep 17 00:00:00 2001 From: Evgeny Barskiy Date: Fri, 14 Nov 2025 19:50:56 +0000 Subject: [PATCH 3/6] removed pointer Signed-off-by: Evgeny Barskiy --- src/server.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/server.c b/src/server.c index 7c8b1be4ed..db76ea7105 100644 --- a/src/server.c +++ b/src/server.c @@ -5292,13 +5292,14 @@ void addReplyCommandInfo(client *c, struct serverCommand *cmd) { addReplyNull(c); } else { /* Use cached response if available for the client's protocol version */ - sds *cache = (c->resp == 2) ? &cmd->info_cache_resp2 : &cmd->info_cache_resp3; + sds cache = (c->resp == 2) ? cmd->info_cache_resp2 : cmd->info_cache_resp3; - if (*cache == NULL) { + if (cache == NULL) { cacheCommandInfo(cmd, c->resp); + cache = (c->resp == 2) ? cmd->info_cache_resp2 : cmd->info_cache_resp3; } - addReplyProto(c, *cache, sdslen(*cache)); + addReplyProto(c, cache, sdslen(cache)); } } @@ -5461,13 +5462,14 @@ static void cacheCommandResponse(int resp) { /* COMMAND (no args) */ void commandCommand(client *c) { /* Use cached response if available for the client's protocol version */ - sds *cache = (c->resp == 2) ? &server.command_response_cache_resp2 : &server.command_response_cache_resp3; + sds cache = (c->resp == 2) ? server.command_response_cache_resp2 : server.command_response_cache_resp3; - if (*cache == NULL) { + if (cache == NULL) { cacheCommandResponse(c->resp); + cache = (c->resp == 2) ? server.command_response_cache_resp2 : server.command_response_cache_resp3; } - addReplyProto(c, *cache, sdslen(*cache)); + addReplyProto(c, cache, sdslen(cache)); } /* COMMAND COUNT */ From ec497e82a6c1c15b289e0ed24cc0e9c50aec9359 Mon Sep 17 00:00:00 2001 From: Evgeny Barskiy Date: Tue, 18 Nov 2025 01:29:02 +0000 Subject: [PATCH 4/6] switch to array Signed-off-by: Evgeny Barskiy --- src/module.c | 12 +++++------- src/server.c | 44 +++++++++++++++++++------------------------- src/server.h | 9 +++++---- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/src/module.c b/src/module.c index e80659316b..76e8f8b4d2 100644 --- a/src/module.c +++ b/src/module.c @@ -12532,13 +12532,11 @@ int moduleFreeCommand(struct ValkeyModule *module, struct serverCommand *cmd) { hdr_close(cmd->latency_histogram); cmd->latency_histogram = NULL; } - if (cmd->info_cache_resp2) { - sdsfree(cmd->info_cache_resp2); - cmd->info_cache_resp2 = NULL; - } - if (cmd->info_cache_resp3) { - sdsfree(cmd->info_cache_resp3); - cmd->info_cache_resp3 = NULL; + for (int i = 0; i < RESP_CACHE_INDEX_MAX; i++) { + if (cmd->info_cache[i]) { + sdsfree(cmd->info_cache[i]); + cmd->info_cache[i] = NULL; + } } moduleFreeArgs(cmd->args, cmd->num_args); zfree(cp); diff --git a/src/server.c b/src/server.c index db76ea7105..da6eeea0c2 100644 --- a/src/server.c +++ b/src/server.c @@ -2342,8 +2342,9 @@ void initServerConfig(void) { * valkey.conf using the rename-command directive. */ server.commands = hashtableCreate(&commandSetType); server.orig_commands = hashtableCreate(&originalCommandSetType); - server.command_response_cache_resp2 = NULL; - server.command_response_cache_resp3 = NULL; + for (int i = 0; i < RESP_CACHE_INDEX_MAX; i++) { + server.command_response_cache[i] = NULL; + } populateCommandTable(); /* Debugging */ @@ -3285,8 +3286,9 @@ int populateCommandStructure(struct serverCommand *c) { c->latency_histogram = NULL; /* Initialize command info cache */ - c->info_cache_resp2 = NULL; - c->info_cache_resp3 = NULL; + for (int i = 0; i < RESP_CACHE_INDEX_MAX; i++) { + c->info_cache[i] = NULL; + } /* Handle the legacy range spec and the "movablekeys" flag (must be done after populating all key specs). */ populateCommandLegacyRangeSpec(c); @@ -5277,11 +5279,7 @@ static void cacheCommandInfo(struct serverCommand *cmd, int resp) { addReplyCommandKeySpecs(caching_client, cmd); addReplyCommandSubCommands(caching_client, cmd, addReplyCommandInfo, 0); - if (resp == 2) { - cmd->info_cache_resp2 = collectCachedResponse(caching_client); - } else { - cmd->info_cache_resp3 = collectCachedResponse(caching_client); - } + cmd->info_cache[RESP_CACHE_INDEX(resp)] = collectCachedResponse(caching_client); deleteCachedResponseClient(caching_client); } @@ -5292,11 +5290,12 @@ void addReplyCommandInfo(client *c, struct serverCommand *cmd) { addReplyNull(c); } else { /* Use cached response if available for the client's protocol version */ - sds cache = (c->resp == 2) ? cmd->info_cache_resp2 : cmd->info_cache_resp3; + int cache_idx = RESP_CACHE_INDEX(c->resp); + sds cache = cmd->info_cache[cache_idx]; if (cache == NULL) { cacheCommandInfo(cmd, c->resp); - cache = (c->resp == 2) ? cmd->info_cache_resp2 : cmd->info_cache_resp3; + cache = cmd->info_cache[cache_idx]; } addReplyProto(c, cache, sdslen(cache)); @@ -5426,13 +5425,11 @@ void getKeysSubcommand(client *c) { /* Invalidate the cached COMMAND response when command table changes */ void invalidateCommandCache(void) { - if (server.command_response_cache_resp2) { - sdsfree(server.command_response_cache_resp2); - server.command_response_cache_resp2 = NULL; - } - if (server.command_response_cache_resp3) { - sdsfree(server.command_response_cache_resp3); - server.command_response_cache_resp3 = NULL; + for (int i = 0; i < RESP_CACHE_INDEX_MAX; i++) { + if (server.command_response_cache[i]) { + sdsfree(server.command_response_cache[i]); + server.command_response_cache[i] = NULL; + } } } @@ -5450,11 +5447,7 @@ static void cacheCommandResponse(int resp) { } hashtableResetIterator(&iter); - if (resp == 2) { - server.command_response_cache_resp2 = collectCachedResponse(caching_client); - } else { - server.command_response_cache_resp3 = collectCachedResponse(caching_client); - } + server.command_response_cache[RESP_CACHE_INDEX(resp)] = collectCachedResponse(caching_client); deleteCachedResponseClient(caching_client); } @@ -5462,11 +5455,12 @@ static void cacheCommandResponse(int resp) { /* COMMAND (no args) */ void commandCommand(client *c) { /* Use cached response if available for the client's protocol version */ - sds cache = (c->resp == 2) ? server.command_response_cache_resp2 : server.command_response_cache_resp3; + int cache_idx = RESP_CACHE_INDEX(c->resp); + sds cache = server.command_response_cache[cache_idx]; if (cache == NULL) { cacheCommandResponse(c->resp); - cache = (c->resp == 2) ? server.command_response_cache_resp2 : server.command_response_cache_resp3; + cache = server.command_response_cache[cache_idx]; } addReplyProto(c, cache, sdslen(cache)); diff --git a/src/server.h b/src/server.h index 6bc2579681..cf834faacc 100644 --- a/src/server.h +++ b/src/server.h @@ -1566,6 +1566,9 @@ struct malloc_stats { #define CACHE_CONN_TYPE_RESP3 (1 << 2) #define CACHE_CONN_TYPE_MAX (1 << 3) +#define RESP_CACHE_INDEX_MAX 2 /* [0]=RESP2, [1]=RESP3 */ +#define RESP_CACHE_INDEX(resp) ((resp) == 3 ? 1 : 0) /* Convert RESP version to cache array index */ + /*----------------------------------------------------------------------------- * TLS Context Configuration *----------------------------------------------------------------------------*/ @@ -1677,8 +1680,7 @@ struct valkeyServer { serverDb **db; /* each db created when it's first used */ hashtable *commands; /* Command table */ hashtable *orig_commands; /* Command table before command renaming. */ - sds command_response_cache_resp2; /* Cached COMMAND response for RESP2 */ - sds command_response_cache_resp3; /* Cached COMMAND response for RESP3 */ + sds command_response_cache[RESP_CACHE_INDEX_MAX]; /* Cached COMMAND response: [0]=RESP2, [1]=RESP3 */ aeEventLoop *el; _Atomic AeIoState io_poll_state; /* Indicates the state of the IO polling. */ int io_ae_fired_events; /* Number of poll events received by the IO thread. */ @@ -2628,8 +2630,7 @@ struct serverCommand { * (not the fullname), and the value is the serverCommand structure pointer. */ struct serverCommand *parent; struct ValkeyModuleCommand *module_cmd; /* A pointer to the module command data (NULL if native command) */ - sds info_cache_resp2; /* Cached COMMAND INFO response for RESP2 */ - sds info_cache_resp3; /* Cached COMMAND INFO response for RESP3 */ + sds info_cache[RESP_CACHE_INDEX_MAX]; /* Cached COMMAND INFO response: [0]=RESP2, [1]=RESP3 */ }; struct serverError { From 6aae939884c71c149a99542ea092a196d5578db5 Mon Sep 17 00:00:00 2001 From: Evgeny Barskiy Date: Wed, 19 Nov 2025 19:18:26 +0000 Subject: [PATCH 5/6] change aggregate function to allow buffer Signed-off-by: Evgeny Barskiy --- src/cluster.c | 2 ++ src/networking.c | 12 ++++++++---- src/server.c | 26 ++------------------------ 3 files changed, 12 insertions(+), 28 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index a188bef335..42bdebfaf1 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1522,6 +1522,8 @@ sds generateClusterSlotResponse(int resp) { } } setDeferredArrayLen(recording_client, slot_replylen, num_primaries); + /* For cluster slots, deferred length should put all data in reply list, not buffer */ + serverAssert(recording_client->bufpos == 0); sds cluster_slot_response = aggregateClientOutputBuffer(recording_client); deleteCachedResponseClient(recording_client); return cluster_slot_response; diff --git a/src/networking.c b/src/networking.c index 4bcf0370e2..9fe7a7e66a 100644 --- a/src/networking.c +++ b/src/networking.c @@ -464,17 +464,21 @@ int prepareClientToWrite(client *c) { return C_OK; } -/* Returns everything in the client reply linked list in a SDS format. +/* Returns everything in the client reply buffer and linked list in a SDS format. * This should only be used only with a caching client. */ sds aggregateClientOutputBuffer(client *c) { sds cmd_response = sdsempty(); + + /* First, collect from the fixed buffer if any */ + if (c->bufpos > 0) { + cmd_response = sdscatlen(cmd_response, c->buf, c->bufpos); + } + + /* Then, collect from the reply list */ listIter li; listNode *ln; clientReplyBlock *val_block; listRewind(c->reply, &li); - - /* Here, c->buf is not used, thus we confirm c->bufpos remains 0. */ - serverAssert(c->bufpos == 0); while ((ln = listNext(&li)) != NULL) { val_block = (clientReplyBlock *)listNodeValue(ln); cmd_response = sdscatlen(cmd_response, val_block->buf, val_block->used); diff --git a/src/server.c b/src/server.c index da6eeea0c2..d2e14789a8 100644 --- a/src/server.c +++ b/src/server.c @@ -5230,28 +5230,6 @@ void addReplyCommandSubCommands(client *c, hashtableResetIterator(&iter); } -/* Collect all output from a caching client (both buffer and reply list) */ -static sds collectCachedResponse(client *c) { - sds response = sdsempty(); - - /* First, collect from the fixed buffer if any */ - if (c->bufpos > 0) { - response = sdscatlen(response, c->buf, c->bufpos); - } - - /* Then, collect from the reply list */ - listIter li; - listNode *ln; - clientReplyBlock *val_block; - listRewind(c->reply, &li); - while ((ln = listNext(&li)) != NULL) { - val_block = (clientReplyBlock *)listNodeValue(ln); - response = sdscatlen(response, val_block->buf, val_block->used); - } - - return response; -} - /* Forward declaration */ void addReplyCommandInfo(client *c, struct serverCommand *cmd); @@ -5279,7 +5257,7 @@ static void cacheCommandInfo(struct serverCommand *cmd, int resp) { addReplyCommandKeySpecs(caching_client, cmd); addReplyCommandSubCommands(caching_client, cmd, addReplyCommandInfo, 0); - cmd->info_cache[RESP_CACHE_INDEX(resp)] = collectCachedResponse(caching_client); + cmd->info_cache[RESP_CACHE_INDEX(resp)] = aggregateClientOutputBuffer(caching_client); deleteCachedResponseClient(caching_client); } @@ -5447,7 +5425,7 @@ static void cacheCommandResponse(int resp) { } hashtableResetIterator(&iter); - server.command_response_cache[RESP_CACHE_INDEX(resp)] = collectCachedResponse(caching_client); + server.command_response_cache[RESP_CACHE_INDEX(resp)] = aggregateClientOutputBuffer(caching_client); deleteCachedResponseClient(caching_client); } From 8b77e7a96775394cb5523c1c0a0904fcbde6dddd Mon Sep 17 00:00:00 2001 From: Evgeny Barskiy Date: Sat, 22 Nov 2025 07:14:43 +0000 Subject: [PATCH 6/6] vibe test Signed-off-by: Evgeny Barskiy --- tests/unit/command.tcl | 162 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 tests/unit/command.tcl diff --git a/tests/unit/command.tcl b/tests/unit/command.tcl new file mode 100644 index 0000000000..c419ecc7ba --- /dev/null +++ b/tests/unit/command.tcl @@ -0,0 +1,162 @@ +set testmodule [file normalize tests/modules/basics.so] + +start_server {tags {"introspection modules"}} { + test {COMMAND caches response} { + # Call COMMAND twice and verify we get the same commands + set cmd1 [r command] + set cmd2 [r command] + assert_equal [llength $cmd1] [llength $cmd2] + } + + test {COMMAND cache is invalidated on module load} { + # Get initial command list + set commands_before [r command] + set count_before [llength $commands_before] + + # Verify module commands are not present + set module_commands_found 0 + foreach cmd $commands_before { + set name [lindex $cmd 0] + if {[string match "test.*" $name]} { + incr module_commands_found + } + } + assert_equal $module_commands_found 0 + + # Load module which should invalidate the cache + r module load $testmodule + + # Get command list again + set commands_after [r command] + set count_after [llength $commands_after] + + # Verify we have more commands now + assert {$count_after > $count_before} + + # Verify module commands are now present + set module_commands_found 0 + foreach cmd $commands_after { + set name [lindex $cmd 0] + if {[string match "test.*" $name]} { + incr module_commands_found + } + } + assert {$module_commands_found > 0} + } + + test {COMMAND INFO works with module commands} { + # Verify we can get info about a specific module command + set info [r command info test.basics] + assert_equal [llength $info] 1 + assert_equal [lindex [lindex $info 0] 0] "test.basics" + } + + test {COMMAND COUNT includes module commands} { + set count_with_module [r command count] + + # Count should be greater than typical server commands + assert {$count_with_module > 200} + } + + test {COMMAND cache is invalidated on module unload} { + # Get command list with module loaded + set commands_before [r command] + set count_before [llength $commands_before] + + # Verify module commands are present + set module_commands_found 0 + foreach cmd $commands_before { + set name [lindex $cmd 0] + if {[string match "test.*" $name]} { + incr module_commands_found + } + } + assert {$module_commands_found > 0} + + # Unload module which should invalidate the cache + r module unload test + + # Get command list again + set commands_after [r command] + set count_after [llength $commands_after] + + # Verify we have fewer commands now + assert {$count_after < $count_before} + + # Verify module commands are no longer present + set module_commands_found 0 + foreach cmd $commands_after { + set name [lindex $cmd 0] + if {[string match "test.*" $name]} { + incr module_commands_found + } + } + assert_equal $module_commands_found 0 + } + + test {COMMAND INFO returns empty for unloaded module commands} { + # Verify we get empty/null response for module command after unload + set info [r command info test.basics] + # COMMAND INFO returns a list with one empty element for non-existent commands + assert_equal [llength $info] 1 + assert_equal [lindex $info 0] {} + } +} + +start_server {tags {"introspection modules"}} { + test {COMMAND cache invalidation with multiple load/unload cycles} { + # Get baseline + set baseline_count [r command count] + + # Load module + r module load $testmodule + set count_loaded [r command count] + assert {$count_loaded > $baseline_count} + + # Unload module + r module unload test + set count_unloaded [r command count] + assert_equal $count_unloaded $baseline_count + + # Load again + r module load $testmodule + set count_loaded_again [r command count] + assert_equal $count_loaded_again $count_loaded + + # Unload again + r module unload test + set count_unloaded_again [r command count] + assert_equal $count_unloaded_again $baseline_count + } +} + +start_server {tags {"introspection modules"}} { + test {COMMAND cache works correctly with RESP2 and RESP3} { + # Test with RESP2 + r hello 2 + set commands_resp2_before [r command] + + # Load module + r module load $testmodule + set commands_resp2_after [r command] + assert {[llength $commands_resp2_after] > [llength $commands_resp2_before]} + + # Switch to RESP3 and verify cache works + r hello 3 + set commands_resp3 [r command] + # Both should have the same number of commands + assert_equal [llength $commands_resp3] [llength $commands_resp2_after] + + # Unload module + r module unload test + + # Verify both RESP versions see the change + set commands_resp3_after [r command] + assert {[llength $commands_resp3_after] < [llength $commands_resp3]} + + r hello 2 + set commands_resp2_final [r command] + assert_equal [llength $commands_resp2_final] [llength $commands_resp3_after] + } +} +