[BUG]SUBSCRIBE command send one request but reply with multiply replies

[hwware]

Describe the bug

When I revisit the issue from redis https://github.com/redis/redis/issues/11046, the problem is fixed in pr https://github.com/redis/redis/pull/11047 from redis-cli side. But after reading them carefully, I think it still belongs to the server side issue. Indeed, there are 2 bugs here.

To reproduce

bug 1. Under the non-subpub mode, UNSUBSCRIBE, PUNSUBSCRIBE and SUNSUBSCRIBE should report an error message. bug 2. When clients run the SUBSCRIBE and this kinds of commands, the clients send one request but they receive multiply response from server sides

Expected behavior

bug 1. We fix the problem in PR https://github.com/valkey-io/valkey/pull/759 bug 2. Because this is a big break changes, we have 2 options at least: 1. add one more parameter and recommend clients using the latest update feature, 2. Add a new command and deprecate all existing commands

Additional information

If we fix in the server side, maybe this is one break change.

One candidate solution as below:

[hwware]

@enjoy-binbin @zuiderkwast

[zuiderkwast]

valkey-cli thinks that it is in "subscribed mode" even after unsubscribing to all channels. It is not the same as the pubsub client flag on the server. Valkey-cli is not aware of the client flag. In valkey-cli, you need to press Ctrl+C to exit subscribed mode and when you do that, valkey-cli reconnects.

If we start changing the behaviour depending on the cliet flag on the server side, this is unexpected to clients, because they are usually not aware of the client flags.

Why do you want it to return an error? Isn't it better that UNSUBSCRIBE behaves in the same way always?

I think the real problem is actually that SUBSCRIBE and UNSUBSCRIBE can return more than one reply. This is confusing for clients, even valkey-cli before it was fixed. Client's can't just pass a command to the server without checking which command they are sending. They need to treat [P|S][UN]SUBSCRIBE specially.

[hpatro]

I think this is the same issue which I had raised earlier in redis https://github.com/redis/redis/issues/12592. @zuiderkwast and I discussed a solution over there. I think it's easy to maintain the subscription information and take action on it in the cli code.

[zuiderkwast]

@hpatro that would be even better. We can't just rely on UNSUBSCRIBE returning zero in the last field, because there may still be shard-channel subscriptions.

@hwware created a PR to print a message but it doesn't seem to be linked. I think we can do both.

[kamyuentse]

I think the point is: the client only send ONE request with multiple channels to server, while the server reply MULTIPLE replies.

[zuiderkwast]

@kamyuentse Yes, but this is what SUBSCRIBE ch1 ch2 does too. It returns one response per channel. But that's only in subscribed mode?

In topics/protocol.md we can read this:

When a RESP2 connection subscribes to a Pub/Sub channel, the protocol changes semantics and becomes a push protocol. The client no longer requires sending commands because the server will automatically send new messages to the client (for the channels the client is subscribed to) as soon as they are received.

... and in topics/pubsub.md:

In subscribe, unsubscribe, psubscribe and punsubscribe message types, the last argument is the count of subscriptions still active. This number is the total number of channels and patterns the client is still subscribed to. So the client will exit the Pub/Sub state only when this count drops to zero as a result of unsubscribing from all the channels and patterns.

(The above needs to be edited to include sharded-pubsub channels too.)

I think this is what we can consider a bug: If the client is not subscribed to any channel, it should be a request-response protocol with one response for every request.