From c525d7a8c6ba19f5b7e53726319cc578767a62d9 Mon Sep 17 00:00:00 2001 From: Twice Date: Thu, 24 Oct 2024 17:58:16 +0800 Subject: [PATCH] feat(cmd): avoid to use CommandAttributes::flags directly (#2619) --- src/cluster/cluster.cc | 6 +- src/commands/cmd_cluster.cc | 10 ++-- src/commands/cmd_function.cc | 2 +- src/commands/cmd_geo.cc | 2 +- src/commands/cmd_pubsub.cc | 18 +++--- src/commands/cmd_replication.cc | 10 ++-- src/commands/cmd_script.cc | 2 +- src/commands/cmd_search.cc | 20 +++---- src/commands/cmd_server.cc | 74 ++++++++++++------------ src/commands/cmd_stream.cc | 6 +- src/commands/cmd_txn.cc | 8 +-- src/commands/commander.cc | 2 +- src/commands/commander.h | 99 ++++++++++++++++++++++++--------- src/server/redis_connection.cc | 10 ++-- src/server/server.cc | 2 +- 15 files changed, 161 insertions(+), 110 deletions(-) diff --git a/src/cluster/cluster.cc b/src/cluster/cluster.cc index 95b076aa07f..16e72b62fbc 100644 --- a/src/cluster/cluster.cc +++ b/src/cluster/cluster.cc @@ -871,6 +871,8 @@ Status Cluster::CanExecByMySelf(const redis::CommandAttributes *attributes, cons cross_slot_ok = true; } + uint64_t flags = attributes->GenerateFlags(cmd_tokens); + if (myself_ && myself_ == slots_nodes_[slot]) { // We use central controller to manage the topology of the cluster. // Server can't change the topology directly, so we record the migrated slots @@ -880,7 +882,7 @@ Status Cluster::CanExecByMySelf(const redis::CommandAttributes *attributes, cons } // To keep data consistency, slot will be forbidden write while sending the last incremental data. // During this phase, the requests of the migrating slot has to be rejected. - if ((attributes->flags & redis::kCmdWrite) && IsWriteForbiddenSlot(slot)) { + if ((flags & redis::kCmdWrite) && IsWriteForbiddenSlot(slot)) { return {Status::RedisTryAgain, "Can't write to slot being migrated which is in write forbidden phase"}; } @@ -903,7 +905,7 @@ Status Cluster::CanExecByMySelf(const redis::CommandAttributes *attributes, cons return Status::OK(); // I'm serving the imported slot } - if (myself_ && myself_->role == kClusterSlave && !(attributes->flags & redis::kCmdWrite) && + if (myself_ && myself_->role == kClusterSlave && !(flags & redis::kCmdWrite) && nodes_.find(myself_->master_id) != nodes_.end() && nodes_[myself_->master_id] == slots_nodes_[slot] && conn->IsFlagEnabled(redis::Connection::kReadOnly)) { return Status::OK(); // My master is serving this slot diff --git a/src/commands/cmd_cluster.cc b/src/commands/cmd_cluster.cc index 11f604690ab..ab23f86d2a5 100644 --- a/src/commands/cmd_cluster.cc +++ b/src/commands/cmd_cluster.cc @@ -358,10 +358,10 @@ class CommandAsking : public Commander { }; REDIS_REGISTER_COMMANDS(Cluster, - MakeCmdAttr("cluster", -2, "cluster no-script", 0, 0, 0, GenerateClusterFlag), - MakeCmdAttr("clusterx", -2, "cluster no-script", 0, 0, 0, GenerateClusterFlag), - MakeCmdAttr("readonly", 1, "cluster no-multi", 0, 0, 0), - MakeCmdAttr("readwrite", 1, "cluster no-multi", 0, 0, 0), - MakeCmdAttr("asking", 1, "cluster", 0, 0, 0), ) + MakeCmdAttr("cluster", -2, "cluster no-script", NO_KEY, GenerateClusterFlag), + MakeCmdAttr("clusterx", -2, "cluster no-script", NO_KEY, GenerateClusterFlag), + MakeCmdAttr("readonly", 1, "cluster no-multi", NO_KEY), + MakeCmdAttr("readwrite", 1, "cluster no-multi", NO_KEY), + MakeCmdAttr("asking", 1, "cluster", NO_KEY), ) } // namespace redis diff --git a/src/commands/cmd_function.cc b/src/commands/cmd_function.cc index 53fbc0c03f4..ee5a580d314 100644 --- a/src/commands/cmd_function.cc +++ b/src/commands/cmd_function.cc @@ -110,7 +110,7 @@ uint64_t GenerateFunctionFlags(uint64_t flags, const std::vector &a } REDIS_REGISTER_COMMANDS( - Function, MakeCmdAttr("function", -2, "exclusive no-script", 0, 0, 0, GenerateFunctionFlags), + Function, MakeCmdAttr("function", -2, "exclusive no-script", NO_KEY, GenerateFunctionFlags), MakeCmdAttr>("fcall", -3, "exclusive write no-script", GetScriptEvalKeyRange), MakeCmdAttr>("fcall_ro", -3, "read-only ro-script no-script", GetScriptEvalKeyRange)); diff --git a/src/commands/cmd_geo.cc b/src/commands/cmd_geo.cc index 3cf41c84e91..4c354611581 100644 --- a/src/commands/cmd_geo.cc +++ b/src/commands/cmd_geo.cc @@ -278,7 +278,7 @@ class CommandGeoRadius : public CommandGeoBase { count_ = *parse_result; i += 2; - } else if ((attributes_->flags & kCmdWrite) && + } else if ((attributes_->InitialFlags() & kCmdWrite) && (util::ToLower(args_[i]) == "store" || util::ToLower(args_[i]) == "storedist") && i + 1 < args_.size()) { store_key_ = args_[i + 1]; diff --git a/src/commands/cmd_pubsub.cc b/src/commands/cmd_pubsub.cc index 346dfebdd8a..e7e4dee7826 100644 --- a/src/commands/cmd_pubsub.cc +++ b/src/commands/cmd_pubsub.cc @@ -253,14 +253,14 @@ class CommandPubSub : public Commander { }; REDIS_REGISTER_COMMANDS( - Pubsub, MakeCmdAttr("publish", 3, "read-only pub-sub", 0, 0, 0), - MakeCmdAttr("mpublish", -3, "read-only pub-sub", 0, 0, 0), - MakeCmdAttr("subscribe", -2, "read-only pub-sub no-multi no-script", 0, 0, 0), - MakeCmdAttr("unsubscribe", -1, "read-only pub-sub no-multi no-script", 0, 0, 0), - MakeCmdAttr("psubscribe", -2, "read-only pub-sub no-multi no-script", 0, 0, 0), - MakeCmdAttr("punsubscribe", -1, "read-only pub-sub no-multi no-script", 0, 0, 0), - MakeCmdAttr("ssubscribe", -2, "read-only pub-sub no-multi no-script", 0, 0, 0), - MakeCmdAttr("sunsubscribe", -1, "read-only pub-sub no-multi no-script", 0, 0, 0), - MakeCmdAttr("pubsub", -2, "read-only pub-sub no-script", 0, 0, 0), ) + Pubsub, MakeCmdAttr("publish", 3, "read-only pub-sub", NO_KEY), + MakeCmdAttr("mpublish", -3, "read-only pub-sub", NO_KEY), + MakeCmdAttr("subscribe", -2, "read-only pub-sub no-multi no-script", NO_KEY), + MakeCmdAttr("unsubscribe", -1, "read-only pub-sub no-multi no-script", NO_KEY), + MakeCmdAttr("psubscribe", -2, "read-only pub-sub no-multi no-script", NO_KEY), + MakeCmdAttr("punsubscribe", -1, "read-only pub-sub no-multi no-script", NO_KEY), + MakeCmdAttr("ssubscribe", -2, "read-only pub-sub no-multi no-script", NO_KEY), + MakeCmdAttr("sunsubscribe", -1, "read-only pub-sub no-multi no-script", NO_KEY), + MakeCmdAttr("pubsub", -2, "read-only pub-sub no-script", NO_KEY), ) } // namespace redis diff --git a/src/commands/cmd_replication.cc b/src/commands/cmd_replication.cc index 387b212741a..71fd7fb626d 100644 --- a/src/commands/cmd_replication.cc +++ b/src/commands/cmd_replication.cc @@ -345,10 +345,10 @@ class CommandDBName : public Commander { }; REDIS_REGISTER_COMMANDS( - Replication, MakeCmdAttr("replconf", -3, "read-only replication no-script", 0, 0, 0), - MakeCmdAttr("psync", -2, "read-only replication no-multi no-script", 0, 0, 0), - MakeCmdAttr("_fetch_meta", 1, "read-only replication no-multi no-script", 0, 0, 0), - MakeCmdAttr("_fetch_file", 2, "read-only replication no-multi no-script", 0, 0, 0), - MakeCmdAttr("_db_name", 1, "read-only replication no-multi", 0, 0, 0), ) + Replication, MakeCmdAttr("replconf", -3, "read-only replication no-script", NO_KEY), + MakeCmdAttr("psync", -2, "read-only replication no-multi no-script", NO_KEY), + MakeCmdAttr("_fetch_meta", 1, "read-only replication no-multi no-script", NO_KEY), + MakeCmdAttr("_fetch_file", 2, "read-only replication no-multi no-script", NO_KEY), + MakeCmdAttr("_db_name", 1, "read-only replication no-multi", NO_KEY), ) } // namespace redis diff --git a/src/commands/cmd_script.cc b/src/commands/cmd_script.cc index 1eb85705ee4..40c19a17ecf 100644 --- a/src/commands/cmd_script.cc +++ b/src/commands/cmd_script.cc @@ -130,6 +130,6 @@ REDIS_REGISTER_COMMANDS( MakeCmdAttr("evalsha", -3, "exclusive write no-script", GetScriptEvalKeyRange), MakeCmdAttr("eval_ro", -3, "read-only no-script ro-script", GetScriptEvalKeyRange), MakeCmdAttr("evalsha_ro", -3, "read-only no-script ro-script", GetScriptEvalKeyRange), - MakeCmdAttr("script", -2, "exclusive no-script", 0, 0, 0), ) + MakeCmdAttr("script", -2, "exclusive no-script", NO_KEY), ) } // namespace redis diff --git a/src/commands/cmd_search.cc b/src/commands/cmd_search.cc index 5d9e220e738..f6d5ab05e25 100644 --- a/src/commands/cmd_search.cc +++ b/src/commands/cmd_search.cc @@ -509,15 +509,15 @@ class CommandFTTagVals : public Commander { }; REDIS_REGISTER_COMMANDS(Search, - MakeCmdAttr("ft.create", -2, "write exclusive no-multi no-script slow", 0, 0, - 0), - MakeCmdAttr("ft.searchsql", -2, "read-only", 0, 0, 0), - MakeCmdAttr("ft.search", -3, "read-only", 0, 0, 0), - MakeCmdAttr("ft.explainsql", -2, "read-only", 0, 0, 0), - MakeCmdAttr("ft.explain", -3, "read-only", 0, 0, 0), - MakeCmdAttr("ft.info", 2, "read-only", 0, 0, 0), - MakeCmdAttr("ft._list", 1, "read-only", 0, 0, 0), - MakeCmdAttr("ft.dropindex", 2, "write exclusive no-multi no-script", 0, 0, 0), - MakeCmdAttr("ft.tagvals", 3, "read-only slow", 0, 0, 0)); + MakeCmdAttr("ft.create", -2, "write exclusive no-multi no-script slow", + NO_KEY), + MakeCmdAttr("ft.searchsql", -2, "read-only", NO_KEY), + MakeCmdAttr("ft.search", -3, "read-only", NO_KEY), + MakeCmdAttr("ft.explainsql", -2, "read-only", NO_KEY), + MakeCmdAttr("ft.explain", -3, "read-only", NO_KEY), + MakeCmdAttr("ft.info", 2, "read-only", NO_KEY), + MakeCmdAttr("ft._list", 1, "read-only", NO_KEY), + MakeCmdAttr("ft.dropindex", 2, "write exclusive no-multi no-script", NO_KEY), + MakeCmdAttr("ft.tagvals", 3, "read-only slow", NO_KEY)); } // namespace redis diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc index 66637f4f86e..260edca1ebc 100644 --- a/src/commands/cmd_server.cc +++ b/src/commands/cmd_server.cc @@ -1331,43 +1331,43 @@ class CommandPollUpdates : public Commander { Format format_ = Format::Raw; }; -REDIS_REGISTER_COMMANDS(Server, MakeCmdAttr("auth", 2, "read-only ok-loading", 0, 0, 0), - MakeCmdAttr("ping", -1, "read-only", 0, 0, 0), - MakeCmdAttr("select", 2, "read-only", 0, 0, 0), - MakeCmdAttr("info", -1, "read-only ok-loading", 0, 0, 0), - MakeCmdAttr("role", 1, "read-only ok-loading", 0, 0, 0), - MakeCmdAttr("config", -2, "read-only", 0, 0, 0, GenerateConfigFlag), - MakeCmdAttr("namespace", -3, "read-only", 0, 0, 0), - MakeCmdAttr("keys", 2, "read-only slow", 0, 0, 0), - MakeCmdAttr("flushdb", 1, "write no-dbsize-check", 0, 0, 0), - MakeCmdAttr("flushall", 1, "write no-dbsize-check", 0, 0, 0), - MakeCmdAttr("dbsize", -1, "read-only", 0, 0, 0), - MakeCmdAttr("slowlog", -2, "read-only", 0, 0, 0), - MakeCmdAttr("perflog", -2, "read-only", 0, 0, 0), - MakeCmdAttr("client", -2, "read-only", 0, 0, 0), - MakeCmdAttr("monitor", 1, "read-only no-multi", 0, 0, 0), - MakeCmdAttr("shutdown", 1, "read-only no-multi no-script", 0, 0, 0), - MakeCmdAttr("quit", 1, "read-only", 0, 0, 0), - MakeCmdAttr("scan", -2, "read-only", 0, 0, 0), - MakeCmdAttr("randomkey", 1, "read-only", 0, 0, 0), - MakeCmdAttr("debug", -2, "read-only exclusive", 0, 0, 0), - MakeCmdAttr("command", -1, "read-only", 0, 0, 0), - MakeCmdAttr("echo", 2, "read-only", 0, 0, 0), - MakeCmdAttr("time", 1, "read-only ok-loading", 0, 0, 0), - MakeCmdAttr("disk", 3, "read-only", 0, 0, 0), - MakeCmdAttr("memory", 3, "read-only", 0, 0, 0), - MakeCmdAttr("hello", -1, "read-only ok-loading", 0, 0, 0), +REDIS_REGISTER_COMMANDS(Server, MakeCmdAttr("auth", 2, "read-only ok-loading", NO_KEY), + MakeCmdAttr("ping", -1, "read-only", NO_KEY), + MakeCmdAttr("select", 2, "read-only", NO_KEY), + MakeCmdAttr("info", -1, "read-only ok-loading", NO_KEY), + MakeCmdAttr("role", 1, "read-only ok-loading", NO_KEY), + MakeCmdAttr("config", -2, "read-only", NO_KEY, GenerateConfigFlag), + MakeCmdAttr("namespace", -3, "read-only", NO_KEY), + MakeCmdAttr("keys", 2, "read-only slow", NO_KEY), + MakeCmdAttr("flushdb", 1, "write no-dbsize-check", NO_KEY), + MakeCmdAttr("flushall", 1, "write no-dbsize-check", NO_KEY), + MakeCmdAttr("dbsize", -1, "read-only", NO_KEY), + MakeCmdAttr("slowlog", -2, "read-only", NO_KEY), + MakeCmdAttr("perflog", -2, "read-only", NO_KEY), + MakeCmdAttr("client", -2, "read-only", NO_KEY), + MakeCmdAttr("monitor", 1, "read-only no-multi", NO_KEY), + MakeCmdAttr("shutdown", 1, "read-only no-multi no-script", NO_KEY), + MakeCmdAttr("quit", 1, "read-only", NO_KEY), + MakeCmdAttr("scan", -2, "read-only", NO_KEY), + MakeCmdAttr("randomkey", 1, "read-only", NO_KEY), + MakeCmdAttr("debug", -2, "read-only exclusive", NO_KEY), + MakeCmdAttr("command", -1, "read-only", NO_KEY), + MakeCmdAttr("echo", 2, "read-only", NO_KEY), + MakeCmdAttr("time", 1, "read-only ok-loading", NO_KEY), + MakeCmdAttr("disk", 3, "read-only", NO_KEY), + MakeCmdAttr("memory", 3, "read-only", NO_KEY), + MakeCmdAttr("hello", -1, "read-only ok-loading", NO_KEY), MakeCmdAttr("restore", -4, "write", 1, 1, 1), - MakeCmdAttr("compact", 1, "read-only no-script", 0, 0, 0), - MakeCmdAttr("bgsave", 1, "read-only no-script", 0, 0, 0), - MakeCmdAttr("lastsave", 1, "read-only", 0, 0, 0), - MakeCmdAttr("flushbackup", 1, "read-only no-script", 0, 0, 0), - MakeCmdAttr("slaveof", 3, "read-only exclusive no-script", 0, 0, 0), - MakeCmdAttr("stats", 1, "read-only", 0, 0, 0), - MakeCmdAttr("rdb", -3, "write exclusive", 0, 0, 0), - MakeCmdAttr("reset", 1, "ok-loading multi no-script pub-sub", 0, 0, 0), - MakeCmdAttr("applybatch", -2, "write no-multi", 0, 0, 0), - MakeCmdAttr("dump", 2, "read-only", 0, 0, 0), - MakeCmdAttr("pollupdates", -2, "read-only", 0, 0, 0), ) + MakeCmdAttr("compact", 1, "read-only no-script", NO_KEY), + MakeCmdAttr("bgsave", 1, "read-only no-script", NO_KEY), + MakeCmdAttr("lastsave", 1, "read-only", NO_KEY), + MakeCmdAttr("flushbackup", 1, "read-only no-script", NO_KEY), + MakeCmdAttr("slaveof", 3, "read-only exclusive no-script", NO_KEY), + MakeCmdAttr("stats", 1, "read-only", NO_KEY), + MakeCmdAttr("rdb", -3, "write exclusive", NO_KEY), + MakeCmdAttr("reset", 1, "ok-loading multi no-script pub-sub", NO_KEY), + MakeCmdAttr("applybatch", -2, "write no-multi", NO_KEY), + MakeCmdAttr("dump", 2, "read-only", NO_KEY), + MakeCmdAttr("pollupdates", -2, "read-only", NO_KEY), ) } // namespace redis diff --git a/src/commands/cmd_stream.cc b/src/commands/cmd_stream.cc index 3345c3b8461..903b0031c5d 100644 --- a/src/commands/cmd_stream.cc +++ b/src/commands/cmd_stream.cc @@ -1884,12 +1884,12 @@ REDIS_REGISTER_COMMANDS(Stream, MakeCmdAttr("xack", -4, "write no-d MakeCmdAttr("xautoclaim", -6, "write", 1, 1, 1), MakeCmdAttr("xgroup", -4, "write", 2, 2, 1), MakeCmdAttr("xlen", -2, "read-only", 1, 1, 1), - MakeCmdAttr("xinfo", -2, "read-only", 0, 0, 0), + MakeCmdAttr("xinfo", -2, "read-only", NO_KEY), MakeCmdAttr("xpending", -3, "read-only", 1, 1, 1), MakeCmdAttr("xrange", -4, "read-only", 1, 1, 1), MakeCmdAttr("xrevrange", -2, "read-only", 1, 1, 1), - MakeCmdAttr("xread", -4, "read-only", 0, 0, 0), - MakeCmdAttr("xreadgroup", -7, "write", 0, 0, 0), + MakeCmdAttr("xread", -4, "read-only", NO_KEY), + MakeCmdAttr("xreadgroup", -7, "write", NO_KEY), MakeCmdAttr("xtrim", -4, "write no-dbsize-check", 1, 1, 1), MakeCmdAttr("xsetid", -3, "write", 1, 1, 1)) diff --git a/src/commands/cmd_txn.cc b/src/commands/cmd_txn.cc index 2984d621c14..92113fee222 100644 --- a/src/commands/cmd_txn.cc +++ b/src/commands/cmd_txn.cc @@ -119,10 +119,10 @@ class CommandUnwatch : public Commander { } }; -REDIS_REGISTER_COMMANDS(Txn, MakeCmdAttr("multi", 1, "multi", 0, 0, 0), - MakeCmdAttr("discard", 1, "multi", 0, 0, 0), - MakeCmdAttr("exec", 1, "exclusive multi slow", 0, 0, 0), +REDIS_REGISTER_COMMANDS(Txn, MakeCmdAttr("multi", 1, "multi", NO_KEY), + MakeCmdAttr("discard", 1, "multi", NO_KEY), + MakeCmdAttr("exec", 1, "exclusive multi slow", NO_KEY), MakeCmdAttr("watch", -2, "multi", 1, -1, 1), - MakeCmdAttr("unwatch", 1, "multi", 0, 0, 0), ) + MakeCmdAttr("unwatch", 1, "multi", NO_KEY), ) } // namespace redis diff --git a/src/commands/commander.cc b/src/commands/commander.cc index d9c68d2d810..ecab960f35a 100644 --- a/src/commands/commander.cc +++ b/src/commands/commander.cc @@ -48,7 +48,7 @@ std::string CommandTable::GetCommandInfo(const CommandAttributes *command_attrib command.append(redis::BulkString(command_attributes->name)); command.append(redis::Integer(command_attributes->arity)); command_flags.append(redis::MultiLen(1)); - command_flags.append(redis::BulkString(command_attributes->flags & kCmdWrite ? "write" : "readonly")); + command_flags.append(redis::BulkString(command_attributes->InitialFlags() & kCmdWrite ? "write" : "readonly")); command.append(command_flags); command.append(redis::Integer(command_attributes->key_range.first_key)); command.append(redis::Integer(command_attributes->key_range.last_key)); diff --git a/src/commands/commander.h b/src/commands/commander.h index b0389d44eb0..3c09fe87957 100644 --- a/src/commands/commander.h +++ b/src/commands/commander.h @@ -152,7 +152,58 @@ using CommandKeyRangeVecGen = std::function(const s using AdditionalFlagGen = std::function &)>; +struct NoKeyInThisCommand {}; +static constexpr const NoKeyInThisCommand NO_KEY{}; + struct CommandAttributes { + CommandAttributes(std::string name, int arity, CommandCategory category, uint64_t flags, AdditionalFlagGen flag_gen, + NoKeyInThisCommand, CommanderFactory factory) + : name(std::move(name)), + arity(arity), + category(category), + key_range{0, 0, 0}, + factory(std::move(factory)), + flags_(flags), + flag_gen_(std::move(flag_gen)) {} + + CommandAttributes(std::string name, int arity, CommandCategory category, uint64_t flags, AdditionalFlagGen flag_gen, + CommandKeyRange key_range, CommanderFactory factory) + : name(std::move(name)), + arity(arity), + category(category), + key_range(key_range), + factory(std::move(factory)), + flags_(flags), + flag_gen_(std::move(flag_gen)) { + if (key_range.first_key <= 0 || key_range.key_step <= 0 || + (key_range.last_key >= 0 && key_range.last_key < key_range.first_key)) { + std::cout << fmt::format("Encountered invalid key range in command {}", this->name) << std::endl; + std::abort(); + } + } + + CommandAttributes(std::string name, int arity, CommandCategory category, uint64_t flags, AdditionalFlagGen flag_gen, + CommandKeyRangeGen key_range, CommanderFactory factory) + : name(std::move(name)), + arity(arity), + category(category), + key_range{-1, 0, 0}, + key_range_gen(std::move(key_range)), + factory(std::move(factory)), + flags_(flags), + flag_gen_(std::move(flag_gen)) {} + + CommandAttributes(std::string name, int arity, CommandCategory category, uint64_t flags, AdditionalFlagGen flag_gen, + CommandKeyRangeVecGen key_range, CommanderFactory factory) + : name(std::move(name)), + arity(arity), + category(category), + key_range{-2, 0, 0}, + key_range_vec_gen(std::move(key_range)), + factory(std::move(factory)), + flags_(flags), + flag_gen_(std::move(flag_gen)) {} + // command name std::string name; @@ -164,13 +215,8 @@ struct CommandAttributes { // category of this command, e.g. key, string, hash CommandCategory category; - // bitmap of enum CommandFlags - uint64_t flags; - - // additional flags regarding to dynamic command arguments - AdditionalFlagGen flag_gen; - // static determined key range + // if key_range.first_key == 0, there's no key in this command args CommandKeyRange key_range; // if key_range.first_key == -1, key_range_gen is used instead @@ -182,9 +228,11 @@ struct CommandAttributes { // commander object generator CommanderFactory factory; + uint64_t InitialFlags() const { return flags_; } + auto GenerateFlags(const std::vector &args) const { - uint64_t res = flags; - if (flag_gen) res = flag_gen(res, args); + uint64_t res = flags_; + if (flag_gen_) res = flag_gen_(res, args); return res; } @@ -212,6 +260,13 @@ struct CommandAttributes { } } } + + private: + // bitmap of enum CommandFlags + uint64_t flags_; + + // additional flags regarding to dynamic command arguments + AdditionalFlagGen flag_gen_; }; using CommandMap = std::map; @@ -257,23 +312,21 @@ inline uint64_t ParseCommandFlags(const std::string &description, const std::str return flags; } +template +auto MakeCmdAttr(const std::string &name, int arity, const std::string &description, NoKeyInThisCommand no_key, + const AdditionalFlagGen &flag_gen = {}) { + CommandAttributes attr(name, arity, CommandCategory::Unknown, ParseCommandFlags(description, name), flag_gen, no_key, + []() -> std::unique_ptr { return std::unique_ptr(new T()); }); + + return attr; +} + template auto MakeCmdAttr(const std::string &name, int arity, const std::string &description, int first_key, int last_key, int key_step = 1, const AdditionalFlagGen &flag_gen = {}) { - CommandAttributes attr{name, - arity, - CommandCategory::Unknown, - ParseCommandFlags(description, name), - flag_gen, + CommandAttributes attr(name, arity, CommandCategory::Unknown, ParseCommandFlags(description, name), flag_gen, {first_key, last_key, key_step}, - {}, - {}, - []() -> std::unique_ptr { return std::unique_ptr(new T()); }}; - - if ((first_key > 0 && key_step <= 0) || (first_key > 0 && last_key >= 0 && last_key < first_key)) { - std::cout << fmt::format("Encountered invalid key range in command {}", name) << std::endl; - std::abort(); - } + []() -> std::unique_ptr { return std::unique_ptr(new T()); }); return attr; } @@ -286,9 +339,7 @@ auto MakeCmdAttr(const std::string &name, int arity, const std::string &descript CommandCategory::Unknown, ParseCommandFlags(description, name), flag_gen, - {-1, 0, 0}, gen, - {}, []() -> std::unique_ptr { return std::unique_ptr(new T()); }}; return attr; @@ -302,8 +353,6 @@ auto MakeCmdAttr(const std::string &name, int arity, const std::string &descript CommandCategory::Unknown, ParseCommandFlags(description, name), flag_gen, - {-2, 0, 0}, - {}, vec_gen, []() -> std::unique_ptr { return std::unique_ptr(new T()); }}; diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc index 6647c308528..24ea47b9051 100644 --- a/src/server/redis_connection.cc +++ b/src/server/redis_connection.cc @@ -361,10 +361,9 @@ Status Connection::ExecuteCommand(engine::Context &ctx, const std::string &cmd_n return s; } -static bool IsCmdForIndexing(const CommandAttributes *attr) { - return (attr->flags & redis::kCmdWrite) && - (attr->category == CommandCategory::Hash || attr->category == CommandCategory::JSON || - attr->category == CommandCategory::Key); +static bool IsCmdForIndexing(uint64_t cmd_flags, CommandCategory cmd_cat) { + return (cmd_flags & redis::kCmdWrite) && + (cmd_cat == CommandCategory::Hash || cmd_cat == CommandCategory::JSON || cmd_cat == CommandCategory::Key); } void Connection::ExecuteCommands(std::deque *to_process_cmds) { @@ -503,7 +502,8 @@ void Connection::ExecuteCommands(std::deque *to_process_cmds) { // TODO: transaction support for index recording std::vector index_records; - if (!srv_->index_mgr.index_map.empty() && IsCmdForIndexing(attributes) && !config->cluster_enabled) { + if (!srv_->index_mgr.index_map.empty() && IsCmdForIndexing(cmd_flags, attributes->category) && + !config->cluster_enabled) { attributes->ForEachKeyRange( [&, this](const std::vector &args, const CommandKeyRange &key_range) { key_range.ForEachKey( diff --git a/src/server/server.cc b/src/server/server.cc index f004a19ecae..f20b6824fc7 100644 --- a/src/server/server.cc +++ b/src/server/server.cc @@ -1983,7 +1983,7 @@ void Server::updateAllWatchedKeys() { } void Server::UpdateWatchedKeysFromArgs(const std::vector &args, const redis::CommandAttributes &attr) { - if ((attr.flags & redis::kCmdWrite) && watched_key_size_ > 0) { + if ((attr.GenerateFlags(args) & redis::kCmdWrite) && watched_key_size_ > 0) { if (attr.key_range.first_key > 0) { updateWatchedKeysFromRange(args, attr.key_range); } else if (attr.key_range.first_key == -1) {