Skip to content

Commit

Permalink
When in cluster mode we should be more permissive about what we retur…
Browse files Browse the repository at this point in the history
…n with MGET. Currently we check if all keys are the same slot but we could retunr if all slots are on this node instead.

Signed-off-by: John Sully <[email protected]>
  • Loading branch information
JohnSully committed Jun 28, 2024
1 parent 7719dbb commit a63b137
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/cluster.c
Original file line number Diff line number Diff line change
Expand Up @@ -1057,8 +1057,14 @@ getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, int argc, int
}
} else {
/* If it is not the first key/channel, make sure it is exactly
* the same key/channel as the first we saw. */
if (slot != thisslot) {
* the same key/channel as the first we saw or an allowed crossslot situation. */
int prevent_crossslot = (cmd_flags & CMD_WRITE) // eliminate issues with client->current_slot
|| pubsubshard_included // pubsub does not benefit and too many edge cases
|| c->cmd->proc == execCommand // We do not permit crossslot transactions to prevent client code which will break when cluster topology changes
// finally, if any key is migrating we cannot permit the crossslot since we don't check if the specific keys are affected
// this could potentially be relaxed in the future.
|| migrating_slot || importing_slot || getMigratingSlotDest(slot) != NULL || getImportingSlotSource(slot) != NULL;
if (slot != thisslot && prevent_crossslot) {
/* Error: multiple keys from different slots. */
getKeysFreeResult(&result);
if (error_code) *error_code = CLUSTER_REDIR_CROSS_SLOT;
Expand Down
3 changes: 2 additions & 1 deletion src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -5388,7 +5388,8 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) {
"executable:%s\r\n", server.executable ? server.executable : "",
"config_file:%s\r\n", server.configfile ? server.configfile : "",
"io_threads_active:%i\r\n", server.io_threads_active,
"availability_zone:%s\r\n", server.availability_zone));
"availability_zone:%s\r\n", server.availability_zone,
"features:%s\r\n", "cluster_mget"));
/* clang-format on */

/* Conditional properties */
Expand Down

0 comments on commit a63b137

Please sign in to comment.