I was thinking that redis needs a RESET command that would make an existing client connection forget all the state it keeps with regards to transactions, watches, pubsub channels (and everything else that might be stateful in the future) and always returns always successfully OK or RESET (the latter might be easier to look for without parsing the sent commands).
This can be very useful in the context of a client library which uses connection pooling.
If you want to reuse an existing socket connection to the server in the pool, you must be sure that the connection is in an empty state before adding it back to the pool, since the user might issue commands that are not-safe between different contexts.
Doing it manually requires quite a bit of parsing and book keeping with regards to which commands were sent and what their side effects are (and if they executed ok).
Comment From: yatsukhnenko
It's probably should be CLIENT RESET. We also think than this functionality would be useful.
Comment From: tzickel
@antirez Would be nice for Redis 6 GA :)
Comment From: tzickel
Guess it wasn't added :(
A workaround I've found is that given that today the statefull commands are transactions, and pubsub and monitor, is that if you split them into 2 types (push and non-push). The non-push can just call DISCARD always in the end before returning to the pool (ignoring the result), and push can just be closed and not returned to the pull (this connections usually are not high frequency anyhow).
Comment From: yossigo
Given this some thought. It makes sense for most of the state -- watched keys, Pub/Sub subscriptions, ACL user, client name, etc.
I do have a big concern about breaking/complicating MULTI semantics though. So, a client would have to issue a DISCARD (which may fail) followed by CLIENT RESET to be sure it's not stuck in a transaction. Might not be so favorable to clients but definitely maintains better backwards compatibility and protocol clarity.
@tzickel How does that sound to you?
Comment From: tzickel
Well, today Redis is even more complex than what it was when I opened the issue a few years ago (ACL + RESP3 + client side caching, maybe other stuff as well).
In my opinion when thinking about taking a connection from connection pool, some stuff are set in the connection phase (ACL user + client name), and some are not (multi, pubsub, watch, etc...) but if someone has another idea, maybe there should be a per client configuration on what to reset when RESET is called...
Which semantics are problematic for having the RESET command cancel a MULTI command without DISCARD ? MULTI is a very big state change, and if RESET is documented as resetting state, I think it's expected that it will be aborted.
My concept is can a low level library talking to a Redis server that has connection pooling, can be sure that no matter if a higher up (a higher level abstraction library or the user running commands directly to the server via a connection from the pool) does something wonkey, if a connection is returned to the pool, than the next time it's taken it won't do something unexpected (such as reading result of a previous pub/sub command on the connection, which is quite hard to do at the low level, if you don't follow exactly which commands are sent and their expected outcome, I can elaborate if needed).
Idea from now (to make processing faster): Because a low level library should not be dealing with all possible state changes (thus future proofing it to future state changing features), an better implementation for example can be RESET [SOME ID MADE UP BY THE LIBRARY] that is sent when the connection is put back to the pool. Then the return value of the RESET command will be that ID as a string (or OK if no ID supplied)? So this way, when returning to the pool, it doesn't need to waste time reading the response, but when the connection is picked up from the pool the next time, the library will first read responses till it sees the ID, then it will know the connection is free.
As per confusion, I don't think that a user should be using this command at all unless he knows what he is doing, this is mainly for low level libraries (or higher level libraries who want an easier life).
Comment From: yossigo
The semantics problem is that by definition you don't exit a MULTI state unless you EXEC or DISCARD. If we expect CLIENT RESET to also cancel this state we're potentially breaking lots of things around Redis.
You're right that Redis has gotten more complex and there are more things to potentially reset. If clients need to be selective about what they reset (e.g. name, ACL user, RESP version, etc.) then perhaps this is an indication that a global reset is not as useful as it seems.
I think saving a "state snapshot" per client creates too much complexity. Does this mean we have to store the client name in case it was changed? All subscribed channels? And there's lots more of course.
Comment From: tzickel
The RESET command is just for taking a connection from a connection pool, to be in a state where new commands running won't be related to commands that were on that connection before it was returned to the pool.
WATCH / MULTI of-course are offenders here, but you can just call DISCARD to be sure they will be reset.
PUB/SUB commands are bigger offenders because there is no quick command to make sure that the connection has finished processing all PUB/SUB messages. (but maybe it's less of an issue, because before RESP3 you could easily know when a connection is in push only mode, and just never return it to the pool, because opening such connections happens less frequently).
ACL/AUTH is not related as that is something that is usually done at the connection initialization. (I don't think it's a valid workflow to change users after the fact, especially in a connection pool environment where you don't know which pool you'll get, see DATABASE below).
CLIENT NAME is again something you do in connection initialization (why change by a user when you know you are backed by a pool anyhow), and does not effect the commands themselves anyhow, so it's not really an issue.
DATABASE most clients disallow changing the database # after connection initialization for sanity reasons. Instead they expect you to make a new pool for each database #. I think Redis makes it harder to enforce when LUA scripts can change it without the client knowing.
I'm not sure if there are other commands which can be offenders in Redis today in a similar way. If not then maybe the solution of the low level Redis library just knowing if the connection is a push connection (and never return it to the pool) or regular one (and just call DISCARD optimistically) might be sufficient (I think when I've opened the issue I had an idea to reclaim the PUBSUB connections as well, but then thought that maybe they aren't being opened / closed with a high frequency).
Comment From: oranagra
for what it worth, i'm in favor of a RESET command that will bring the client to a similar state as disconnection and re-connection (reducing the need for a new socket). in which case the client will be responsible of re-AUTH, re-CLIENT SETNAME, and everything else from scratch, but will not have to worry about any leftovers like MULTI state, and other unexpected flags.
Comment From: yossigo
@oranagra Please see my note about MULTI above. I really don't think we want CLIENT RESET to exit MULTI state.
Comment From: oranagra
@yossigo so you're ok if a client issues DISCARD and CLIENT RESET together, right?
what about a single CLIENT RESET WITH_DISCRAD or CLIENT RESET ALL?
considering that we document that CLIENT RESET discards MULTI, i don't see why the above are different.
i.e. if we document that CLIENT RESET is like disconnecting the socket and reconnecting, the state after it is clear, and i don't see why a DISCARD will be needed.
Comment From: yossigo
@oranagra My problem is just with the complexity involved. Currently the definition is very simple and we'll be introducing additional complexity (i.e. MULTI can now be terminated by a CLIENT command but only with a specific subcommand).
Comment From: yatsukhnenko
@yossigo it may be RESET command as @tzickel proposed and not CLIENT RESET. Also you may add logic for reset connection to DISCARD command and document it.