redis-plus-plus
redis-plus-plus copied to clipboard
sewenew
Avoid resetting the connection when calling Pipeline::discard()
Hi @sewenew,
This PipelineImpl::discard() function has a name that does not really describe what it does:
https://github.com/sewenew/redis-plus-plus/blob/66d7c31a04e3d189ac36b362b1dc9d9197e2bdbb/src/sw/redis%2B%2B/pipeline.h#L39-L42
Would it not be a good idea to deprecate and replace it with something like PipelineImpl::reconnect()?
Maybe PipelineImpl::discard() should also do nothing, unless the QueuedRedis::_new_connection flag is set, but I have not looked at the consequences of doing this.
Lastly, I saw that Redis has a RESET command that, among other things, does:
Discards the current MULTI transaction block, if one exists.
RESET does a lot of other stuff that's probably not desired, but it doesn't drop the connection.
How about just calling RESET instead of reconnecting?
Thanks & regards
Instead of rename the method, we should reimplement the method.
It’s a bad implement the discard method by reconnecting the connection, although it’s the simplest way to do that.
RESET seems to be a new command, and I’ll do some research on it. Thanks for the info.
I’ll try to find a better way, and sorry for the confusing code...
Regards
Hi @sewenew No problem. I won't mind if you close this issue in order to reduce the issue-clutter. You could just add this to the TODO LIST (#74) issue. BTW, would it be possible for you to contact me via my email, please? Regards
The TODO list is more or less like a feature list, while this issue is about some optimization. I'll leave this issue open.
Regards
By the way, I think we've already contacted with email, and I've replied your email (gmail) at 29 Jan.
Not sure if the reply has reached you. Anyway, I'll send your another one. Keep in touch!
Hi @sewenew
RESET seems to be a new command, and I’ll do some research on it. Thanks for the info.
You are right. According to the Redis release notes, it was added in rel 6.2 RC1, in Dec 2020.
Anyhow, I think this issue's name is not relevant any more. Should we not change it to something like 'Avoid resetting the connection when calling Pipeline::discard()'?
I am personally not using the Pipeline::discard() method, as it resets the connection at the moment.
Regards
@wingunder I agree with you. I've changed title, and thanks for the suggestion :)
Regards