This is an automated email from the ASF dual-hosted git repository.

twice 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 7a3cc8cc feat(cmd): add blocking flag and remove useless flags (#2637)
7a3cc8cc is described below

commit 7a3cc8cc32f0723ebe24c0d4340db864030f9b5a
Author: Twice <[email protected]>
AuthorDate: Sun Nov 3 00:50:08 2024 +0800

    feat(cmd): add blocking flag and remove useless flags (#2637)
    
    Co-authored-by: hulk <[email protected]>
---
 src/commands/cmd_function.cc    |  9 +++++----
 src/commands/cmd_list.cc        |  9 +++++----
 src/commands/cmd_pubsub.cc      | 19 +++++++++----------
 src/commands/cmd_replication.cc | 11 +++++------
 src/commands/cmd_script.cc      | 12 ++++++------
 src/commands/cmd_server.cc      |  2 +-
 src/commands/cmd_stream.cc      |  4 ++--
 src/commands/cmd_zset.cc        |  6 +++---
 src/commands/commander.h        | 17 +++++------------
 src/server/redis_connection.cc  | 11 +++++++----
 10 files changed, 48 insertions(+), 52 deletions(-)

diff --git a/src/commands/cmd_function.cc b/src/commands/cmd_function.cc
index ff41257a..c229cdaa 100644
--- a/src/commands/cmd_function.cc
+++ b/src/commands/cmd_function.cc
@@ -109,9 +109,10 @@ uint64_t GenerateFunctionFlags(uint64_t flags, const 
std::vector<std::string> &a
   return flags;
 }
 
-REDIS_REGISTER_COMMANDS(
-    Function, MakeCmdAttr<CommandFunction>("function", -2, "exclusive 
no-script", NO_KEY, GenerateFunctionFlags),
-    MakeCmdAttr<CommandFCall<>>("fcall", -3, "exclusive write no-script", 
GetScriptEvalKeyRange),
-    MakeCmdAttr<CommandFCall<true>>("fcall_ro", -3, "read-only ro-script 
no-script", GetScriptEvalKeyRange));
+REDIS_REGISTER_COMMANDS(Function,
+                        MakeCmdAttr<CommandFunction>("function", -2, 
"exclusive no-script", NO_KEY,
+                                                     GenerateFunctionFlags),
+                        MakeCmdAttr<CommandFCall<>>("fcall", -3, "exclusive 
write no-script", GetScriptEvalKeyRange),
+                        MakeCmdAttr<CommandFCall<true>>("fcall_ro", -3, 
"read-only no-script", GetScriptEvalKeyRange));
 
 }  // namespace redis
diff --git a/src/commands/cmd_list.cc b/src/commands/cmd_list.cc
index 367d8db6..cf588ec0 100644
--- a/src/commands/cmd_list.cc
+++ b/src/commands/cmd_list.cc
@@ -865,14 +865,15 @@ class CommandLPos : public Commander {
   PosSpec spec_;
 };
 
-REDIS_REGISTER_COMMANDS(List, MakeCmdAttr<CommandBLPop>("blpop", -3, "write 
no-script", 1, -2, 1),
-                        MakeCmdAttr<CommandBRPop>("brpop", -3, "write 
no-script", 1, -2, 1),
-                        MakeCmdAttr<CommandBLMPop>("blmpop", -5, "write 
no-script", CommandBLMPop::keyRangeGen),
+REDIS_REGISTER_COMMANDS(List, MakeCmdAttr<CommandBLPop>("blpop", -3, "write 
no-script blocking", 1, -2, 1),
+                        MakeCmdAttr<CommandBRPop>("brpop", -3, "write 
no-script blocking", 1, -2, 1),
+                        MakeCmdAttr<CommandBLMPop>("blmpop", -5, "write 
no-script blocking",
+                                                   CommandBLMPop::keyRangeGen),
                         MakeCmdAttr<CommandLIndex>("lindex", 3, "read-only", 
1, 1, 1),
                         MakeCmdAttr<CommandLInsert>("linsert", 5, "write 
slow", 1, 1, 1),
                         MakeCmdAttr<CommandLLen>("llen", 2, "read-only", 1, 1, 
1),
                         MakeCmdAttr<CommandLMove>("lmove", 5, "write", 1, 2, 
1),
-                        MakeCmdAttr<CommandBLMove>("blmove", 6, "write", 1, 2, 
1),
+                        MakeCmdAttr<CommandBLMove>("blmove", 6, "write 
blocking", 1, 2, 1),
                         MakeCmdAttr<CommandLPop>("lpop", -2, "write", 1, 1, 
1),  //
                         MakeCmdAttr<CommandLPos>("lpos", -3, "read-only", 1, 
1, 1),
                         MakeCmdAttr<CommandLPush>("lpush", -3, "write", 1, 1, 
1),
diff --git a/src/commands/cmd_pubsub.cc b/src/commands/cmd_pubsub.cc
index e7e4dee7..8aaa7f4b 100644
--- a/src/commands/cmd_pubsub.cc
+++ b/src/commands/cmd_pubsub.cc
@@ -252,15 +252,14 @@ class CommandPubSub : public Commander {
   std::string subcommand_;
 };
 
-REDIS_REGISTER_COMMANDS(
-    Pubsub, MakeCmdAttr<CommandPublish>("publish", 3, "read-only pub-sub", 
NO_KEY),
-    MakeCmdAttr<CommandMPublish>("mpublish", -3, "read-only pub-sub", NO_KEY),
-    MakeCmdAttr<CommandSubscribe>("subscribe", -2, "read-only pub-sub no-multi 
no-script", NO_KEY),
-    MakeCmdAttr<CommandUnSubscribe>("unsubscribe", -1, "read-only pub-sub 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandPSubscribe>("psubscribe", -2, "read-only pub-sub 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandPUnSubscribe>("punsubscribe", -1, "read-only pub-sub 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandSSubscribe>("ssubscribe", -2, "read-only pub-sub 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandSUnSubscribe>("sunsubscribe", -1, "read-only pub-sub 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandPubSub>("pubsub", -2, "read-only pub-sub no-script", 
NO_KEY), )
+REDIS_REGISTER_COMMANDS(Pubsub, MakeCmdAttr<CommandPublish>("publish", 3, 
"read-only", NO_KEY),
+                        MakeCmdAttr<CommandMPublish>("mpublish", -3, 
"read-only", NO_KEY),
+                        MakeCmdAttr<CommandSubscribe>("subscribe", -2, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandUnSubscribe>("unsubscribe", -1, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandPSubscribe>("psubscribe", -2, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandPUnSubscribe>("punsubscribe", -1, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandSSubscribe>("ssubscribe", -2, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandSUnSubscribe>("sunsubscribe", -1, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandPubSub>("pubsub", -2, "read-only 
no-script", NO_KEY), )
 
 }  // namespace redis
diff --git a/src/commands/cmd_replication.cc b/src/commands/cmd_replication.cc
index e649516e..4d46e11d 100644
--- a/src/commands/cmd_replication.cc
+++ b/src/commands/cmd_replication.cc
@@ -344,11 +344,10 @@ class CommandDBName : public Commander {
   }
 };
 
-REDIS_REGISTER_COMMANDS(
-    Replication, MakeCmdAttr<CommandReplConf>("replconf", -3, "read-only 
replication no-script", NO_KEY),
-    MakeCmdAttr<CommandPSync>("psync", -2, "read-only replication no-multi 
no-script", NO_KEY),
-    MakeCmdAttr<CommandFetchMeta>("_fetch_meta", 1, "read-only replication 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandFetchFile>("_fetch_file", 2, "read-only replication 
no-multi no-script", NO_KEY),
-    MakeCmdAttr<CommandDBName>("_db_name", 1, "read-only replication 
no-multi", NO_KEY), )
+REDIS_REGISTER_COMMANDS(Replication, MakeCmdAttr<CommandReplConf>("replconf", 
-3, "read-only no-script", NO_KEY),
+                        MakeCmdAttr<CommandPSync>("psync", -2, "read-only 
no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandFetchMeta>("_fetch_meta", 1, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandFetchFile>("_fetch_file", 2, 
"read-only no-multi no-script", NO_KEY),
+                        MakeCmdAttr<CommandDBName>("_db_name", 1, "read-only 
no-multi", NO_KEY), )
 
 }  // namespace redis
diff --git a/src/commands/cmd_script.cc b/src/commands/cmd_script.cc
index 3098d005..b4cf3539 100644
--- a/src/commands/cmd_script.cc
+++ b/src/commands/cmd_script.cc
@@ -125,11 +125,11 @@ uint64_t GenerateScriptFlags(uint64_t flags, const 
std::vector<std::string> &arg
   return flags;
 }
 
-REDIS_REGISTER_COMMANDS(
-    Script, MakeCmdAttr<CommandEval>("eval", -3, "exclusive write no-script", 
GetScriptEvalKeyRange),
-    MakeCmdAttr<CommandEvalSHA>("evalsha", -3, "exclusive write no-script", 
GetScriptEvalKeyRange),
-    MakeCmdAttr<CommandEvalRO>("eval_ro", -3, "read-only no-script ro-script", 
GetScriptEvalKeyRange),
-    MakeCmdAttr<CommandEvalSHARO>("evalsha_ro", -3, "read-only no-script 
ro-script", GetScriptEvalKeyRange),
-    MakeCmdAttr<CommandScript>("script", -2, "exclusive no-script", NO_KEY), )
+REDIS_REGISTER_COMMANDS(Script,
+                        MakeCmdAttr<CommandEval>("eval", -3, "exclusive write 
no-script", GetScriptEvalKeyRange),
+                        MakeCmdAttr<CommandEvalSHA>("evalsha", -3, "exclusive 
write no-script", GetScriptEvalKeyRange),
+                        MakeCmdAttr<CommandEvalRO>("eval_ro", -3, "read-only 
no-script", GetScriptEvalKeyRange),
+                        MakeCmdAttr<CommandEvalSHARO>("evalsha_ro", -3, 
"read-only no-script", GetScriptEvalKeyRange),
+                        MakeCmdAttr<CommandScript>("script", -2, "exclusive 
no-script", NO_KEY), )
 
 }  // namespace redis
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index 16fea925..e2938267 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -1364,7 +1364,7 @@ REDIS_REGISTER_COMMANDS(Server, 
MakeCmdAttr<CommandAuth>("auth", 2, "read-only o
                         MakeCmdAttr<CommandSlaveOf>("slaveof", 3, "read-only 
exclusive no-script", NO_KEY),
                         MakeCmdAttr<CommandStats>("stats", 1, "read-only", 
NO_KEY),
                         MakeCmdAttr<CommandRdb>("rdb", -3, "write exclusive", 
NO_KEY),
-                        MakeCmdAttr<CommandReset>("reset", 1, "ok-loading 
multi no-script pub-sub", NO_KEY),
+                        MakeCmdAttr<CommandReset>("reset", 1, "ok-loading 
multi no-script", NO_KEY),
                         MakeCmdAttr<CommandApplyBatch>("applybatch", -2, 
"write no-multi", NO_KEY),
                         MakeCmdAttr<CommandDump>("dump", 2, "read-only", 1, 1, 
1),
                         MakeCmdAttr<CommandPollUpdates>("pollupdates", -2, 
"read-only", NO_KEY), )
diff --git a/src/commands/cmd_stream.cc b/src/commands/cmd_stream.cc
index a7aa9802..521e5ca3 100644
--- a/src/commands/cmd_stream.cc
+++ b/src/commands/cmd_stream.cc
@@ -1888,8 +1888,8 @@ REDIS_REGISTER_COMMANDS(Stream, 
MakeCmdAttr<CommandXAck>("xack", -4, "write no-d
                         MakeCmdAttr<CommandXPending>("xpending", -3, 
"read-only", 1, 1, 1),
                         MakeCmdAttr<CommandXRange>("xrange", -4, "read-only", 
1, 1, 1),
                         MakeCmdAttr<CommandXRevRange>("xrevrange", -2, 
"read-only", 1, 1, 1),
-                        MakeCmdAttr<CommandXRead>("xread", -4, "read-only", 
NO_KEY),
-                        MakeCmdAttr<CommandXReadGroup>("xreadgroup", -7, 
"write", NO_KEY),
+                        MakeCmdAttr<CommandXRead>("xread", -4, "read-only 
blocking", NO_KEY),
+                        MakeCmdAttr<CommandXReadGroup>("xreadgroup", -7, 
"write blocking", NO_KEY),
                         MakeCmdAttr<CommandXTrim>("xtrim", -4, "write 
no-dbsize-check", 1, 1, 1),
                         MakeCmdAttr<CommandXSetId>("xsetid", -3, "write", 1, 
1, 1))
 
diff --git a/src/commands/cmd_zset.cc b/src/commands/cmd_zset.cc
index d26abb54..5b1f392f 100644
--- a/src/commands/cmd_zset.cc
+++ b/src/commands/cmd_zset.cc
@@ -1549,10 +1549,10 @@ REDIS_REGISTER_COMMANDS(ZSet, 
MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, 
"read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 
1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 
1, 1),
-                        MakeCmdAttr<CommandBZPopMax>("bzpopmax", -3, "write", 
1, -2, 1),
-                        MakeCmdAttr<CommandBZPopMin>("bzpopmin", -3, "write", 
1, -2, 1),
+                        MakeCmdAttr<CommandBZPopMax>("bzpopmax", -3, "write 
blocking", 1, -2, 1),
+                        MakeCmdAttr<CommandBZPopMin>("bzpopmin", -3, "write 
blocking", 1, -2, 1),
                         MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 
CommandZMPop::Range),
-                        MakeCmdAttr<CommandBZMPop>("bzmpop", -5, "write", 
CommandBZMPop::Range),
+                        MakeCmdAttr<CommandBZMPop>("bzmpop", -5, "write 
blocking", CommandBZMPop::Range),
                         MakeCmdAttr<CommandZRangeStore>("zrangestore", -5, 
"write", 1, 1, 1),
                         MakeCmdAttr<CommandZRange>("zrange", -4, "read-only", 
1, 1, 1),
                         MakeCmdAttr<CommandZRevRange>("zrevrange", -4, 
"read-only", 1, 1, 1),
diff --git a/src/commands/commander.h b/src/commands/commander.h
index 87efdb01..066c4ce8 100644
--- a/src/commands/commander.h
+++ b/src/commands/commander.h
@@ -58,18 +58,15 @@ struct CommandAttributes;
 enum CommandFlags : uint64_t {
   kCmdWrite = 1ULL << 0,           // "write" flag
   kCmdReadOnly = 1ULL << 1,        // "read-only" flag
-  kCmdReplication = 1ULL << 2,     // "replication" flag
-  kCmdPubSub = 1ULL << 3,          // "pub-sub" flag
-  kCmdScript = 1ULL << 4,          // "script" flag
   kCmdLoading = 1ULL << 5,         // "ok-loading" flag
-  kCmdMulti = 1ULL << 6,           // "multi" flag
+  kCmdEndMulti = 1ULL << 6,        // "multi" flag, for ending a MULTI scope
   kCmdExclusive = 1ULL << 7,       // "exclusive" flag
   kCmdNoMulti = 1ULL << 8,         // "no-multi" flag
   kCmdNoScript = 1ULL << 9,        // "no-script" flag
-  kCmdROScript = 1ULL << 10,       // "ro-script" flag for read-only script 
commands
   kCmdCluster = 1ULL << 11,        // "cluster" flag
   kCmdNoDBSizeCheck = 1ULL << 12,  // "no-dbsize-check" flag
   kCmdSlow = 1ULL << 13,           // "slow" flag
+  kCmdBlocking = 1ULL << 14,       // "blocking" flag
 };
 
 enum class CommandCategory : uint8_t {
@@ -302,28 +299,24 @@ inline uint64_t ParseCommandFlags(const std::string 
&description, const std::str
       flags |= kCmdWrite;
     else if (flag == "read-only")
       flags |= kCmdReadOnly;
-    else if (flag == "replication")
-      flags |= kCmdReplication;
-    else if (flag == "pub-sub")
-      flags |= kCmdPubSub;
     else if (flag == "ok-loading")
       flags |= kCmdLoading;
     else if (flag == "exclusive")
       flags |= kCmdExclusive;
     else if (flag == "multi")
-      flags |= kCmdMulti;
+      flags |= kCmdEndMulti;
     else if (flag == "no-multi")
       flags |= kCmdNoMulti;
     else if (flag == "no-script")
       flags |= kCmdNoScript;
-    else if (flag == "ro-script")
-      flags |= kCmdROScript;
     else if (flag == "cluster")
       flags |= kCmdCluster;
     else if (flag == "no-dbsize-check")
       flags |= kCmdNoDBSizeCheck;
     else if (flag == "slow")
       flags |= kCmdSlow;
+    else if (flag == "blocking")
+      flags |= kCmdBlocking;
     else {
       std::cout << fmt::format("Encountered non-existent flag '{}' in command 
{} in command attribute parsing", flag,
                                cmd_name)
diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc
index 24ea47b9..584b5068 100644
--- a/src/server/redis_connection.cc
+++ b/src/server/redis_connection.cc
@@ -425,8 +425,11 @@ void Connection::ExecuteCommands(std::deque<CommandTokens> 
*to_process_cmds) {
       concurrency = srv_->WorkConcurrencyGuard();
     }
 
-    if (cmd_flags & kCmdROScript) {
-      // if executing read only lua script commands, set current connection.
+    auto category = attributes->category;
+    if ((category == CommandCategory::Function || category == 
CommandCategory::Script) && (cmd_flags & kCmdReadOnly)) {
+      // FIXME: since read-only script commands are not exclusive,
+      // SetCurrentConnection here is weird and can cause many issues,
+      // we should pass the Connection directly to the lua context instead
       srv_->SetCurrentConnection(this);
     }
 
@@ -471,8 +474,8 @@ void Connection::ExecuteCommands(std::deque<CommandTokens> 
*to_process_cmds) {
       DisableFlag(kAsking);
     }
 
-    // We don't execute commands, but queue them, ant then execute in EXEC 
command
-    if (is_multi_exec && !in_exec_ && !(cmd_flags & kCmdMulti)) {
+    // We don't execute commands, but queue them, and then execute in EXEC 
command
+    if (is_multi_exec && !in_exec_ && !(cmd_flags & kCmdEndMulti)) {
       multi_cmds_.emplace_back(cmd_tokens);
       Reply(redis::SimpleString("QUEUED"));
       continue;

Reply via email to