kvrocks
kvrocks copied to clipboard
apache
Support the `hello` command
Search before asking
- [X] I had searched in the issues and found no similar issues.
Motivation
Redis supported the hello command to detect the different protocol and some Redis SDKs will use it to switch a different protocol. So it will be good to support the command to make those SDKs happy.
Solution
Add the hello command and keep the output same with https://redis.io/commands/hello/
Are you willing to submit a PR?
- [ ] I'm willing to submit a PR!
Hi hulk, after reading document and code of redis (mainly https://github.com/redis/redis/blob/unstable/src/networking.c#L3432 ), I found that hello defines the protocol redis use ( RESP2 or RESP3), and return the data via hello's "protocol".
After go through the code of kvrocks ( redis_cmd and server ), I didn't found implementions for RESP3, did I miss it?
@mapleFU You're right that we only implemented RESP2 and have no plan to implement RESP3 recently. The main purpose of this command is to compatible with some Redis clients, which may send the hello command to check the protocol version after connecting to the server.
@mapleFU You're right that we only implemented RESP2 and have no plan to implement RESP3 recently. The main purpose of this command is to compatible with some Redis clients, which may send the
hellocommand to check the protocol version after connecting to the server.
So, user should get response like https://github.com/redis/redis/blob/unstable/src/networking.c#L3439 if they use protocol as 3? What about acl?
@mapleFU You're right that we only implemented RESP2 and have no plan to implement RESP3 recently. The main purpose of this command is to compatible with some Redis clients, which may send the
hellocommand to check the protocol version after connecting to the server.So, user should get response like https://github.com/redis/redis/blob/unstable/src/networking.c#L3439 if they use
protocolas3? What about acl?
Yes, can respond the protocol version is out of range like Redis. For the ACL, we don't support the user/password like Redis 6/7, so I think we can just read ignore the AUTH parameters. That said those clients must use legacy way to auth.
It's interesting that when executing HELLO 3 AUTH default foobar with requirepass foobar Kvrocks returns "NOAUTH Authentication required." instead of ERR unknown command so that some assumption made by go-redis doesn't work.
cc @vmihailenco this can be an incompatible issue with Kvrocks and go-redis, but I suppose it should be resolved by Kvrocks support HELLO or at least returns ERR unknown command on unknown command.
Good catch. I think the right behavior should return ERR unknown command on Kvrock's side, but unfortunately that Kvrocks will check the auth first now.