Add command "Client Capa subv2" to change behavior for SUBSCRIBE and SSUBSCRIBE commands: one call with only one response

[hwware]

Now for the command SUBSCRIBE and SSUBSCRIBE, if any client call with multiply channels, multiply responses come back.

It does not match with the Valkey command response logic, it causes client side to parse the response with extra effort on them. The problem exists long time.

Thus, to fix this design error, I add a new command parameter: client capa subv2
The command "client capa" is imported in Valkey 8, and I add a new parameter "subv2" for it
Note: subv2 is just a temporarily name, I have no idea which name is better, welcome any suggestion.

The update behavior as below: one call with one response

Note: for existing users, I think this is a break change. If any client chooses the behavior that one call with one response, the command "client capa subv2" can be called first.

[codecov]

Codecov Report

Attention: Patch coverage is 96.96970% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.60%. Comparing base (983bb51) to head (c97985f).

Files with missing lines	Patch %	Lines
src/pubsub.c	95.65%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #962      +/-   ##
============================================
- Coverage     70.62%   70.60%   -0.02%     
============================================
  Files           114      114              
  Lines         61695    61718      +23     
============================================
+ Hits          43571    43579       +8     
- Misses        18124    18139      +15     

Files with missing lines	Coverage Δ
src/networking.c	88.43% <100.00%> (+<0.01%)	⬆️
src/server.h	100.00% <ø> (ø)
src/valkey-cli.c	55.57% <100.00%> (+0.05%)	⬆️
src/pubsub.c	96.23% <95.65%> (-0.14%)	⬇️

... and 8 files with indirect coverage changes

[zuiderkwast]

This was a design error from the beginning, and another design error when RESP3 was introduced, but it is too late to change it now.

This is a serious breaking change that will break all clients that support pubsub. This is very dangerous to do, considering there are many redis-compatible services out there and clients try to be compatible with not only valkey.

[zuiderkwast]

So with RESP3, this is still no normal in-band replies and one push message (instead of one per channel). This is still the main problem of [UN][P|S]SUBSCRIBE IMO.

To fix this behaviour, SUBSCRIBE should return an in-band reply. Either the list of channels as an array or map-reply, or just +OK and send the channels as push.

[hwware]

I do not want to import this change in Valkey 8.0. I wish we could do this change on Valkey 8.X or 9 version. Error need to be correct, never late ^_^

[zuiderkwast]

I think we cannot fix it ever. Only maybe with a CLIENT CAPA or RESP4.

[madolson]

Yeah, I agree with this being a potentially significant breaking change. I think maybe CLIENT CAPA could fix it (or RESP4).

[hwware]

I update the design, add a new parameter for client capa command, but the parameter name is just temporarily. If you have better name, please let me know or we can discuss it in the meeting

[zuiderkwast]

@hwware What exactly is the problem with the old behaviour according to you?

According to the docs, when a client enters subscribed mode, the protocol changes semantics from a request-response protocol to a push protocol. The commands and the responses are no longer related to each other. This was for RESP2 and many commands are forbidden in subscribed mode to make sure the client can understand the responses. It is a limited number of possible responses. The client can receive pubsub messages and notifications that a channel is subscribed or unsubscribed, asynchronously, but not exactly as a reply to each command.

In RESP3, it is different, because here clients can call any command in subscribed mode. The reason for allowing this was that RESP3 has a push (out-of-band) type. The client just needs to know the very strange fact that SUBSCRIBE and UNSUBSCRIBE receive no in-band reply at all.

Now, there are two things that make this more complicated:

1. [S|P][UN]SUBSCRIBE can return an error. An error is in-band. An error can happen due to ACL rules and other reasons. For sharded-pubsub, it can even be a MOVED redirect or a CLUSTERDOWN error (slot not served). To check for errors, a client needs to wait for at least one 1) "subscribe" 2) "ch" 3) 1 out-of-band message to be sure there was no error.
2. For sharded-pubsub, when a slot is migrated, the subscribers on the migrating node are automatically unsubscribed. They receive a message like 1) "sunsubscribe" 2) "ch" 3) 0 which is not matching any command the client has sent. It is just a spontaneous message.

Most of this is possible for clients to handle, even if it is very complex to handle error replies in subscribed mode, but one problem is not possible to solve. This one: #1066.

I have another idea that can solve the issue of matching request-response pairs:

CLIENT CAPA subscribe-ok: Makes the [S|P][UN]SUBSCRIBE return +OK on success and an error otherwise, so there is always exactly one in-band reply to each command. The notifications for each channel are received out-of-band as before.

[hwware]

@hwware What exactly is the problem with the old behaviour according to you?

According to the docs, when a client enters subscribed mode, the protocol changes semantics from a request-response protocol to a push protocol. The commands and the responses are no longer related to each other. This was for RESP2 and many commands are forbidden in subscribed mode to make sure the client can understand the responses. It is a limited number of possible responses. The client can receive pubsub messages and notifications that a channel is subscribed or unsubscribed, asynchronously, but not exactly as a reply to each command.

In RESP3, it is different, because here clients can call any command in subscribed mode. The reason for allowing this was that RESP3 has a push (out-of-band) type. The client just needs to know the very strange fact that SUBSCRIBE and UNSUBSCRIBE receive no in-band reply at all.

Now, there are two things that make this more complicated:

1. [S|P][UN]SUBSCRIBE can return an error. An error is in-band. An error can happen due to ACL rules and other reasons. For sharded-pubsub, it can even be a MOVED redirect or a CLUSTERDOWN error (slot not served). To check for errors, a client needs to wait for at least one 1) "subscribe" 2) "ch" 3) 1 out-of-band message to be sure there was no error.
2. For sharded-pubsub, when a slot is migrated, the subscribers on the migrating node are automatically unsubscribed. They receive a message like 1) "sunsubscribe" 2) "ch" 3) 0 which is not matching any command the client has sent. It is just a spontaneous message.

Most of this is possible for clients to handle, even if it is very complex to handle error replies in subscribed mode, but one problem is not possible to solve. This one: #1066.

I have another idea that can solve the issue of matching request-response pairs:

CLIENT CAPA subscribe-ok: Makes the [S|P][UN]SUBSCRIBE return +OK on success and an error otherwise, so there is always exactly one in-band reply to each command. The notifications for each channel are received out-of-band as before.

Hi @zuiderkwast As what you said, the current behavior is that the protocol changes semantics from a request-response protocol to a push protocol if the client enters subscribed mode. The commands and the responses are no longer related to each other. Thus, the client could receive multiply responses from one command. It results the client side (such as Java side, or Javascript etc) need to check which mode or protocol the server current supports now. It causes the client side programming more complex.

Thus, in my PR, I think we should unify the request-response protocol and push protocol response way , make the response always matches with the command (1:1). We should try our best to decrease the client programming complexity. (Or by my pr, give the client one option)

[zuiderkwast]

Thus, in my PR, I think we should unify the request-response protocol and push protocol response way , make the response always matches with the command (1:1). We should try our best to decrease the client programming complexity. (Or by my pr, give the client one option)

OK, but there are responses that can't be matched with a command, the messages sent by PUBLISH:

1) "message"
2) "ch"
3) "hello"

(And additionally the spontaneous sunsubscribe message which is sent by the server when a slot is deleted or migrated.)

That's why in RESP2 can never be 1:1 request-response protocol in pubsub. We need an out-of-band message for this, which we have in RESP3.

If we just use it correctly, pubsub can be a 1:1 where only in-band replies are matched with a command and push messages are out-of-band. Clients that want better semantics can use RESP3.

[zuiderkwast]

CLIENT CAPA is not enough, because the client needs to know if the server supports the feature too. The client can check the response of HELLO to see the Valkey version. Can the client trust the version number to infer the subscribe behaviour?

Alternatives:

• Use a command to enable this behaviour, such as CLIENT SUBSCRIBE-OK ON. If the server doesn't support this command, the client will receive an error reply. Similar to CLIENT TRACKING ON, CLIENT NO-TOUCH ON and CLIENT NO-EVICT ON.
• Add new commands for subscribing and unsubscribing, such as SSUBSCRIBE2, PUNSUBSCRIBE2, etc. that return OK or ERR. It is many new commands though.