go-redis
go-redis copied to clipboard
redis
HELLO command not supported for twemproxy RESP2 protocol
Expected Behavior
When we do client.Ping() command through twemproxy, it should return PONG as success value or error with message. This is just sample of my expectation if succeed without error.
Current Behavior
When we do client.Ping() command through twemproxy, it return error EOF. And in the nutcracker logs, it said the hello command is unsupported yet.
Possible Solution
I checked the code, it's called at here https://github.com/redis/go-redis/blob/master/redis.go#L288-L301 maybe we can add handler for EOF error, but still not sure if this EOF error type can be occured on other case or my case only
Steps to Reproduce
- Run redis on local using default config
- Run twemproxy on local using default config
- Do
redisClient.Ping()using go-redis latest version (mine is using v9.0.5)
Context (Environment)
I want to use go-redis v9 with twemproxy, but it return EOF which is not handled yet by go-redis pkg, so currently i downgrading the pkg version to v8 to avoid this hello command issues
Detailed Description
Still not determined
Possible Implementation
Still not determined
@monkey92t Send Hello Version only if Version is greater than 2? How do you think?
Hello should be acceptable for any redis server after version 6.0. Ideally this would continue to be available to servers even with a value of 2 - and many (others) of us rely on the current behaviour. Making HELLO optional/disableable when a Version is set would be an alternative...
I found that there are some magic logic like ReadTimeout -2 indicates that doesn't set timeout. Maybe we can make protocol -1 mean that doesn't send Hello.
@monkey92t Send Hello Version only if Version is greater than 2? How do you think?
what version do you mean? is it the RESP version? if yes, official docs said Redis version 6 and above supports two protocols: the old protocol, RESP2, and a new one introduced with Redis 6, RESP3.
I found that there are some magic logic like ReadTimeout -2 indicates that doesn't set timeout. Maybe we can make protocol -1 mean that doesn't send Hello.
hmm, if looking the value, is supported only 2 or 3. can we make it 2 instead of -1? https://github.com/redis/go-redis/blob/0bdc7dd8986e4923b3309f880245b9311d09f273/options.go#L48C2-L50
2 is not a good choice. Since redis-server may default RESP version to 3 in the future. That's why @chayim suggest that make HELLO optional.
can we make sure if protocol options is not used at somewhere? to prevent any unexpected error
Hi everybody,
We also have the same problem :( We use twemproxy in production, and we can't upgrade to v9, because of it.
I can appreciate the change - but I'm a bit torn. TWEMPROXY can continue to use V8 - and we could push V9+ to be Redis 6+. Given the redis release cycle, (6.0, 6.2, 7.0, and soon 7.2rc*) have been release since the last 5.0 release in 2021 - my gut says this is the right decision.
I can appreciate the change - but I'm a bit torn. TWEMPROXY can continue to use V8 - and we could push V9+ to be Redis 6+. Given the redis release cycle, (6.0, 6.2, 7.0, and soon 7.2rc*) have been release since the last 5.0 release in 2021 - my gut says this is the right decision.
Even though 5.0 was released a long time ago, there are still a large number of services that have not been upgraded to 6.0 for various reasons, and I think it's important to allow users of these services to still be able to connect to the server using the latest version of the go-redis library. Using special logic for compatibility would at least make it easier for users to connect to older versions of the server by modifying the configuration, rather than having to fall back to an old version of the library and recompile the binary.
redis 5 is rather old, and doesn't receive security updates for CVEs at this time. Newer versions do. I believe that we should neither invest in, nor ongoingly support these changes. @uglide @ofekshenawa thoughts?
I'm with @chayim on this one. Twemproxy is abandonware. There are many variable and properly supported options to replace Twemproxy in 2023. For existing legacy projects, Go-Redis V8 can be used.
Hello guys, eventually I remove the twemproxy to avoid this issue.
However, if you wanna ask why i'm not still using V8 for legacy or current project (that use twemproxy) because when i use another pkg that has redis required lib, sometimes it use latest version (V9 above). So my choice is only:
- fix this HELLO command issue for twemproxy RESP2 protocol, or
- Don't use twemproxy at all
both choices will allow me to upgrade the version to V9😁 feel free to close this issue since mine is solved