Skip to content

Commit

Permalink
feat(cmd): add blocking flag and remove useless flags (#2637)
Browse files Browse the repository at this point in the history 
Co-authored-by: hulk <hulk.website@gmail.com>
  • Loading branch information
@PragmaTwice @git-hulk
PragmaTwice and git-hulk authored Nov 2, 2024
1 parent 8cf3162 commit 7a3cc8c
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 52 deletions.
9 changes: 5 additions & 4 deletions src/commands/cmd_function.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 5 additions & 4 deletions src/commands/cmd_list.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
19 changes: 9 additions & 10 deletions src/commands/cmd_pubsub.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 5 additions & 6 deletions src/commands/cmd_replication.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 6 additions & 6 deletions src/commands/cmd_script.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/commands/cmd_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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), )
Expand Down
4 changes: 2 additions & 2 deletions src/commands/cmd_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
6 changes: 3 additions & 3 deletions src/commands/cmd_zset.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
17 changes: 5 additions & 12 deletions src/commands/commander.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 7 additions & 4 deletions src/server/redis_connection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7a3cc8c

Please sign in to comment.