From fbdc8adbf45764deba63282dbdead73ade9140b5 Mon Sep 17 00:00:00 2001 From: hulk Date: Wed, 5 Jul 2023 12:47:09 +0800 Subject: [PATCH 1/3] Fix the inconsistent response behavior with Redis in memory usage command (#1539) --- src/commands/cmd_server.cc | 9 ++++++++- tests/gocase/unit/disk/disk_test.go | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index 18c56f86266..7a796aa3b7a 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -291,7 +291,14 @@ class CommandDisk : public Commander { uint64_t result = 0; s = disk_db.GetKeySize(args_[2], type, &result); - if (!s.ok()) return {Status::RedisExecErr, s.ToString()}; + if (!s.ok()) { + // Redis returns the Nil string when the key does not exist + if (s.IsNotFound()) { + *output = redis::NilString(); + return Status::OK(); + } + return {Status::RedisExecErr, s.ToString()}; + } *output = redis::Integer(result); return Status::OK(); diff --git a/tests/gocase/unit/disk/disk_test.go b/tests/gocase/unit/disk/disk_test.go index f161ddbc6e2..9f0a279303e 100644 --- a/tests/gocase/unit/disk/disk_test.go +++ b/tests/gocase/unit/disk/disk_test.go @@ -140,10 +140,10 @@ func TestDisk(t *testing.T) { }) t.Run("Disk usage nonexistent key ", func(t *testing.T) { - require.ErrorContains(t, rdb.Do(ctx, "Disk", "usage", "nonexistentkey").Err(), "Not found") + require.ErrorIs(t, rdb.Do(ctx, "Disk", "usage", "nonexistentkey").Err(), redis.Nil) }) - t.Run("Memory usage existing key - check that Kvrocks support it", func(t *testing.T) { + t.Run("Memory usage - check that Kvrocks support it", func(t *testing.T) { key := "arbitrary-key" require.NoError(t, rdb.Del(ctx, key).Err()) @@ -153,5 +153,8 @@ func TestDisk(t *testing.T) { size, err := rdb.MemoryUsage(ctx, key).Result() require.NoError(t, err) require.Greater(t, size, int64(0)) + + _, err = rdb.MemoryUsage(ctx, "nonexistentkey").Result() + require.ErrorIs(t, err, redis.Nil) }) } From 49993a1b769edd573a953e6d100736b54928636d Mon Sep 17 00:00:00 2001 From: hulk Date: Wed, 5 Jul 2023 16:00:10 +0800 Subject: [PATCH 2/3] Fix compile error after upgrading the rocksdb (#1542) --- cmake/rocksdb.cmake | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmake/rocksdb.cmake b/cmake/rocksdb.cmake index 0c9c201ea34..e57a12fe94a 100644 --- a/cmake/rocksdb.cmake +++ b/cmake/rocksdb.cmake @@ -23,6 +23,10 @@ if (DISABLE_JEMALLOC) set(COMPILE_WITH_JEMALLOC OFF) endif() +if (NOT PORTABLE) + set(PORTABLE 0) +endif() + include(cmake/utils.cmake) FetchContent_DeclareGitHubWithMirror(rocksdb From 74f710f6317ee8bb7c3a186b0593b0fcf668add8 Mon Sep 17 00:00:00 2001 From: Binbin Date: Wed, 5 Jul 2023 18:35:57 +0800 Subject: [PATCH 3/3] Fix ZMPOP/BZMPOP duplicate parameters (#1543) --- src/commands/cmd_zset.cc | 18 +++++++++------- tests/gocase/unit/type/zset/zset_test.go | 27 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/commands/cmd_zset.cc b/src/commands/cmd_zset.cc index 35a6a6ca840..a226d8702c0 100644 --- a/src/commands/cmd_zset.cc +++ b/src/commands/cmd_zset.cc @@ -470,11 +470,11 @@ class CommandZMPop : public Commander { } while (parser.Good()) { - if (parser.EatEqICase("min")) { + if (flag_ == ZSET_NONE && parser.EatEqICase("min")) { flag_ = ZSET_MIN; - } else if (parser.EatEqICase("max")) { + } else if (flag_ == ZSET_NONE && parser.EatEqICase("max")) { flag_ = ZSET_MAX; - } else if (parser.EatEqICase("count")) { + } else if (count_ == 0 && parser.EatEqICase("count")) { count_ = GET_OR_RET(parser.TakeInt(NumericRange{1, std::numeric_limits::max()})); } else { return parser.InvalidSyntax(); @@ -483,6 +483,7 @@ class CommandZMPop : public Commander { if (flag_ == ZSET_NONE) { return parser.InvalidSyntax(); } + if (count_ == 0) count_ = 1; return Commander::Parse(args); } @@ -514,7 +515,7 @@ class CommandZMPop : public Commander { int numkeys_; std::vector keys_; enum { ZSET_MIN, ZSET_MAX, ZSET_NONE } flag_ = ZSET_NONE; - int count_ = 1; + int count_ = 0; }; class CommandBZMPop : public Commander, @@ -535,11 +536,11 @@ class CommandBZMPop : public Commander, } while (parser.Good()) { - if (parser.EatEqICase("min")) { + if (flag_ == ZSET_NONE && parser.EatEqICase("min")) { flag_ = ZSET_MIN; - } else if (parser.EatEqICase("max")) { + } else if (flag_ == ZSET_NONE && parser.EatEqICase("max")) { flag_ = ZSET_MAX; - } else if (parser.EatEqICase("count")) { + } else if (count_ == 0 && parser.EatEqICase("count")) { count_ = GET_OR_RET(parser.TakeInt(NumericRange{1, std::numeric_limits::max()})); } else { return parser.InvalidSyntax(); @@ -549,6 +550,7 @@ class CommandBZMPop : public Commander, if (flag_ == ZSET_NONE) { return parser.InvalidSyntax(); } + if (count_ == 0) count_ = 1; return Commander::Parse(args); } @@ -659,7 +661,7 @@ class CommandBZMPop : public Commander, int num_keys_; std::vector keys_; enum { ZSET_MIN, ZSET_MAX, ZSET_NONE } flag_ = ZSET_NONE; - int count_ = 1; + int count_ = 0; Server *svr_ = nullptr; Connection *conn_ = nullptr; UniqueEvent timer_; diff --git a/tests/gocase/unit/type/zset/zset_test.go b/tests/gocase/unit/type/zset/zset_test.go index a6b150c770b..1b5a2980956 100644 --- a/tests/gocase/unit/type/zset/zset_test.go +++ b/tests/gocase/unit/type/zset/zset_test.go @@ -392,6 +392,19 @@ func basicTests(t *testing.T, rdb *redis.Client, ctx context.Context, encoding s require.EqualValues(t, 0, rdb.Exists(ctx, "zseta", "zsetb").Val()) }) + t.Run(fmt.Sprintf("ZMPOP error - %s", encoding), func(t *testing.T) { + rdb.Del(ctx, "zseta") + rdb.Del(ctx, "zsetb") + + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", 1, "zseta").Err(), ".*wrong number of arguments.*") + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", "wrong_numkeys", "zseta", "zsetb").Err(), ".*not started as an integer.*") + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", 2, "zseta", "min").Err(), ".*syntax error.*") + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", 2, "zseta", "zsetb", "min", "min").Err(), ".*syntax error.*") + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", 1, "zseta", "min", "max").Err(), ".*syntax error.*") + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", 1, "zseta", "min", "count", "wrong_count").Err(), ".*not started as an integer.*") + util.ErrorRegexp(t, rdb.Do(ctx, "zmpop", 1, "zseta", "min", "count", 1, "count", 10).Err(), ".*syntax error.*") + }) + t.Run(fmt.Sprintf("BZMPOP basics - %s", encoding), func(t *testing.T) { rdb.Del(ctx, "zseta") rdb.Del(ctx, "zsetb") @@ -427,6 +440,20 @@ func basicTests(t *testing.T, rdb *redis.Client, ctx context.Context, encoding s require.Equal(t, []redis.Z{{Score: 1, Member: "a"}, {Score: 2, Member: "b"}}, zset) }) + t.Run(fmt.Sprintf("BZMPOP error - %s", encoding), func(t *testing.T) { + rdb.Del(ctx, "zseta") + rdb.Del(ctx, "zsetb") + + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, 1, "zseta").Err(), ".*wrong number of arguments.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", "wrong_timeout", 1, "zseta", "min").Err(), ".*not started as a number.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, "wrong_numkeys", "zseta", "min").Err(), ".*not started as an integer.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, 2, "zseta", "min").Err(), ".*syntax error.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, 1, "zseta", "min", "max").Err(), ".*syntax error.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, 2, "zseta", "zsetb", "min", "min").Err(), ".*syntax error.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, 1, "zseta", "min", "count", "wrong_count").Err(), ".*not started as an integer.*") + util.ErrorRegexp(t, rdb.Do(ctx, "bzmpop", 0.1, 1, "zseta", "min", "count", 1, "count", 10).Err(), ".*syntax error.*") + }) + t.Run(fmt.Sprintf("ZRANGESTORE basics - %s", encoding), func(t *testing.T) { rdb.Del(ctx, "zsrc") rdb.Del(ctx, "zdst")