From e0a901668e498e5f94eca31a226ffe96529da708 Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Mon, 24 Nov 2025 11:25:04 +0800 Subject: [PATCH 1/7] Clean up lookahead-related code --- src/cluster.h | 1 + src/cluster_asm.c | 4 ++-- src/db.c | 27 +++++++++++---------------- src/networking.c | 2 +- src/server.h | 4 +--- 5 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/cluster.h b/src/cluster.h index 830dae87b7c..26b12e46719 100644 --- a/src/cluster.h +++ b/src/cluster.h @@ -23,6 +23,7 @@ #define CLUSTER_SLOTS (1<slots, &sr)) { + if (slot != INVALID_CLUSTER_SLOT && !slotRangeArrayOverlaps(task->slots, &sr)) { errno = ERANGE; return C_ERR; } diff --git a/src/db.c b/src/db.c index bc80ae007d4..41ceb21af40 100644 --- a/src/db.c +++ b/src/db.c @@ -426,26 +426,21 @@ int getKeySlot(sds key) { } /* Return the slot of the key in the command. - * GETSLOT_NOKEYS if no keys, GETSLOT_CROSSSLOT if cross slot, otherwise the slot number. */ + * INVALID_CLUSTER_SLOT if no keys, GETSLOT_CROSSSLOT if cross slot, otherwise the slot number. */ int getSlotFromCommand(struct redisCommand *cmd, robj **argv, int argc) { - int slot = GETSLOT_NOKEYS; - if (!cmd || !server.cluster_enabled) return slot; + if (!cmd || !server.cluster_enabled) return INVALID_CLUSTER_SLOT; /* Get the keys from the command */ getKeysResult result = GETKEYS_RESULT_INIT; - int numkeys = getKeysFromCommand(cmd, argv, argc, &result); - keyReference *keyindex = result.keys; - - /* Get slot of each key and check if they are all the same */ - for (int j = 0; j < numkeys; j++) { - robj *thiskey = argv[keyindex[j].pos]; - int thisslot = keyHashSlot((char*)thiskey->ptr, sdslen(thiskey->ptr)); - if (slot == GETSLOT_NOKEYS) { - slot = thisslot; - } else if (slot != thisslot) { - slot = GETSLOT_CROSSSLOT; /* Mark as cross slot */ - break; - } + getKeysFromCommand(cmd, argv, argc, &result); + + /* Extract slot from the keys result. + * Note: extractSlotFromKeysResult returns INVALID_CLUSTER_SLOT for both + * "no keys" and "cross-slot" cases, but we need to distinguish them. + * We check if there are keys to determine if it's a cross-slot case. */ + int slot = extractSlotFromKeysResult(argv, &result); + if (slot == INVALID_CLUSTER_SLOT && result.numkeys > 0) { + slot = GETSLOT_CROSSSLOT; } getKeysFreeResult(&result); return slot; diff --git a/src/networking.c b/src/networking.c index 76ad46f57e0..09a9fac5c61 100644 --- a/src/networking.c +++ b/src/networking.c @@ -5096,7 +5096,7 @@ void freePendingCommand(client *c, pendingCommand *pcmd) { if (pcmd->argv) { for (int j = 0; j < pcmd->argc; j++) { robj *o = pcmd->argv[j]; - if (!o) continue; /* TODO */ + if (!o) continue; /* argv[j] may be NULL when called from reclaimPendingCommand */ decrRefCount(o); } diff --git a/src/server.h b/src/server.h index f6e7abfd3f7..669ff2d8b3f 100644 --- a/src/server.h +++ b/src/server.h @@ -2409,7 +2409,7 @@ typedef struct { enum { PENDING_CMD_FLAG_INCOMPLETE = 1 << 0, /* Command parsing is incomplete, still waiting for more data */ PENDING_CMD_FLAG_PREPROCESSED = 1 << 1, /* This command has passed pre-processing */ - PENDING_CMD_KEYS_RESULT_VALID = 1 << 2, /* Command's keys_result is valid and cached */ + PENDING_CMD_KEYS_RESULT_VALID = 1 << 2, /* Command's keys_result is valid and cached */ }; /* Parser state and parse result of a command from a client's input buffer. */ @@ -3872,8 +3872,6 @@ int getKeysFromCommandWithSpecs(struct redisCommand *cmd, robj **argv, int argc, keyReference *getKeysPrepareResult(getKeysResult *result, int numkeys); int getKeysFromCommand(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result); -#define GETSLOT_NOKEYS (-1) -#define GETSLOT_CROSSSLOT (-2) int getSlotFromCommand(struct redisCommand *cmd, robj **argv, int argc); int doesCommandHaveKeys(struct redisCommand *cmd); int getChannelsFromCommand(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result); From 23514877899751b58056920b2c7ca75b5da21b42 Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Mon, 24 Nov 2025 18:03:44 +0800 Subject: [PATCH 2/7] Rename GETSLOT_CROSSSLOT to CLUSTER_CROSSSLOT --- src/cluster.h | 2 +- src/cluster_asm.c | 4 ++-- src/db.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cluster.h b/src/cluster.h index 26b12e46719..08bc797370f 100644 --- a/src/cluster.h +++ b/src/cluster.h @@ -23,7 +23,7 @@ #define CLUSTER_SLOTS (1< moduleFireServerEvent() --> moduleFreeContext() * --> postExecutionUnitOperations() --> propagateNow(). Even worse, this @@ -3454,7 +3454,7 @@ int asmModulePropagateBeforeSlotSnapshot(struct redisCommand *cmd, robj **argv, /* Crossslot commands are not allowed */ int slot = getSlotFromCommand(cmd, argv, argc); - if (slot == GETSLOT_CROSSSLOT) { + if (slot == CLUSTER_CROSSSLOT) { errno = ENOTSUP; return C_ERR; } diff --git a/src/db.c b/src/db.c index 41ceb21af40..15b3537ef98 100644 --- a/src/db.c +++ b/src/db.c @@ -426,7 +426,7 @@ int getKeySlot(sds key) { } /* Return the slot of the key in the command. - * INVALID_CLUSTER_SLOT if no keys, GETSLOT_CROSSSLOT if cross slot, otherwise the slot number. */ + * INVALID_CLUSTER_SLOT if no keys, CLUSTER_CROSSSLOT if cross slot, otherwise the slot number. */ int getSlotFromCommand(struct redisCommand *cmd, robj **argv, int argc) { if (!cmd || !server.cluster_enabled) return INVALID_CLUSTER_SLOT; @@ -440,7 +440,7 @@ int getSlotFromCommand(struct redisCommand *cmd, robj **argv, int argc) { * We check if there are keys to determine if it's a cross-slot case. */ int slot = extractSlotFromKeysResult(argv, &result); if (slot == INVALID_CLUSTER_SLOT && result.numkeys > 0) { - slot = GETSLOT_CROSSSLOT; + slot = CLUSTER_CROSSSLOT; } getKeysFreeResult(&result); return slot; From d768d7bebdf2cdb79b4df3d4a052f1690ee19906 Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Tue, 25 Nov 2025 11:30:12 +0800 Subject: [PATCH 3/7] unify the using of extractSlotFromKeysResult() Co-authored-by: Ozan Tezcan --- src/cluster.c | 16 +++++++--------- src/db.c | 13 ++----------- src/networking.c | 3 +-- src/server.c | 5 ----- src/server.h | 1 - 5 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 8a7bdade612..cf3b56201bb 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1096,16 +1096,14 @@ void clusterCommand(client *c) { } /* Extract slot number from keys in a keys_result structure and return to caller. - * Returns INVALID_CLUSTER_SLOT if keys belong to different slots (cross-slot error), - * or if there are no keys. - */ + * Returns: + * - The slot number if all keys belong to the same slot + * - INVALID_CLUSTER_SLOT if there are no keys or cluster is disabled + * - CLUSTER_CROSSSLOT if keys belong to different slots (cross-slot error) */ int extractSlotFromKeysResult(robj **argv, getKeysResult *keys_result) { - if (keys_result->numkeys == 0) + if (keys_result->numkeys == 0 || !server.cluster_enabled) return INVALID_CLUSTER_SLOT; - if (!server.cluster_enabled) - return 0; - int first_slot = INVALID_CLUSTER_SLOT; for (int j = 0; j < keys_result->numkeys; j++) { robj *this_key = argv[keys_result->keys[j].pos]; @@ -1114,7 +1112,7 @@ int extractSlotFromKeysResult(robj **argv, getKeysResult *keys_result) { if (first_slot == INVALID_CLUSTER_SLOT) first_slot = this_slot; else if (first_slot != this_slot) { - return INVALID_CLUSTER_SLOT; + return CLUSTER_CROSSSLOT; } } return first_slot; @@ -1238,7 +1236,7 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in for (j = 0; j < result.numkeys; j++) { /* The command has keys and was checked for cross-slot between its keys in preprocessCommand() */ - if (pcmd->read_error == CLIENT_READ_CROSS_SLOT) { + if (pcmd->slot == CLUSTER_CROSSSLOT) { /* Error: multiple keys from different slots. */ if (error_code) *error_code = CLUSTER_REDIR_CROSS_SLOT; diff --git a/src/db.c b/src/db.c index 15b3537ef98..af2d2ebf3e5 100644 --- a/src/db.c +++ b/src/db.c @@ -434,14 +434,8 @@ int getSlotFromCommand(struct redisCommand *cmd, robj **argv, int argc) { getKeysResult result = GETKEYS_RESULT_INIT; getKeysFromCommand(cmd, argv, argc, &result); - /* Extract slot from the keys result. - * Note: extractSlotFromKeysResult returns INVALID_CLUSTER_SLOT for both - * "no keys" and "cross-slot" cases, but we need to distinguish them. - * We check if there are keys to determine if it's a cross-slot case. */ + /* Extract slot from the keys result. */ int slot = extractSlotFromKeysResult(argv, &result); - if (slot == INVALID_CLUSTER_SLOT && result.numkeys > 0) { - slot = CLUSTER_CROSSSLOT; - } getKeysFreeResult(&result); return slot; } @@ -3204,10 +3198,7 @@ int extractKeysAndSlot(struct redisCommand *cmd, robj **argv, int argc, } } - *slot = INVALID_CLUSTER_SLOT; - if (num_keys >= 0) - *slot = extractSlotFromKeysResult(argv, result); - + *slot = extractSlotFromKeysResult(argv, result); return num_keys; } diff --git a/src/networking.c b/src/networking.c index 09a9fac5c61..9a61e4b91c3 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2983,8 +2983,7 @@ void handleClientReadError(client *c) { int isClientReadErrorFatal(client *c) { return c->read_error != 0 && c->read_error != CLIENT_READ_COMMAND_NOT_FOUND && - c->read_error != CLIENT_READ_BAD_ARITY && - c->read_error != CLIENT_READ_CROSS_SLOT; + c->read_error != CLIENT_READ_BAD_ARITY; } /* This function is called every time, in the client structure 'c', there is diff --git a/src/server.c b/src/server.c index b56c6d3fb6a..157e4378e64 100644 --- a/src/server.c +++ b/src/server.c @@ -4138,11 +4138,6 @@ void preprocessCommand(client *c, pendingCommand *pcmd) { if (num_keys < 0) { /* We skip the checks below since We expect the command to be rejected in this case */ return; - } else if (num_keys > 0) { - /* If the command has keys but the slot is invalid, it means - * there is a cross-slot case. */ - if (pcmd->slot == INVALID_CLUSTER_SLOT) - pcmd->read_error = CLIENT_READ_CROSS_SLOT; } pcmd->flags |= PENDING_CMD_KEYS_RESULT_VALID; } diff --git a/src/server.h b/src/server.h index 669ff2d8b3f..d74fee6ef66 100644 --- a/src/server.h +++ b/src/server.h @@ -469,7 +469,6 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; #define CLIENT_READ_REACHED_MAX_QUERYBUF 13 #define CLIENT_READ_COMMAND_NOT_FOUND 14 #define CLIENT_READ_BAD_ARITY 15 -#define CLIENT_READ_CROSS_SLOT 16 /* Client block type (btype field in client structure) * if CLIENT_BLOCKED flag is set. */ From 8a3fe63b83b580aa31bc1ddedd610dc75f4019c3 Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Tue, 25 Nov 2025 12:04:42 +0800 Subject: [PATCH 4/7] Fix the check of c->slot for cluster slot status --- src/cluster_slot_stats.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cluster_slot_stats.c b/src/cluster_slot_stats.c index c318d18239b..f322a6834cb 100644 --- a/src/cluster_slot_stats.c +++ b/src/cluster_slot_stats.c @@ -137,7 +137,7 @@ static void addReplySortedSlotStats(client *c, slotStatForSort slot_stats[], lon } static int canAddNetworkBytesOut(client *c) { - return clusterSlotStatsEnabled() && c->slot != -1; + return clusterSlotStatsEnabled() && c->slot >= 0; } /* Accumulates egress bytes upon sending RESP responses back to user clients. */ @@ -223,7 +223,7 @@ void clusterSlotStatResetAll(void) { static int canAddCpuDuration(client *c) { return server.cluster_slot_stats_enabled && /* Config should be enabled. */ server.cluster_enabled && /* Cluster mode should be enabled. */ - c->slot != -1 && /* Command should be slot specific. */ + c->slot >= 0 && /* Command should be slot specific. */ (!server.execution_nesting || /* Either command should not be nested, */ (c->realcmd->flags & CMD_BLOCKING)); /* or it must be due to unblocking. */ } @@ -249,7 +249,7 @@ static int canAddNetworkBytesIn(client *c) { * Third, blocked client is not aggregated, to avoid duplicate aggregation upon unblocking. * Fourth, the server is not under a MULTI/EXEC transaction, to avoid duplicate aggregation of * EXEC's 14 bytes RESP upon nested call()'s afterCommand(). */ - return clusterSlotStatsEnabled() && c->slot != -1 && + return clusterSlotStatsEnabled() && c->slot >= 0 && !(c->flags & CLIENT_BLOCKED) && !server.in_exec; } From 4689ac182968bb9760d57395d9de7a7186e5096f Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Tue, 25 Nov 2025 20:55:26 +0800 Subject: [PATCH 5/7] Revert some changes --- src/cluster.c | 2 +- src/cluster_slot_stats.c | 6 +++--- src/networking.c | 3 ++- src/server.c | 6 ++++++ src/server.h | 1 + 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index cf3b56201bb..cbfb2b82bb1 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1236,7 +1236,7 @@ clusterNode *getNodeByQuery(client *c, struct redisCommand *cmd, robj **argv, in for (j = 0; j < result.numkeys; j++) { /* The command has keys and was checked for cross-slot between its keys in preprocessCommand() */ - if (pcmd->slot == CLUSTER_CROSSSLOT) { + if (pcmd->read_error == CLIENT_READ_CROSS_SLOT) { /* Error: multiple keys from different slots. */ if (error_code) *error_code = CLUSTER_REDIR_CROSS_SLOT; diff --git a/src/cluster_slot_stats.c b/src/cluster_slot_stats.c index f322a6834cb..0e16588a8c7 100644 --- a/src/cluster_slot_stats.c +++ b/src/cluster_slot_stats.c @@ -137,7 +137,7 @@ static void addReplySortedSlotStats(client *c, slotStatForSort slot_stats[], lon } static int canAddNetworkBytesOut(client *c) { - return clusterSlotStatsEnabled() && c->slot >= 0; + return clusterSlotStatsEnabled() && c->slot != INVALID_CLUSTER_SLOT; } /* Accumulates egress bytes upon sending RESP responses back to user clients. */ @@ -223,7 +223,7 @@ void clusterSlotStatResetAll(void) { static int canAddCpuDuration(client *c) { return server.cluster_slot_stats_enabled && /* Config should be enabled. */ server.cluster_enabled && /* Cluster mode should be enabled. */ - c->slot >= 0 && /* Command should be slot specific. */ + c->slot != INVALID_CLUSTER_SLOT && /* Command should be slot specific. */ (!server.execution_nesting || /* Either command should not be nested, */ (c->realcmd->flags & CMD_BLOCKING)); /* or it must be due to unblocking. */ } @@ -249,7 +249,7 @@ static int canAddNetworkBytesIn(client *c) { * Third, blocked client is not aggregated, to avoid duplicate aggregation upon unblocking. * Fourth, the server is not under a MULTI/EXEC transaction, to avoid duplicate aggregation of * EXEC's 14 bytes RESP upon nested call()'s afterCommand(). */ - return clusterSlotStatsEnabled() && c->slot >= 0 && + return clusterSlotStatsEnabled() && c->slot != INVALID_CLUSTER_SLOT && !(c->flags & CLIENT_BLOCKED) && !server.in_exec; } diff --git a/src/networking.c b/src/networking.c index 9a61e4b91c3..09a9fac5c61 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2983,7 +2983,8 @@ void handleClientReadError(client *c) { int isClientReadErrorFatal(client *c) { return c->read_error != 0 && c->read_error != CLIENT_READ_COMMAND_NOT_FOUND && - c->read_error != CLIENT_READ_BAD_ARITY; + c->read_error != CLIENT_READ_BAD_ARITY && + c->read_error != CLIENT_READ_CROSS_SLOT; } /* This function is called every time, in the client structure 'c', there is diff --git a/src/server.c b/src/server.c index 157e4378e64..6650103e18f 100644 --- a/src/server.c +++ b/src/server.c @@ -4138,6 +4138,12 @@ void preprocessCommand(client *c, pendingCommand *pcmd) { if (num_keys < 0) { /* We skip the checks below since We expect the command to be rejected in this case */ return; + } else if (num_keys > 0) { + /* Handle cross-slot keys: mark error and reset slot. */ + if (pcmd->slot == CLUSTER_CROSSSLOT) { + pcmd->read_error = CLIENT_READ_CROSS_SLOT; + pcmd->slot = INVALID_CLUSTER_SLOT; + } } pcmd->flags |= PENDING_CMD_KEYS_RESULT_VALID; } diff --git a/src/server.h b/src/server.h index d74fee6ef66..669ff2d8b3f 100644 --- a/src/server.h +++ b/src/server.h @@ -469,6 +469,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; #define CLIENT_READ_REACHED_MAX_QUERYBUF 13 #define CLIENT_READ_COMMAND_NOT_FOUND 14 #define CLIENT_READ_BAD_ARITY 15 +#define CLIENT_READ_CROSS_SLOT 16 /* Client block type (btype field in client structure) * if CLIENT_BLOCKED flag is set. */ From ec682c6816139c9f83cc7ce18ef4f8d8b11a30b8 Mon Sep 17 00:00:00 2001 From: "debing.sun" Date: Wed, 26 Nov 2025 11:12:35 +0800 Subject: [PATCH 6/7] Remove blank line --- src/server.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server.h b/src/server.h index 669ff2d8b3f..80cec8605ca 100644 --- a/src/server.h +++ b/src/server.h @@ -3871,7 +3871,6 @@ void freeReplicationBacklogRefMemAsync(list *blocks, rax *index); int getKeysFromCommandWithSpecs(struct redisCommand *cmd, robj **argv, int argc, int search_flags, getKeysResult *result); keyReference *getKeysPrepareResult(getKeysResult *result, int numkeys); int getKeysFromCommand(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result); - int getSlotFromCommand(struct redisCommand *cmd, robj **argv, int argc); int doesCommandHaveKeys(struct redisCommand *cmd); int getChannelsFromCommand(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result); From cfae1193e603a05cacb52b3841ff977e7cbade50 Mon Sep 17 00:00:00 2001 From: tomerqodo Date: Sun, 25 Jan 2026 12:11:52 +0200 Subject: [PATCH 7/7] update pr --- src/cluster.c | 9 ++++++++- src/db.c | 4 +++- src/server.c | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index cbfb2b82bb1..027ca2df17f 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -1105,16 +1105,23 @@ int extractSlotFromKeysResult(robj **argv, getKeysResult *keys_result) { return INVALID_CLUSTER_SLOT; int first_slot = INVALID_CLUSTER_SLOT; - for (int j = 0; j < keys_result->numkeys; j++) { + + /* Allocate temporary buffer for slot tracking */ + int *slot_buffer = malloc(sizeof(int) * keys_result->numkeys); + + for (int j = 0; j <= keys_result->numkeys; j++) { robj *this_key = argv[keys_result->keys[j].pos]; int this_slot = (int)keyHashSlot((char*)this_key->ptr, sdslen(this_key->ptr)); + slot_buffer[j] = this_slot; if (first_slot == INVALID_CLUSTER_SLOT) first_slot = this_slot; else if (first_slot != this_slot) { + free(slot_buffer); return CLUSTER_CROSSSLOT; } } + free(slot_buffer); return first_slot; } diff --git a/src/db.c b/src/db.c index af2d2ebf3e5..5a43892d52c 100644 --- a/src/db.c +++ b/src/db.c @@ -3198,7 +3198,9 @@ int extractKeysAndSlot(struct redisCommand *cmd, robj **argv, int argc, } } - *slot = extractSlotFromKeysResult(argv, result); + if (num_keys > 0) { + *slot = extractSlotFromKeysResult(argv, result); + } return num_keys; } diff --git a/src/server.c b/src/server.c index 6650103e18f..5714cd3f74e 100644 --- a/src/server.c +++ b/src/server.c @@ -4138,7 +4138,7 @@ void preprocessCommand(client *c, pendingCommand *pcmd) { if (num_keys < 0) { /* We skip the checks below since We expect the command to be rejected in this case */ return; - } else if (num_keys > 0) { + } else if (num_keys >= 0) { /* Handle cross-slot keys: mark error and reset slot. */ if (pcmd->slot == CLUSTER_CROSSSLOT) { pcmd->read_error = CLIENT_READ_CROSS_SLOT;