From d521a6519f0d5b0560182cda3e400b2e70963cb9 Mon Sep 17 00:00:00 2001 From: Nikhil Manglore Date: Wed, 27 Nov 2024 22:30:58 +0000 Subject: [PATCH 1/3] Fixed CROSSSLOT ERROR in Cluster Mode for PUBSUB SHARDNUMSUB Signed-off-by: Nikhil Manglore --- src/commands/pubsub-shardnumsub.json | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/commands/pubsub-shardnumsub.json b/src/commands/pubsub-shardnumsub.json index 8eeca4e31b..75f7742fc0 100644 --- a/src/commands/pubsub-shardnumsub.json +++ b/src/commands/pubsub-shardnumsub.json @@ -20,6 +20,25 @@ "multiple": true } ], + "key_specs": [ + { + "flags": [ + "NOT_KEY" + ], + "begin_search": { + "index": { + "pos": 1 + } + }, + "find_keys": { + "range": { + "lastkey": -1, + "step": 1, + "limit": 0 + } + } + } + ], "reply_schema": { "description": "The number of subscribers per shard channel, each even element (including 0th) is channel name, each odd element is the number of subscribers.", "type": "array" From 2c3c08e3af6dac2625e6b7037d80dd6dceae8836 Mon Sep 17 00:00:00 2001 From: Nikhil Manglore Date: Mon, 2 Dec 2024 18:18:36 +0000 Subject: [PATCH 2/3] Updated commands.def for issue #560 (CROSSSLOT ERROR) Signed-off-by: Nikhil Manglore --- src/commands.def | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/commands.def b/src/commands.def index ecc77126af..b15205aced 100644 --- a/src/commands.def +++ b/src/commands.def @@ -4704,7 +4704,9 @@ struct COMMAND_ARG PUBSUB_SHARDCHANNELS_Args[] = { #ifndef SKIP_CMD_KEY_SPECS_TABLE /* PUBSUB SHARDNUMSUB key specs */ -#define PUBSUB_SHARDNUMSUB_Keyspecs NULL +keySpec PUBSUB_SHARDNUMSUB_Keyspecs[1] = { +{NULL,CMD_KEY_NOT_KEY,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={-1,1,0}} +}; #endif /* PUBSUB SHARDNUMSUB argument table */ @@ -4719,7 +4721,7 @@ struct COMMAND_STRUCT PUBSUB_Subcommands[] = { {MAKE_CMD("numpat","Returns a count of unique pattern subscriptions.","O(1)","2.8.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBSUB_NUMPAT_History,0,PUBSUB_NUMPAT_Tips,0,pubsubCommand,2,CMD_PUBSUB|CMD_LOADING|CMD_STALE,0,PUBSUB_NUMPAT_Keyspecs,0,NULL,0)}, {MAKE_CMD("numsub","Returns a count of subscribers to channels.","O(N) for the NUMSUB subcommand, where N is the number of requested channels","2.8.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBSUB_NUMSUB_History,0,PUBSUB_NUMSUB_Tips,0,pubsubCommand,-2,CMD_PUBSUB|CMD_LOADING|CMD_STALE,0,PUBSUB_NUMSUB_Keyspecs,0,NULL,1),.args=PUBSUB_NUMSUB_Args}, {MAKE_CMD("shardchannels","Returns the active shard channels.","O(N) where N is the number of active shard channels, and assuming constant time pattern matching (relatively short shard channels).","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBSUB_SHARDCHANNELS_History,0,PUBSUB_SHARDCHANNELS_Tips,0,pubsubCommand,-2,CMD_PUBSUB|CMD_LOADING|CMD_STALE,0,PUBSUB_SHARDCHANNELS_Keyspecs,0,NULL,1),.args=PUBSUB_SHARDCHANNELS_Args}, -{MAKE_CMD("shardnumsub","Returns the count of subscribers of shard channels.","O(N) for the SHARDNUMSUB subcommand, where N is the number of requested shard channels","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBSUB_SHARDNUMSUB_History,0,PUBSUB_SHARDNUMSUB_Tips,0,pubsubCommand,-2,CMD_PUBSUB|CMD_LOADING|CMD_STALE,0,PUBSUB_SHARDNUMSUB_Keyspecs,0,NULL,1),.args=PUBSUB_SHARDNUMSUB_Args}, +{MAKE_CMD("shardnumsub","Returns the count of subscribers of shard channels.","O(N) for the SHARDNUMSUB subcommand, where N is the number of requested shard channels","7.0.0",CMD_DOC_NONE,NULL,NULL,"pubsub",COMMAND_GROUP_PUBSUB,PUBSUB_SHARDNUMSUB_History,0,PUBSUB_SHARDNUMSUB_Tips,0,pubsubCommand,-2,CMD_PUBSUB|CMD_LOADING|CMD_STALE,0,PUBSUB_SHARDNUMSUB_Keyspecs,1,NULL,1),.args=PUBSUB_SHARDNUMSUB_Args}, {0} }; From 90dc6f301918223c2b8b554860d0ef52ac1edc3f Mon Sep 17 00:00:00 2001 From: Nikhil Manglore Date: Tue, 3 Dec 2024 00:58:19 +0000 Subject: [PATCH 3/3] Fixed keyspecs starting position and added testcases Signed-off-by: Nikhil Manglore --- src/commands.def | 2 +- src/commands/pubsub-shardnumsub.json | 2 +- tests/unit/cluster/sharded-pubsub.tcl | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/commands.def b/src/commands.def index b15205aced..c352a85c6f 100644 --- a/src/commands.def +++ b/src/commands.def @@ -4705,7 +4705,7 @@ struct COMMAND_ARG PUBSUB_SHARDCHANNELS_Args[] = { #ifndef SKIP_CMD_KEY_SPECS_TABLE /* PUBSUB SHARDNUMSUB key specs */ keySpec PUBSUB_SHARDNUMSUB_Keyspecs[1] = { -{NULL,CMD_KEY_NOT_KEY,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={-1,1,0}} +{NULL,CMD_KEY_NOT_KEY,KSPEC_BS_INDEX,.bs.index={2},KSPEC_FK_RANGE,.fk.range={-1,1,0}} }; #endif diff --git a/src/commands/pubsub-shardnumsub.json b/src/commands/pubsub-shardnumsub.json index 75f7742fc0..1fc3ac13eb 100644 --- a/src/commands/pubsub-shardnumsub.json +++ b/src/commands/pubsub-shardnumsub.json @@ -27,7 +27,7 @@ ], "begin_search": { "index": { - "pos": 1 + "pos": 2 } }, "find_keys": { diff --git a/tests/unit/cluster/sharded-pubsub.tcl b/tests/unit/cluster/sharded-pubsub.tcl index b5b19ff481..e3a4f900f5 100644 --- a/tests/unit/cluster/sharded-pubsub.tcl +++ b/tests/unit/cluster/sharded-pubsub.tcl @@ -53,4 +53,23 @@ start_cluster 1 1 {tags {external:skip cluster}} { catch {[$replica EXEC]} err assert_match {EXECABORT*} $err } + + test "SHARDNUMSUB for CROSSSLOT Error in cluster mode" { + set rd1 [valkey_deferring_client] + set rd2 [valkey_deferring_client] + set rd3 [valkey_deferring_client] + + assert_equal {1} [ssubscribe $rd1 chan1] + assert_equal {1} [ssubscribe $rd2 chan2] + assert_error "CROSSSLOT Keys in request don't hash to the same slot" {r pubsub shardnumsub chan1 chan2} + + assert_equal {1} [ssubscribe $rd3 {"{chan1}2"}] + assert_equal {chan1 1 {{chan1}2} 1} [r pubsub shardnumsub "chan1" "{chan1}2"] + + # clean up clients + $rd1 close + $rd2 close + $rd3 close + } + } \ No newline at end of file