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;