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

[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.

[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 redis/redis#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.

[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.