diff --git a/src/pubsub.c b/src/pubsub.c index 7199be1e0..b9198d263 100644 --- a/src/pubsub.c +++ b/src/pubsub.c @@ -293,7 +293,10 @@ int pubsubUnsubscribeChannel(client *c, robj *channel, int notify, pubsubtype ty retval = 1; /* Remove the client from the channel -> clients list hash table */ if (server.cluster_enabled && type.shard) { - slot = getKeySlot(channel->ptr); + /* Compute the slot from the channel directly instead of using getKeySlot(), + * because the unsubscribe may be triggered by a different client, and + * getKeySlot() would return the cached slot of that client. */ + slot = keyHashSlot(channel->ptr, sdslen(channel->ptr)); } de = kvstoreDictFind(*type.serverPubSubChannels, slot, channel); serverAssertWithInfo(c,NULL,de != NULL); diff --git a/tests/unit/cluster/sharded-pubsub.tcl b/tests/unit/cluster/sharded-pubsub.tcl index 57b550ab7..5f78b7f0f 100644 --- a/tests/unit/cluster/sharded-pubsub.tcl +++ b/tests/unit/cluster/sharded-pubsub.tcl @@ -64,4 +64,31 @@ start_cluster 1 1 {tags {external:skip cluster}} { catch {[$replica EXEC]} err assert_match {EXECABORT*} $err } + + # Regression: shard channel slot must not follow getKeySlot() current_client + # cache when CLIENT KILL runs inside another client's EXEC (pubsubUnsubscribeChannel). + test {Shard pubsub: CLIENT KILL subscriber inside MULTI/EXEC (cross-slot)} { + # SET fixes the transaction client's slot to keyk's slot; the subscriber must + # use a shard channel in a different slot so a wrong-slot lookup would fail. + set keyk "{06S}k" + set channel "{Qi}ch" + assert {[R 0 cluster keyslot $channel] != [R 0 cluster keyslot $keyk]} + + set rd_sub [redis_deferring_client] + $rd_sub client id + set cid [$rd_sub read] + $rd_sub ssubscribe $channel + $rd_sub read + + $primary multi + $primary set $keyk v + $primary client kill id $cid + set got [$primary exec] + + assert_equal {OK 1} $got + assert_equal PONG [$primary ping] + + catch {$rd_sub read} + $rd_sub close + } }