Support the hello command

[git-hulk]

Search before asking

• 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!

[mapleFU]

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?

[git-hulk]

@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]

@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.

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?

[git-hulk]

@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.

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?

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.

[tisonkun]

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.

[git-hulk]

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.

[tisonkun]

@git-hulk I change the order in #830 and all tests look good.

[git-hulk]

@git-hulk I change the order in #830 and all tests look good.

Thanks!