This is an automated email from the ASF dual-hosted git repository.
hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 86d34ac7 Fix missing fields in HELLO command response (#2150)
86d34ac7 is described below
commit 86d34ac766d2765223594cdb11f03b5eef33345e
Author: hulk <[email protected]>
AuthorDate: Mon Mar 11 13:54:57 2024 +0800
Fix missing fields in HELLO command response (#2150)
Currently, we're missing the 'version', 'role', 'modules' in the HELLO
command which may cause compatibility issue in some clients like Java lettuce:
```
Caused by: java.lang.IllegalArgumentException: Version must not be null
at
io.lettuce.core.internal.LettuceAssert.notNull(LettuceAssert.java:71)
at
io.lettuce.core.RedisHandshake$RedisVersion.<init>(RedisHandshake.java:333)
at
io.lettuce.core.RedisHandshake$RedisVersion.of(RedisHandshake.java:362)
at
io.lettuce.core.RedisHandshake.applyPostHandshake(RedisHandshake.java:249)
at
io.lettuce.core.RedisHandshake.lambda$initialize$0(RedisHandshake.java:99)
```
For the HELLO command response refer: https://redis.io/commands/hello/
---
src/commands/cmd_server.cc | 8 ++++++++
src/server/server.cc | 2 --
src/server/server.h | 2 ++
tests/gocase/unit/hello/hello_test.go | 28 ++++++++++++++++------------
tests/gocase/unit/protocol/protocol_test.go | 18 ++++++++++++++++--
5 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index 0463cf8f..a1e878bd 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -800,6 +800,9 @@ class CommandHello final : public Commander {
std::vector<std::string> output_list;
output_list.push_back(redis::BulkString("server"));
output_list.push_back(redis::BulkString("redis"));
+ output_list.push_back(redis::BulkString("version"));
+ // What the client want is the Redis compatible version instead of the
Kvrocks version.
+ output_list.push_back(redis::BulkString(REDIS_VERSION));
output_list.push_back(redis::BulkString("proto"));
if (srv->GetConfig()->resp3_enabled) {
output_list.push_back(redis::Integer(protocol));
@@ -815,6 +818,11 @@ class CommandHello final : public Commander {
} else {
output_list.push_back(redis::BulkString("standalone"));
}
+ output_list.push_back(redis::BulkString("role"));
+ output_list.push_back(redis::BulkString(srv->IsSlave() ? "slave" :
"master"));
+ // For Kvrocks, the modules is not supported.
+ output_list.push_back(redis::BulkString("modules"));
+ output_list.push_back(conn->NilArray());
*output = conn->HeaderOfMap(output_list.size() / 2);
for (const auto &item : output_list) {
*output += item;
diff --git a/src/server/server.cc b/src/server/server.cc
index cd6b4acd..ca0b2acd 100644
--- a/src/server/server.cc
+++ b/src/server/server.cc
@@ -51,8 +51,6 @@
#include "version.h"
#include "worker.h"
-constexpr const char *REDIS_VERSION = "4.0.0";
-
Server::Server(engine::Storage *storage, Config *config)
: storage(storage), start_time_(util::GetTimeStamp()), config_(config),
namespace_(storage) {
// init commands stats here to prevent concurrent insert, and cause core
diff --git a/src/server/server.h b/src/server/server.h
index 4f8fe314..c4deddfb 100644
--- a/src/server/server.h
+++ b/src/server/server.h
@@ -53,6 +53,8 @@
#include "tls_util.h"
#include "worker.h"
+constexpr const char *REDIS_VERSION = "4.0.0";
+
struct DBScanInfo {
time_t last_scan_time = 0;
KeyNumStats key_num_stats;
diff --git a/tests/gocase/unit/hello/hello_test.go
b/tests/gocase/unit/hello/hello_test.go
index 36296b05..ab3d417b 100644
--- a/tests/gocase/unit/hello/hello_test.go
+++ b/tests/gocase/unit/hello/hello_test.go
@@ -44,15 +44,19 @@ func TestHello(t *testing.T) {
t.Run("hello with protocol 2", func(t *testing.T) {
r := rdb.Do(ctx, "HELLO", "2")
rList := r.Val().([]interface{})
- require.EqualValues(t, rList[2], "proto")
- require.EqualValues(t, rList[3], 2)
+ require.EqualValues(t, "version", rList[2])
+ require.EqualValues(t, "4.0.0", rList[3])
+ require.EqualValues(t, "proto", rList[4])
+ require.EqualValues(t, 2, rList[5])
})
t.Run("hello with protocol 3", func(t *testing.T) {
r := rdb.Do(ctx, "HELLO", "3")
rList := r.Val().([]interface{})
- require.EqualValues(t, rList[2], "proto")
- require.EqualValues(t, rList[3], 2)
+ require.EqualValues(t, "version", rList[2])
+ require.EqualValues(t, "4.0.0", rList[3])
+ require.EqualValues(t, "proto", rList[4])
+ require.EqualValues(t, 2, rList[5])
})
t.Run("hello with wrong protocol", func(t *testing.T) {
@@ -68,8 +72,8 @@ func TestHello(t *testing.T) {
t.Run("hello with non protocol", func(t *testing.T) {
r := rdb.Do(ctx, "HELLO", "2", "SETNAME", "kvrocks")
rList := r.Val().([]interface{})
- require.EqualValues(t, rList[2], "proto")
- require.EqualValues(t, rList[3], 2)
+ require.EqualValues(t, "proto", rList[4])
+ require.EqualValues(t, 2, rList[5])
r = rdb.Do(ctx, "CLIENT", "GETNAME")
require.EqualValues(t, r.Val(), "kvrocks")
@@ -89,8 +93,8 @@ func TestEnableRESP3(t *testing.T) {
r, err := rdb.Do(ctx, "HELLO", "2").Result()
require.NoError(t, err)
rList := r.([]interface{})
- require.EqualValues(t, rList[2], "proto")
- require.EqualValues(t, rList[3], 2)
+ require.EqualValues(t, "proto", rList[4])
+ require.EqualValues(t, 2, rList[5])
r, err = rdb.Do(ctx, "HELLO", "3").Result()
require.NoError(t, err)
@@ -141,8 +145,8 @@ func TestHelloWithAuth(t *testing.T) {
t.Run("hello with non protocol", func(t *testing.T) {
r := rdb.Do(ctx, "HELLO", "2", "AUTH", "foobar", "SETNAME",
"kvrocks")
rList := r.Val().([]interface{})
- require.EqualValues(t, rList[2], "proto")
- require.EqualValues(t, rList[3], 2)
+ require.EqualValues(t, "proto", rList[4])
+ require.EqualValues(t, 2, rList[5])
r = rdb.Do(ctx, "CLIENT", "GETNAME")
require.EqualValues(t, r.Val(), "kvrocks")
@@ -151,8 +155,8 @@ func TestHelloWithAuth(t *testing.T) {
t.Run("hello with non protocol", func(t *testing.T) {
r := rdb.Do(ctx, "HELLO", "2", "AUTH", "default", "foobar",
"SETNAME", "kvrocks")
rList := r.Val().([]interface{})
- require.EqualValues(t, rList[2], "proto")
- require.EqualValues(t, rList[3], 2)
+ require.EqualValues(t, "proto", rList[4])
+ require.EqualValues(t, 2, rList[5])
r = rdb.Do(ctx, "CLIENT", "GETNAME")
require.EqualValues(t, r.Val(), "kvrocks")
diff --git a/tests/gocase/unit/protocol/protocol_test.go
b/tests/gocase/unit/protocol/protocol_test.go
index e4f22327..9becba5e 100644
--- a/tests/gocase/unit/protocol/protocol_test.go
+++ b/tests/gocase/unit/protocol/protocol_test.go
@@ -239,7 +239,14 @@ func TestProtocolRESP3(t *testing.T) {
t.Run("debug protocol string", func(t *testing.T) {
require.NoError(t, c.WriteArgs("HELLO", "3"))
- values := []string{"%3", "$6", "server", "$5", "redis", "$5",
"proto", ":3", "$4", "mode", "$10", "standalone"}
+ values := []string{"%6",
+ "$6", "server", "$5", "redis",
+ "$7", "version", "$5", "4.0.0",
+ "$5", "proto", ":3",
+ "$4", "mode", "$10", "standalone",
+ "$4", "role", "$6", "master",
+ "$7", "modules", "_",
+ }
for _, line := range values {
c.MustRead(t, line)
}
@@ -289,7 +296,14 @@ func TestProtocolRESP3(t *testing.T) {
})
require.NoError(t, c.WriteArgs("HELLO", "3"))
- values := []string{"%3", "$6", "server", "$5", "redis", "$5",
"proto", ":3", "$4", "mode", "$10", "standalone"}
+ values := []string{"%6",
+ "$6", "server", "$5", "redis",
+ "$7", "version", "$5", "4.0.0",
+ "$5", "proto", ":3",
+ "$4", "mode", "$10", "standalone",
+ "$4", "role", "$6", "master",
+ "$7", "modules", "_",
+ }
for _, line := range values {
c.MustRead(t, line)
}
