gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Revert "Decline unsubscribe related command in non-subscribed mode" (#1265)

Browse Source 
This PR goal is to revert the changes on PR
https://github.com/valkey-io/valkey/pull/759

Recently, we got some reports that in Valkey 8.0 the PR
https://github.com/valkey-io/valkey/pull/759 (Decline unsubscribe
related command in non-subscribed mode) causes break change.
(https://github.com/valkey-io/valkey/issues/1228)

Although from my thought, call commands "unsubscribeCommand",
"sunsubscribeCommand", "punsubscribeCommand" in request-response mode
make no sense. This is why I created PR
https://github.com/valkey-io/valkey/pull/759

But breaking change is always no good, @valkey-io/core-team How do you
think we revert this PR code changes?

Signed-off-by: hwware <wen.hui.ware@gmail.com>
This commit is contained in:
Wen Hui 2024-11-07 20:05:16 -05:00 committed by GitHub
parent 1c18c80844
commit 3672f9b2c3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 24 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/server.c
View File

@ -4165,12 +4165,6 @@ int processCommand(client *c) {
return C_OK;
}
/* Not allow several UNSUBSCRIBE commands executed under non-pubsub mode */
if (!c->flag.pubsub && (c->cmd->proc == unsubscribeCommand || c->cmd->proc == sunsubscribeCommand ||
c->cmd->proc == punsubscribeCommand)) {
rejectCommandFormat(c, "-NOSUB '%s' command executed not in subscribed mode", c->cmd->fullname);
return C_OK;
}
/* Only allow commands with flag "t", such as INFO, REPLICAOF and so on,
* when replica-serve-stale-data is no and we are a replica with a broken
* link with primary. */

3
tests/unit/info.tcl
View File

@ -424,8 +424,7 @@ start_server {tags {"info" "external:skip"}} {
set info [r info clients]
assert_equal [getInfoProperty $info pubsub_clients] {1}
# non-pubsub clients should not be involved
catch {unsubscribe $rd2 {non-exist-chan}} e
assert_match {*NOSUB*} $e
assert_equal {0} [unsubscribe $rd2 {non-exist-chan}]
set info [r info clients]
assert_equal [getInfoProperty $info pubsub_clients] {1}
# close all clients

25
tests/unit/pubsub.tcl
View File

@ -109,12 +109,9 @@ start_server {tags {"pubsub network"}} {
$rd1 close
}
test "UNSUBSCRIBE and PUNSUBSCRIBE from non-subscribed channels" {
test "UNSUBSCRIBE from non-subscribed channels" {
set rd1 [valkey_deferring_client]
foreach command {unsubscribe punsubscribe} {
catch {$command $rd1 {foo bar quux}} e
assert_match {*NOSUB*} $e
}
assert_equal {0 0 0} [unsubscribe $rd1 {foo bar quux}]
# clean up clients
$rd1 close
}
@ -204,6 +201,14 @@ start_server {tags {"pubsub network"}} {
$rd close
} {0} {resp3}
test "PUNSUBSCRIBE from non-subscribed channels" {
set rd1 [valkey_deferring_client]
assert_equal {0 0 0} [punsubscribe $rd1 {foo.* bar.* quux.*}]
# clean up clients
$rd1 close
}
test "NUMSUB returns numbers, not strings (#1561)" {
r pubsub numsub abc def
} {abc 0 def 0}
@ -241,6 +246,16 @@ start_server {tags {"pubsub network"}} {
$rd1 close
}
test "PUNSUBSCRIBE and UNSUBSCRIBE should always reply" {
# Make sure we are not subscribed to any channel at all.
r punsubscribe
r unsubscribe
# Now check if the commands still reply correctly.
set reply1 [r punsubscribe]
set reply2 [r unsubscribe]
concat $reply1 $reply2
} {punsubscribe {} 0 unsubscribe {} 0}
### Keyspace events notification tests
test "Keyspace notifications: we receive keyspace notifications" {

5
tests/unit/pubsubshard.tcl
View File

@ -74,8 +74,9 @@ start_server {tags {"pubsubshard external:skip"}} {
test "SUNSUBSCRIBE from non-subscribed channels" {
set rd1 [valkey_deferring_client]
catch {sunsubscribe $rd1 {foo}} e
assert_match {*NOSUB*} $e
assert_equal {0} [sunsubscribe $rd1 {foo}]
assert_equal {0} [sunsubscribe $rd1 {bar}]
assert_equal {0} [sunsubscribe $rd1 {quux}]
# clean up clients
$rd1 close