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 ed2a64bc Fix redundant ERR prefix in cluster redirect error message 
(#1928)
ed2a64bc is described below

commit ed2a64bc7504379469160fe4d8decf0a43347745
Author: hulk <[email protected]>
AuthorDate: Fri Dec 8 21:28:21 2023 +0800

    Fix redundant ERR prefix in cluster redirect error message (#1928)
    
    Currently, Kvrocks cluster mode returns a MOVED/TRYAGAIN error message with 
the 'ERR' prefix, which will cause the Redis client can't recognize the 
redirection message. To make it compatible with the Redis client, we should 
remove this prefix.
    
    Before this patch, Kvrocks will return a MOVED error message:
    
    ```
    > redis-cli -p 30001 -c set a 1
    (error) ERR MOVED 15495 127.0.0.1:30005
    ```
    
    After applying this patch, it works well with redis-cli redirection:
    
    ```
    > redis-cli -p 30001 -c set a 1
    OK
    ```
---
 src/server/redis_connection.cc                   |  2 +-
 tests/gocase/integration/cluster/cluster_test.go | 22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc
index 4d468983..73ed3a80 100644
--- a/src/server/redis_connection.cc
+++ b/src/server/redis_connection.cc
@@ -393,7 +393,7 @@ void Connection::ExecuteCommands(std::deque<CommandTokens> 
*to_process_cmds) {
       s = srv_->cluster->CanExecByMySelf(attributes, cmd_tokens, this);
       if (!s.IsOK()) {
         if (is_multi_exec) multi_error_ = true;
-        Reply(redis::Error("ERR " + s.Msg()));
+        Reply(redis::Error(s.Msg()));
         continue;
       }
     }
diff --git a/tests/gocase/integration/cluster/cluster_test.go 
b/tests/gocase/integration/cluster/cluster_test.go
index 1716630a..8bc42fda 100644
--- a/tests/gocase/integration/cluster/cluster_test.go
+++ b/tests/gocase/integration/cluster/cluster_test.go
@@ -237,7 +237,7 @@ func TestClusterSlotSet(t *testing.T) {
 
        slotKey := util.SlotTable[0]
        require.NoError(t, rdb1.Set(ctx, slotKey, 0, 0).Err())
-       util.ErrorRegexp(t, rdb2.Set(ctx, slotKey, 0, 0).Err(), 
fmt.Sprintf(".*MOVED 0.*%d.*", srv1.Port()))
+       util.ErrorRegexp(t, rdb2.Set(ctx, slotKey, 0, 0).Err(), 
fmt.Sprintf("MOVED 0.*%d.*", srv1.Port()))
 
        require.NoError(t, rdb2.Do(ctx, "clusterx", "setslot", "0", "node", 
nodeID2, "3").Err())
        require.NoError(t, rdb1.Do(ctx, "clusterx", "setslot", "0", "node", 
nodeID2, "3").Err())
@@ -254,7 +254,7 @@ func TestClusterSlotSet(t *testing.T) {
        require.EqualValues(t, []redis.ClusterNode{{ID: nodeID1, Addr: 
srv1.HostPort()}}, slots[1].Nodes)
 
        require.NoError(t, rdb2.Set(ctx, slotKey, 0, 0).Err())
-       util.ErrorRegexp(t, rdb1.Set(ctx, slotKey, 0, 0).Err(), 
fmt.Sprintf(".*MOVED 0.*%d.*", srv2.Port()))
+       util.ErrorRegexp(t, rdb1.Set(ctx, slotKey, 0, 0).Err(), 
fmt.Sprintf("MOVED 0.*%d.*", srv2.Port()))
        require.NoError(t, rdb2.Do(ctx, "clusterx", "setslot", "1-3 4", "node", 
nodeID2, "4").Err())
        require.NoError(t, rdb1.Do(ctx, "clusterx", "setslot", "1-3 4", "node", 
nodeID2, "4").Err())
        slots = rdb2.ClusterSlots(ctx).Val()
@@ -292,8 +292,8 @@ func TestClusterMultiple(t *testing.T) {
        }
 
        t.Run("requests on non-init-cluster", func(t *testing.T) {
-               util.ErrorRegexp(t, rdb[0].Set(ctx, util.SlotTable[0], 0, 
0).Err(), ".*CLUSTERDOWN.*not served.*")
-               util.ErrorRegexp(t, rdb[2].Set(ctx, util.SlotTable[16383], 
16383, 0).Err(), ".*CLUSTERDOWN.*not served.*")
+               util.ErrorRegexp(t, rdb[0].Set(ctx, util.SlotTable[0], 0, 
0).Err(), "CLUSTERDOWN.*not served.*")
+               util.ErrorRegexp(t, rdb[2].Set(ctx, util.SlotTable[16383], 
16383, 0).Err(), "CLUSTERDOWN.*not served.*")
        })
 
        clusterNodes := fmt.Sprintf("%s %s %d master - 0-1 3 5-8191\n", 
nodeID[1], srv[1].Host(), srv[1].Port())
@@ -318,11 +318,11 @@ func TestClusterMultiple(t *testing.T) {
 
        t.Run("MOVED slot ip:port if needed", func(t *testing.T) {
                // request node2 that doesn't serve slot 0, we will receive 
MOVED
-               util.ErrorRegexp(t, rdb[2].Set(ctx, util.SlotTable[0], 0, 
0).Err(), fmt.Sprintf(".*MOVED 0.*%d.*", srv[1].Port()))
+               util.ErrorRegexp(t, rdb[2].Set(ctx, util.SlotTable[0], 0, 
0).Err(), fmt.Sprintf("MOVED 0.*%d.*", srv[1].Port()))
                // request node3 that doesn't serve slot 0, we will receive 
MOVED
-               util.ErrorRegexp(t, rdb[3].Get(ctx, util.SlotTable[0]).Err(), 
fmt.Sprintf(".*MOVED 0.*%d.*", srv[1].Port()))
+               util.ErrorRegexp(t, rdb[3].Get(ctx, util.SlotTable[0]).Err(), 
fmt.Sprintf("MOVED 0.*%d.*", srv[1].Port()))
                // request node1 that doesn't serve slot 16383, we will receive 
MOVED, and the MOVED node must be master
-               util.ErrorRegexp(t, rdb[1].Get(ctx, 
util.SlotTable[16383]).Err(), fmt.Sprintf(".*MOVED 16383.*%d.*", srv[2].Port()))
+               util.ErrorRegexp(t, rdb[1].Get(ctx, 
util.SlotTable[16383]).Err(), fmt.Sprintf("MOVED 16383.*%d.*", srv[2].Port()))
        })
 
        t.Run("requests on cluster are ok", func(t *testing.T) {
@@ -338,12 +338,12 @@ func TestClusterMultiple(t *testing.T) {
        })
 
        t.Run("requests non-member of cluster, role is master", func(t 
*testing.T) {
-               util.ErrorRegexp(t, rdb[0].Set(ctx, util.SlotTable[0], 0, 
0).Err(), fmt.Sprintf(".*MOVED 0.*%d.*", srv[1].Port()))
-               util.ErrorRegexp(t, rdb[0].Get(ctx, 
util.SlotTable[16383]).Err(), fmt.Sprintf(".*MOVED 16383.*%d.*", srv[2].Port()))
+               util.ErrorRegexp(t, rdb[0].Set(ctx, util.SlotTable[0], 0, 
0).Err(), fmt.Sprintf("MOVED 0.*%d.*", srv[1].Port()))
+               util.ErrorRegexp(t, rdb[0].Get(ctx, 
util.SlotTable[16383]).Err(), fmt.Sprintf("MOVED 16383.*%d.*", srv[2].Port()))
        })
 
        t.Run("cluster slot is not served", func(t *testing.T) {
-               util.ErrorRegexp(t, rdb[1].Set(ctx, util.SlotTable[2], 2, 
0).Err(), ".*CLUSTERDOWN.*not served.*")
+               util.ErrorRegexp(t, rdb[1].Set(ctx, util.SlotTable[2], 2, 
0).Err(), "CLUSTERDOWN.*not served.*")
        })
 
        t.Run("multiple keys(cross slots) command is wrong", func(t *testing.T) 
{
@@ -365,7 +365,7 @@ func TestClusterMultiple(t *testing.T) {
                require.NoError(t, rdb[1].Set(ctx, util.SlotTable[0], 
"no-multi", 0).Err())
                require.NoError(t, rdb[1].Do(ctx, "MULTI").Err())
                require.NoError(t, rdb[1].Set(ctx, util.SlotTable[0], "multi", 
0).Err())
-               util.ErrorRegexp(t, rdb[1].Set(ctx, util.SlotTable[16383], 0, 
0).Err(), fmt.Sprintf(".*MOVED 16383.*%d.*", srv[2].Port()))
+               util.ErrorRegexp(t, rdb[1].Set(ctx, util.SlotTable[16383], 0, 
0).Err(), fmt.Sprintf("MOVED 16383.*%d.*", srv[2].Port()))
                require.ErrorContains(t, rdb[1].Do(ctx, "EXEC").Err(), 
"EXECABORT")
                require.Equal(t, "no-multi", rdb[1].Get(ctx, 
util.SlotTable[0]).Val())
        })

Reply via email to