From 15640b646f25a04e68443a4a3378ef996a343e5b Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 01:41:29 +0800 Subject: [PATCH 01/11] feat(command): add 'reply' subcommand to control reply mode in CommandClient --- src/commands/cmd_server.cc | 28 +++++++++++++++++++++++++--- src/server/redis_connection.cc | 8 ++++++++ src/server/redis_connection.h | 8 ++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index d1359087c0f..e5a182c3ff1 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -392,7 +392,7 @@ class CommandClient : public Commander { public: Status Parse(const std::vector &args) override { subcommand_ = util::ToLower(args[1]); - // subcommand: getname id kill list info setname + // subcommand: getname id kill list info setname reply if ((subcommand_ == "id" || subcommand_ == "getname" || subcommand_ == "list" || subcommand_ == "info") && args.size() == 2) { return Status::OK(); @@ -412,6 +412,17 @@ class CommandClient : public Commander { return Status::OK(); } + if (subcommand_ == "reply") { + if (args.size() != 3) { + return {Status::RedisParseErr, errInvalidSyntax}; + } + reply_mode_arg_ = util::ToLower(args[2]); + if (reply_mode_arg_ != "on" && reply_mode_arg_ != "off" && reply_mode_arg_ != "skip") { + return {Status::RedisParseErr, errInvalidSyntax}; + } + return Status::OK(); + } + if ((subcommand_ == "kill")) { if (args.size() == 2) { return {Status::RedisParseErr, errInvalidSyntax}; @@ -464,7 +475,7 @@ class CommandClient : public Commander { } return Status::OK(); } - return {Status::RedisInvalidCmd, "Syntax error, try CLIENT LIST|INFO|KILL ip:port|GETNAME|SETNAME"}; + return {Status::RedisInvalidCmd, "Syntax error, try CLIENT LIST|INFO|KILL ip:port|GETNAME|SETNAME|REPLY"}; } Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { @@ -497,15 +508,26 @@ class CommandClient : public Commander { *output = redis::RESP_OK; } return Status::OK(); + } else if (subcommand_ == "reply") { + if (reply_mode_arg_ == "on") { + conn->SetReplyMode(Connection::ReplyMode::ON); + } else if (reply_mode_arg_ == "off") { + conn->SetReplyMode(Connection::ReplyMode::OFF); + } else if (reply_mode_arg_ == "skip") { + conn->SetReplyMode(Connection::ReplyMode::SKIP); + } + *output = redis::RESP_OK; + return Status::OK(); } - return {Status::RedisInvalidCmd, "Syntax error, try CLIENT LIST|INFO|KILL ip:port|GETNAME|SETNAME"}; + return {Status::RedisInvalidCmd, "Syntax error, try CLIENT LIST|INFO|KILL ip:port|GETNAME|SETNAME|REPLY"}; } private: std::string addr_; std::string conn_name_; std::string subcommand_; + std::string reply_mode_arg_; bool skipme_ = false; int64_t kill_type_ = 0; uint64_t id_ = 0; diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index aa21a559509..fd30faa784f 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -130,6 +130,14 @@ void Connection::OnEvent(bufferevent *bev, int16_t events) { } void Connection::Reply(const std::string &msg) { + // CLIENT REPLY logic: OFF disables all replies, SKIP skips one reply + if (reply_mode_ == ReplyMode::OFF) { + return; + } + if (reply_mode_ == ReplyMode::SKIP) { + reply_mode_ = ReplyMode::ON; // Only skip one reply + return; + } owner_->srv->stats.IncrOutboundBytes(msg.size()); redis::Reply(bufferevent_get_output(bev_), msg); } diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index e8b44d94144..fffce6dfd47 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -50,6 +50,8 @@ class Connection : public EvbufCallbackBase { kAsking = 1 << 10, }; + enum class ReplyMode { ON, OFF, SKIP }; + explicit Connection(bufferevent *bev, Worker *owner); ~Connection(); @@ -181,6 +183,10 @@ class Connection : public EvbufCallbackBase { std::set watched_keys; std::atomic watched_keys_modified = false; + // Reply mode getter/setter + void SetReplyMode(ReplyMode mode) { reply_mode_ = mode; } + ReplyMode GetReplyMode() const { return reply_mode_; } + private: uint64_t id_ = 0; std::atomic flags_ = 0; @@ -215,6 +221,8 @@ class Connection : public EvbufCallbackBase { bool importing_ = false; RESP protocol_version_ = RESP::v2; + + ReplyMode reply_mode_ = ReplyMode::ON; }; } // namespace redis From 72f9a296174a927bf5c7f4ec93358bffe9057f39 Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 02:46:10 +0800 Subject: [PATCH 02/11] refactor(reply): replace SKIP mode with skip next reply functionality in CommandClient --- src/commands/cmd_server.cc | 2 +- src/server/redis_connection.cc | 8 ++++---- src/server/redis_connection.h | 10 +++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index e5a182c3ff1..b4ea1e77b31 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -514,7 +514,7 @@ class CommandClient : public Commander { } else if (reply_mode_arg_ == "off") { conn->SetReplyMode(Connection::ReplyMode::OFF); } else if (reply_mode_arg_ == "skip") { - conn->SetReplyMode(Connection::ReplyMode::SKIP); + conn->SetSkipNextReply(true); } *output = redis::RESP_OK; return Status::OK(); diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index fd30faa784f..36eda71bf06 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -130,12 +130,12 @@ void Connection::OnEvent(bufferevent *bev, int16_t events) { } void Connection::Reply(const std::string &msg) { - // CLIENT REPLY logic: OFF disables all replies, SKIP skips one reply - if (reply_mode_ == ReplyMode::OFF) { + // CLIENT REPLY SKIP: skip the next reply only + if (GetAndClearSkipNextReply()) { return; } - if (reply_mode_ == ReplyMode::SKIP) { - reply_mode_ = ReplyMode::ON; // Only skip one reply + // CLIENT REPLY OFF disables all replies + if (reply_mode_ == ReplyMode::OFF) { return; } owner_->srv->stats.IncrOutboundBytes(msg.size()); diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index fffce6dfd47..ae6ffa274e4 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -50,7 +50,7 @@ class Connection : public EvbufCallbackBase { kAsking = 1 << 10, }; - enum class ReplyMode { ON, OFF, SKIP }; + enum class ReplyMode { ON, OFF }; explicit Connection(bufferevent *bev, Worker *owner); ~Connection(); @@ -187,6 +187,13 @@ class Connection : public EvbufCallbackBase { void SetReplyMode(ReplyMode mode) { reply_mode_ = mode; } ReplyMode GetReplyMode() const { return reply_mode_; } + void SetSkipNextReply(bool skip) { skip_next_reply_ = skip; } + bool GetAndClearSkipNextReply() { + bool ret = skip_next_reply_; + skip_next_reply_ = false; + return ret; + } + private: uint64_t id_ = 0; std::atomic flags_ = 0; @@ -223,6 +230,7 @@ class Connection : public EvbufCallbackBase { RESP protocol_version_ = RESP::v2; ReplyMode reply_mode_ = ReplyMode::ON; + bool skip_next_reply_ = false; }; } // namespace redis From 4687b86caeb4c3cba5af47e59cac2a8637319c0c Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 18:45:37 +0800 Subject: [PATCH 03/11] refactor(reply): update reply mode handling to use enum values for clarity --- src/commands/cmd_server.cc | 2 +- src/server/redis_connection.cc | 9 ++++++--- src/server/redis_connection.h | 10 +--------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index b4ea1e77b31..36bb17754b3 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -514,7 +514,7 @@ class CommandClient : public Commander { } else if (reply_mode_arg_ == "off") { conn->SetReplyMode(Connection::ReplyMode::OFF); } else if (reply_mode_arg_ == "skip") { - conn->SetSkipNextReply(true); + conn->SetReplyMode(Connection::ReplyMode::SKIP_NEXT); } *output = redis::RESP_OK; return Status::OK(); diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index 680e42ca175..95ed982b416 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -132,11 +132,14 @@ void Connection::OnEvent(bufferevent *bev, int16_t events) { } void Connection::Reply(const std::string &msg) { - // CLIENT REPLY SKIP: skip the next reply only - if (GetAndClearSkipNextReply()) { + if (reply_mode_ == ReplyMode::SKIP_NEXT) { + reply_mode_ = ReplyMode::SKIP_THIS; + return; + } + if (reply_mode_ == ReplyMode::SKIP_THIS) { + reply_mode_ = ReplyMode::ON; return; } - // CLIENT REPLY OFF disables all replies if (reply_mode_ == ReplyMode::OFF) { return; } diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index ae6ffa274e4..8b055ce1e60 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -50,7 +50,7 @@ class Connection : public EvbufCallbackBase { kAsking = 1 << 10, }; - enum class ReplyMode { ON, OFF }; + enum class ReplyMode { ON, OFF, SKIP_NEXT, SKIP_THIS }; explicit Connection(bufferevent *bev, Worker *owner); ~Connection(); @@ -187,13 +187,6 @@ class Connection : public EvbufCallbackBase { void SetReplyMode(ReplyMode mode) { reply_mode_ = mode; } ReplyMode GetReplyMode() const { return reply_mode_; } - void SetSkipNextReply(bool skip) { skip_next_reply_ = skip; } - bool GetAndClearSkipNextReply() { - bool ret = skip_next_reply_; - skip_next_reply_ = false; - return ret; - } - private: uint64_t id_ = 0; std::atomic flags_ = 0; @@ -230,7 +223,6 @@ class Connection : public EvbufCallbackBase { RESP protocol_version_ = RESP::v2; ReplyMode reply_mode_ = ReplyMode::ON; - bool skip_next_reply_ = false; }; } // namespace redis From f9968caa7a0e91c743ecada2a16eda75f7478ecd Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 18:52:00 +0800 Subject: [PATCH 04/11] refactor(reply): rename reply modes for better clarity and consistency --- src/commands/cmd_server.cc | 2 +- src/server/redis_connection.cc | 6 +++--- src/server/redis_connection.h | 7 ++++++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index 36bb17754b3..70b61585175 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -514,7 +514,7 @@ class CommandClient : public Commander { } else if (reply_mode_arg_ == "off") { conn->SetReplyMode(Connection::ReplyMode::OFF); } else if (reply_mode_arg_ == "skip") { - conn->SetReplyMode(Connection::ReplyMode::SKIP_NEXT); + conn->SetReplyMode(Connection::ReplyMode::SKIP_ONCE_PENDING); } *output = redis::RESP_OK; return Status::OK(); diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index 95ed982b416..12bd8ac1d93 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -132,11 +132,11 @@ void Connection::OnEvent(bufferevent *bev, int16_t events) { } void Connection::Reply(const std::string &msg) { - if (reply_mode_ == ReplyMode::SKIP_NEXT) { - reply_mode_ = ReplyMode::SKIP_THIS; + if (reply_mode_ == ReplyMode::SKIP_ONCE_PENDING) { + reply_mode_ = ReplyMode::SKIP_ONCE_ACTIVE; return; } - if (reply_mode_ == ReplyMode::SKIP_THIS) { + if (reply_mode_ == ReplyMode::SKIP_ONCE_ACTIVE) { reply_mode_ = ReplyMode::ON; return; } diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index 8b055ce1e60..5537fc803ea 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -50,7 +50,12 @@ class Connection : public EvbufCallbackBase { kAsking = 1 << 10, }; - enum class ReplyMode { ON, OFF, SKIP_NEXT, SKIP_THIS }; + enum class ReplyMode { + ON, // Always reply to every command (default) + OFF, // Never reply to any command + SKIP_ONCE_PENDING, // The next command will NOT send a reply, then switch to SKIP_ONCE_ACTIVE + SKIP_ONCE_ACTIVE // This command does NOT send a reply, then automatically switch back to ON + }; explicit Connection(bufferevent *bev, Worker *owner); ~Connection(); From 68643808466c0d7091f6922f97821a3fed193d81 Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 18:59:53 +0800 Subject: [PATCH 05/11] test(reply): add tests for CLIENT REPLY mode switching behavior --- .../unit/introspection/introspection_test.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/gocase/unit/introspection/introspection_test.go b/tests/gocase/unit/introspection/introspection_test.go index af15117dfbf..d3eba647515 100644 --- a/tests/gocase/unit/introspection/introspection_test.go +++ b/tests/gocase/unit/introspection/introspection_test.go @@ -265,6 +265,36 @@ func TestIntrospection(t *testing.T) { require.NoError(t, rdb.Do(ctx, "SET", "key", "value").Err()) require.EqualValues(t, 1, rdb.Do(ctx, "MOVE", "key", "0").Val()) }) + + // Test CLIENT REPLY subcommand behaviors + t.Run("CLIENT REPLY mode switching", func(t *testing.T) { + c := srv.NewTCPClient() + defer func() { require.NoError(t, c.Close()) }() + + // Should reply by default + require.NoError(t, c.WriteArgs("PING")) + c.MustRead(t, "+PONG") + + // Set to OFF, following commands should not reply + require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "OFF")) + c.MustRead(t, "+OK") + require.NoError(t, c.WriteArgs("PING")) + // No reply expected here, do not read + + // Set back to ON, commands should reply again + require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "ON")) + c.MustRead(t, "+OK") + require.NoError(t, c.WriteArgs("PING")) + c.MustRead(t, "+PONG") + + // Set to SKIP, next command should not reply, then reply resumes + require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "SKIP")) + // No reply expected here, do not read + require.NoError(t, c.WriteArgs("PING")) + // Skip one reply, do not read + require.NoError(t, c.WriteArgs("PING")) + c.MustRead(t, "+PONG") + }) } func TestMultiServerIntrospection(t *testing.T) { From 2af9f7a7a895c7ab774e6867ea47da11a617eca6 Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 19:04:23 +0800 Subject: [PATCH 06/11] test(reply): enhance tests for CLIENT REPLY mode with additional ECHO command scenarios --- .../unit/introspection/introspection_test.go | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/gocase/unit/introspection/introspection_test.go b/tests/gocase/unit/introspection/introspection_test.go index d3eba647515..2d520a49429 100644 --- a/tests/gocase/unit/introspection/introspection_test.go +++ b/tests/gocase/unit/introspection/introspection_test.go @@ -272,28 +272,33 @@ func TestIntrospection(t *testing.T) { defer func() { require.NoError(t, c.Close()) }() // Should reply by default - require.NoError(t, c.WriteArgs("PING")) - c.MustRead(t, "+PONG") + require.NoError(t, c.WriteArgs("ECHO", "default")) + c.MustRead(t, `"default"`) // Set to OFF, following commands should not reply require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "OFF")) c.MustRead(t, "+OK") - require.NoError(t, c.WriteArgs("PING")) + require.NoError(t, c.WriteArgs("ECHO", "off")) // No reply expected here, do not read // Set back to ON, commands should reply again require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "ON")) c.MustRead(t, "+OK") - require.NoError(t, c.WriteArgs("PING")) - c.MustRead(t, "+PONG") + require.NoError(t, c.WriteArgs("ECHO", "on")) + c.MustRead(t, `"on"`) // Set to SKIP, next command should not reply, then reply resumes require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "SKIP")) // No reply expected here, do not read - require.NoError(t, c.WriteArgs("PING")) - // Skip one reply, do not read - require.NoError(t, c.WriteArgs("PING")) - c.MustRead(t, "+PONG") + + require.NoError(t, c.WriteArgs("ECHO", "skip1")) + // No reply expected here, do not read + + require.NoError(t, c.WriteArgs("ECHO", "skip2")) + c.MustRead(t, `"skip2"`) + + require.NoError(t, c.WriteArgs("ECHO", "skip3")) + c.MustRead(t, `"skip3"`) }) } From b4e8655b6eeab902b97da22a0ed4f5ae1c89dd5a Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 19:11:51 +0800 Subject: [PATCH 07/11] refactor(reply): improve comment formatting for reply modes in redis_connection.h --- src/server/redis_connection.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index 5537fc803ea..a613c525d15 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -51,10 +51,10 @@ class Connection : public EvbufCallbackBase { }; enum class ReplyMode { - ON, // Always reply to every command (default) - OFF, // Never reply to any command - SKIP_ONCE_PENDING, // The next command will NOT send a reply, then switch to SKIP_ONCE_ACTIVE - SKIP_ONCE_ACTIVE // This command does NOT send a reply, then automatically switch back to ON + ON, // Always reply to every command (default) + OFF, // Never reply to any command + SKIP_ONCE_PENDING, // The next command will NOT send a reply, then switch to SKIP_ONCE_ACTIVE + SKIP_ONCE_ACTIVE // This command does NOT send a reply, then automatically switch back to ON }; explicit Connection(bufferevent *bev, Worker *owner); From 1278ceeb777291973fe1f97d16c4732f1815c5c8 Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 20:51:27 +0800 Subject: [PATCH 08/11] test(reply): update MustRead assertions to use MustReadBulkString for consistency --- tests/gocase/unit/introspection/introspection_test.go | 7 +++---- tests/gocase/util/tcp_client.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/gocase/unit/introspection/introspection_test.go b/tests/gocase/unit/introspection/introspection_test.go index 2d520a49429..956e7b45c39 100644 --- a/tests/gocase/unit/introspection/introspection_test.go +++ b/tests/gocase/unit/introspection/introspection_test.go @@ -273,11 +273,10 @@ func TestIntrospection(t *testing.T) { // Should reply by default require.NoError(t, c.WriteArgs("ECHO", "default")) - c.MustRead(t, `"default"`) + c.MustReadBulkString(t, `"default"`) // Set to OFF, following commands should not reply require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "OFF")) - c.MustRead(t, "+OK") require.NoError(t, c.WriteArgs("ECHO", "off")) // No reply expected here, do not read @@ -285,7 +284,7 @@ func TestIntrospection(t *testing.T) { require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "ON")) c.MustRead(t, "+OK") require.NoError(t, c.WriteArgs("ECHO", "on")) - c.MustRead(t, `"on"`) + c.MustReadBulkString(t, `"on"`) // Set to SKIP, next command should not reply, then reply resumes require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "SKIP")) @@ -295,7 +294,7 @@ func TestIntrospection(t *testing.T) { // No reply expected here, do not read require.NoError(t, c.WriteArgs("ECHO", "skip2")) - c.MustRead(t, `"skip2"`) + c.MustReadBulkString(t, `"skip2"`) require.NoError(t, c.WriteArgs("ECHO", "skip3")) c.MustRead(t, `"skip3"`) diff --git a/tests/gocase/util/tcp_client.go b/tests/gocase/util/tcp_client.go index 46ed3ac9af5..3cd9de2b1a9 100644 --- a/tests/gocase/util/tcp_client.go +++ b/tests/gocase/util/tcp_client.go @@ -87,6 +87,16 @@ func (c *TCPClient) MustReadStrings(t testing.TB, s []string) { } } +func (c *TCPClient) MustReadBulkString(t testing.TB, s string) { + r, err := c.ReadLine() + require.NoError(t, err) + require.Equal(t, "$"+strconv.Itoa(len(s)), r) + + r, err = c.ReadLine() + require.NoError(t, err) + require.Equal(t, s, r) +} + func (c *TCPClient) MustReadStringsWithKey(t testing.TB, key string, s []string) { r, err := c.ReadLine() require.NoError(t, err) From 2e31c0e887c0fa0ed73bd2dd9a385583eeaee18a Mon Sep 17 00:00:00 2001 From: DCjanus Date: Wed, 7 May 2025 20:53:19 +0800 Subject: [PATCH 09/11] fix(test): remove unnecessary quotes in MustReadBulkString assertions for clarity --- tests/gocase/unit/introspection/introspection_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/gocase/unit/introspection/introspection_test.go b/tests/gocase/unit/introspection/introspection_test.go index 956e7b45c39..1b17a98caae 100644 --- a/tests/gocase/unit/introspection/introspection_test.go +++ b/tests/gocase/unit/introspection/introspection_test.go @@ -273,7 +273,7 @@ func TestIntrospection(t *testing.T) { // Should reply by default require.NoError(t, c.WriteArgs("ECHO", "default")) - c.MustReadBulkString(t, `"default"`) + c.MustReadBulkString(t, "default") // Set to OFF, following commands should not reply require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "OFF")) @@ -284,7 +284,7 @@ func TestIntrospection(t *testing.T) { require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "ON")) c.MustRead(t, "+OK") require.NoError(t, c.WriteArgs("ECHO", "on")) - c.MustReadBulkString(t, `"on"`) + c.MustReadBulkString(t, "on") // Set to SKIP, next command should not reply, then reply resumes require.NoError(t, c.WriteArgs("CLIENT", "REPLY", "SKIP")) @@ -294,10 +294,10 @@ func TestIntrospection(t *testing.T) { // No reply expected here, do not read require.NoError(t, c.WriteArgs("ECHO", "skip2")) - c.MustReadBulkString(t, `"skip2"`) + c.MustReadBulkString(t, "skip2") require.NoError(t, c.WriteArgs("ECHO", "skip3")) - c.MustRead(t, `"skip3"`) + c.MustReadBulkString(t, "skip3") }) } From ca8f0afae0e6e365b06bc8ec30cda5dbb7e292ab Mon Sep 17 00:00:00 2001 From: DCjanus Date: Thu, 8 May 2025 17:19:49 +0800 Subject: [PATCH 10/11] refactor(cmd_server): simplify reply mode handling in CommandClient --- src/commands/cmd_server.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index 70b61585175..6d37ae29f77 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -416,8 +416,14 @@ class CommandClient : public Commander { if (args.size() != 3) { return {Status::RedisParseErr, errInvalidSyntax}; } - reply_mode_arg_ = util::ToLower(args[2]); - if (reply_mode_arg_ != "on" && reply_mode_arg_ != "off" && reply_mode_arg_ != "skip") { + auto mode_str = util::ToLower(args[2]); + if (mode_str == "on") { + reply_mode_ = redis::Connection::ReplyMode::ON; + } else if (mode_str == "off") { + reply_mode_ = redis::Connection::ReplyMode::OFF; + } else if (mode_str == "skip") { + reply_mode_ = redis::Connection::ReplyMode::SKIP_ONCE_PENDING; + } else { return {Status::RedisParseErr, errInvalidSyntax}; } return Status::OK(); @@ -509,13 +515,7 @@ class CommandClient : public Commander { } return Status::OK(); } else if (subcommand_ == "reply") { - if (reply_mode_arg_ == "on") { - conn->SetReplyMode(Connection::ReplyMode::ON); - } else if (reply_mode_arg_ == "off") { - conn->SetReplyMode(Connection::ReplyMode::OFF); - } else if (reply_mode_arg_ == "skip") { - conn->SetReplyMode(Connection::ReplyMode::SKIP_ONCE_PENDING); - } + conn->SetReplyMode(reply_mode_); *output = redis::RESP_OK; return Status::OK(); } @@ -527,7 +527,7 @@ class CommandClient : public Commander { std::string addr_; std::string conn_name_; std::string subcommand_; - std::string reply_mode_arg_; + redis::Connection::ReplyMode reply_mode_ = redis::Connection::ReplyMode::ON; bool skipme_ = false; int64_t kill_type_ = 0; uint64_t id_ = 0; From 01f38e5c10af4ad9bd9e0a372b81390d42d69eee Mon Sep 17 00:00:00 2001 From: DCjanus Date: Thu, 8 May 2025 17:32:42 +0800 Subject: [PATCH 11/11] refactor(redis_connection): unify reply mode handling by removing SKIP_ONCE states --- src/commands/cmd_server.cc | 6 ++++-- src/server/redis_connection.cc | 6 +----- src/server/redis_connection.h | 7 +++---- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index 6d37ae29f77..661df2cf599 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -422,7 +422,7 @@ class CommandClient : public Commander { } else if (mode_str == "off") { reply_mode_ = redis::Connection::ReplyMode::OFF; } else if (mode_str == "skip") { - reply_mode_ = redis::Connection::ReplyMode::SKIP_ONCE_PENDING; + reply_mode_ = redis::Connection::ReplyMode::SKIP; } else { return {Status::RedisParseErr, errInvalidSyntax}; } @@ -516,7 +516,9 @@ class CommandClient : public Commander { return Status::OK(); } else if (subcommand_ == "reply") { conn->SetReplyMode(reply_mode_); - *output = redis::RESP_OK; + if (reply_mode_ != redis::Connection::ReplyMode::SKIP) { + *output = redis::RESP_OK; + } return Status::OK(); } diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index 12bd8ac1d93..443f29a07fb 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -132,11 +132,7 @@ void Connection::OnEvent(bufferevent *bev, int16_t events) { } void Connection::Reply(const std::string &msg) { - if (reply_mode_ == ReplyMode::SKIP_ONCE_PENDING) { - reply_mode_ = ReplyMode::SKIP_ONCE_ACTIVE; - return; - } - if (reply_mode_ == ReplyMode::SKIP_ONCE_ACTIVE) { + if (reply_mode_ == ReplyMode::SKIP) { reply_mode_ = ReplyMode::ON; return; } diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h index a613c525d15..d31fbce1c32 100644 --- a/src/server/redis_connection.h +++ b/src/server/redis_connection.h @@ -51,10 +51,9 @@ class Connection : public EvbufCallbackBase { }; enum class ReplyMode { - ON, // Always reply to every command (default) - OFF, // Never reply to any command - SKIP_ONCE_PENDING, // The next command will NOT send a reply, then switch to SKIP_ONCE_ACTIVE - SKIP_ONCE_ACTIVE // This command does NOT send a reply, then automatically switch back to ON + ON, // Always reply to every command (default) + OFF, // Never reply to any command + SKIP // Skip reply for the next command, then automatically switch back to ON }; explicit Connection(bufferevent *bev, Worker *owner);