Merge branch 'unstable' into unstable

src/cluster/cluster.cc

@@ -748,7 +748,7 @@ bool Cluster::IsWriteForbiddenSlot(int slot) { return svr_->slot_migrator->GetFo
 Status Cluster::CanExecByMySelf(const redis::CommandAttributes *attributes, const std::vector<std::string> &cmd_tokens,
                                 redis::Connection *conn) {
   std::vector<int> keys_indexes;
-  auto s = redis::GetKeysFromCommand(attributes, cmd_tokens, &keys_indexes);
+  auto s = redis::CommandTable::GetKeysFromCommand(attributes, cmd_tokens, &keys_indexes);
   // No keys
   if (!s.IsOK()) return Status::OK();
 

src/commands/cmd_cluster.cc

@@ -182,7 +182,7 @@ class CommandClusterX : public Commander {
 
     // CLUSTERX SETSLOT $SLOT_ID NODE $NODE_ID $VERSION
     if (subcommand_ == "setslot" && args_.size() == 6) {
-      Status s = CommanderHelper::ParseSlotRanges(args_[2], slot_ranges_);
+      Status s = CommandTable::ParseSlotRanges(args_[2], slot_ranges_);
       if (!s.IsOK()) {
         return s;
       }

src/commands/cmd_server.cc

@@ -609,7 +609,7 @@ class CommandCommand : public Commander {
  public:
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
     if (args_.size() == 1) {
-      GetAllCommandsInfo(output);
+      CommandTable::GetAllCommandsInfo(output);
     } else {
       std::string sub_command = util::ToLower(args_[1]);
       if ((sub_command == "count" && args_.size() != 2) || (sub_command == "getkeys" && args_.size() < 3) ||
@@ -618,18 +618,18 @@ class CommandCommand : public Commander {
       }
 
       if (sub_command == "count") {
-        *output = redis::Integer(GetCommandNum());
+        *output = redis::Integer(CommandTable::Size());
       } else if (sub_command == "info") {
-        GetCommandsInfo(output, std::vector<std::string>(args_.begin() + 2, args_.end()));
+        CommandTable::GetCommandsInfo(output, std::vector<std::string>(args_.begin() + 2, args_.end()));
       } else if (sub_command == "getkeys") {
-        auto cmd_iter = command_details::original_commands.find(util::ToLower(args_[2]));
-        if (cmd_iter == command_details::original_commands.end()) {
+        auto cmd_iter = CommandTable::GetOriginal()->find(util::ToLower(args_[2]));
+        if (cmd_iter == CommandTable::GetOriginal()->end()) {
           return {Status::RedisUnknownCmd, "Invalid command specified"};
         }
 
         std::vector<int> keys_indexes;
-        auto s = GetKeysFromCommand(cmd_iter->second, std::vector<std::string>(args_.begin() + 2, args_.end()),
-                                    &keys_indexes);
+        auto s = CommandTable::GetKeysFromCommand(
+            cmd_iter->second, std::vector<std::string>(args_.begin() + 2, args_.end()), &keys_indexes);
         if (!s.IsOK()) return s;
 
         if (keys_indexes.size() == 0) {

src/commands/commander.cc

@@ -26,21 +26,21 @@ namespace redis {
 
 RegisterToCommandTable::RegisterToCommandTable(std::initializer_list<CommandAttributes> list) {
   for (const auto &attr : list) {
-    command_details::redis_command_table.emplace_back(attr);
-    command_details::original_commands[attr.name] = &command_details::redis_command_table.back();
-    command_details::commands[attr.name] = &command_details::redis_command_table.back();
+    CommandTable::redis_command_table.emplace_back(attr);
+    CommandTable::original_commands[attr.name] = &CommandTable::redis_command_table.back();
+    CommandTable::commands[attr.name] = &CommandTable::redis_command_table.back();
   }
 }
 
-size_t GetCommandNum() { return command_details::redis_command_table.size(); }
+size_t CommandTable::Size() { return redis_command_table.size(); }
 
-const CommandMap *GetOriginalCommands() { return &command_details::original_commands; }
+const CommandMap *CommandTable::GetOriginal() { return &original_commands; }
 
-CommandMap *GetCommands() { return &command_details::commands; }
+CommandMap *CommandTable::Get() { return &commands; }
 
-void ResetCommands() { command_details::commands = command_details::original_commands; }
+void CommandTable::Reset() { commands = original_commands; }
 
-std::string GetCommandInfo(const CommandAttributes *command_attributes) {
+std::string CommandTable::GetCommandInfo(const CommandAttributes *command_attributes) {
   std::string command, command_flags;
   command.append(redis::MultiLen(6));
   command.append(redis::BulkString(command_attributes->name));
@@ -54,20 +54,20 @@ std::string GetCommandInfo(const CommandAttributes *command_attributes) {
   return command;
 }
 
-void GetAllCommandsInfo(std::string *info) {
-  info->append(redis::MultiLen(command_details::original_commands.size()));
-  for (const auto &iter : command_details::original_commands) {
+void CommandTable::GetAllCommandsInfo(std::string *info) {
+  info->append(redis::MultiLen(original_commands.size()));
+  for (const auto &iter : original_commands) {
     auto command_attribute = iter.second;
     auto command_info = GetCommandInfo(command_attribute);
     info->append(command_info);
   }
 }
 
-void GetCommandsInfo(std::string *info, const std::vector<std::string> &cmd_names) {
+void CommandTable::GetCommandsInfo(std::string *info, const std::vector<std::string> &cmd_names) {
   info->append(redis::MultiLen(cmd_names.size()));
   for (const auto &cmd_name : cmd_names) {
-    auto cmd_iter = command_details::original_commands.find(util::ToLower(cmd_name));
-    if (cmd_iter == command_details::original_commands.end()) {
+    auto cmd_iter = original_commands.find(util::ToLower(cmd_name));
+    if (cmd_iter == original_commands.end()) {
       info->append(redis::NilString());
     } else {
       auto command_attribute = cmd_iter->second;
@@ -86,8 +86,8 @@ void DumpKeyRange(std::vector<int> &keys_index, int argc, const CommandKeyRange
   }
 }
 
-Status GetKeysFromCommand(const CommandAttributes *attributes, const std::vector<std::string> &cmd_tokens,
-                          std::vector<int> *keys_index) {
+Status CommandTable::GetKeysFromCommand(const CommandAttributes *attributes, const std::vector<std::string> &cmd_tokens,
+                                        std::vector<int> *keys_index) {
   if (attributes->key_range.first_key == 0) {
     return {Status::NotOK, "The command has no key arguments"};
   }
@@ -113,11 +113,11 @@ Status GetKeysFromCommand(const CommandAttributes *attributes, const std::vector
   return Status::OK();
 }
 
-bool IsCommandExists(const std::string &name) {
-  return command_details::original_commands.find(util::ToLower(name)) != command_details::original_commands.end();
+bool CommandTable::IsExists(const std::string &name) {
+  return original_commands.find(util::ToLower(name)) != original_commands.end();
 }
 
-Status CommanderHelper::ParseSlotRanges(const std::string &slots_str, std::vector<SlotRange> &slots) {
+Status CommandTable::ParseSlotRanges(const std::string &slots_str, std::vector<SlotRange> &slots) {
   if (slots_str.empty()) {
     return {Status::NotOK, "No slots to parse."};
   }

src/commands/commander.h

@@ -89,11 +89,6 @@ class CommanderWithParseMove : Commander {
   virtual Status ParseMove(std::vector<std::string> &&args) { return Status::OK(); }
 };
 
-class CommanderHelper {
- public:
-  static Status ParseSlotRanges(const std::string &slots_str, std::vector<SlotRange> &slots);
-};
-
 using CommanderFactory = std::function<std::unique_ptr<Commander>()>;
 
 struct CommandKeyRange {
@@ -250,16 +245,36 @@ struct RegisterToCommandTable {
   RegisterToCommandTable(std::initializer_list<CommandAttributes> list);
 };
 
-// these variables cannot be put into source files (to ensure init order for multiple TUs)
-namespace command_details {
-inline std::deque<CommandAttributes> redis_command_table;
+struct CommandTable {
+ public:
+  CommandTable() = delete;
+
+  static CommandMap *Get();
+  static const CommandMap *GetOriginal();
+  static void Reset();
+
+  static void GetAllCommandsInfo(std::string *info);
+  static void GetCommandsInfo(std::string *info, const std::vector<std::string> &cmd_names);
+  static std::string GetCommandInfo(const CommandAttributes *command_attributes);
+  static Status GetKeysFromCommand(const CommandAttributes *attributes, const std::vector<std::string> &cmd_tokens,
+                                   std::vector<int> *keys_indexes);
 
-// Original Command table before rename-command directive
-inline CommandMap original_commands;
+  static size_t Size();
+  static bool IsExists(const std::string &name);
 
-// Command table after rename-command directive
-inline CommandMap commands;
-}  // namespace command_details
+  static Status ParseSlotRanges(const std::string &slots_str, std::vector<SlotRange> &slots);
+
+ private:
+  static inline std::deque<CommandAttributes> redis_command_table;
+
+  // Original Command table before rename-command directive
+  static inline CommandMap original_commands;
+
+  // Command table after rename-command directive
+  static inline CommandMap commands;
+
+  friend struct RegisterToCommandTable;
+};
 
 #define KVROCKS_CONCAT(a, b) a##b                   // NOLINT
 #define KVROCKS_CONCAT2(a, b) KVROCKS_CONCAT(a, b)  // NOLINT
@@ -268,15 +283,4 @@ inline CommandMap commands;
 #define REDIS_REGISTER_COMMANDS(...) \
   static RegisterToCommandTable KVROCKS_CONCAT2(register_to_command_table_, __LINE__){__VA_ARGS__};
 
-size_t GetCommandNum();
-CommandMap *GetCommands();
-void ResetCommands();
-const CommandMap *GetOriginalCommands();
-void GetAllCommandsInfo(std::string *info);
-void GetCommandsInfo(std::string *info, const std::vector<std::string> &cmd_names);
-std::string GetCommandInfo(const CommandAttributes *command_attributes);
-Status GetKeysFromCommand(const CommandAttributes *attributes, const std::vector<std::string> &cmd_tokens,
-                          std::vector<int> *keys_indexes);
-bool IsCommandExists(const std::string &name);
-
 }  // namespace redis

src/config/config.cc

@@ -286,7 +286,7 @@ void Config::initFieldValidator() {
            if (args.size() != 2) {
              return {Status::NotOK, "Invalid rename-command format"};
            }
-           auto commands = redis::GetCommands();
+           auto commands = redis::CommandTable::Get();
            auto cmd_iter = commands->find(util::ToLower(args[0]));
            if (cmd_iter == commands->end()) {
              return {Status::NotOK, "No such command in rename-command"};
@@ -436,7 +436,7 @@ void Config::initFieldCallback() {
              profiling_sample_commands.clear();
              return Status::OK();
            }
-           if (!redis::IsCommandExists(cmd)) {
+           if (!redis::CommandTable::IsExists(cmd)) {
              return {Status::NotOK, cmd + " is not Kvrocks supported command"};
            }
            // profiling_sample_commands use command's original name, regardless of rename-command directive

src/server/server.cc

@@ -56,7 +56,7 @@ constexpr const char *REDIS_VERSION = "4.0.0";
 Server::Server(engine::Storage *storage, Config *config)
     : storage(storage), start_time_(util::GetTimeStamp()), config_(config), namespace_(storage) {
   // init commands stats here to prevent concurrent insert, and cause core
-  auto commands = redis::GetOriginalCommands();
+  auto commands = redis::CommandTable::GetOriginal();
   for (const auto &iter : *commands) {
     stats.commands_stats[iter.first].calls = 0;
     stats.commands_stats[iter.first].latency = 0;
@@ -1509,7 +1509,7 @@ ReplState Server::GetReplicationState() {
 Status Server::LookupAndCreateCommand(const std::string &cmd_name, std::unique_ptr<redis::Commander> *cmd) {
   if (cmd_name.empty()) return {Status::RedisUnknownCmd};
 
-  auto commands = redis::GetCommands();
+  auto commands = redis::CommandTable::Get();
   auto cmd_iter = commands->find(util::ToLower(cmd_name));
   if (cmd_iter == commands->end()) {
     return {Status::RedisUnknownCmd};

src/storage/scripting.cc

@@ -704,7 +704,7 @@ int RedisGenericCommand(lua_State *lua, int raise_error) {
     }
   }
 
-  auto commands = redis::GetCommands();
+  auto commands = redis::CommandTable::Get();
   auto cmd_iter = commands->find(util::ToLower(args[0]));
   if (cmd_iter == commands->end()) {
     PushError(lua, "Unknown Redis command called from Lua script");

tests/cppunit/cluster_test.cc

@@ -217,37 +217,37 @@ TEST(Cluster, ClusterParseSlotRanges) {
   std::vector<SlotRange> slots;
 
   const std::string t_single_slot = "1234";
-  s = redis::CommanderHelper::ParseSlotRanges(t_single_slot, slots);
+  s = redis::CommandTable::ParseSlotRanges(t_single_slot, slots);
   ASSERT_TRUE(s.IsOK());
   s = cluster.SetSlotRanges(slots, node_id, version);
   ASSERT_TRUE(s.IsOK());
   version++;
   slots.clear();
 
   const std::string t_single_ranges = "1234-1236";
-  s = redis::CommanderHelper::ParseSlotRanges(t_single_ranges, slots);
+  s = redis::CommandTable::ParseSlotRanges(t_single_ranges, slots);
   ASSERT_TRUE(s.IsOK());
   s = cluster.SetSlotRanges(slots, node_id, version);
   ASSERT_TRUE(s.IsOK());
   version++;
   slots.clear();
 
   const std::string t_mixed_slot = "10229  16301 4710 3557-8559 ";
-  s = redis::CommanderHelper::ParseSlotRanges(t_mixed_slot, slots);
+  s = redis::CommandTable::ParseSlotRanges(t_mixed_slot, slots);
   ASSERT_TRUE(s.IsOK());
   s = cluster.SetSlotRanges(slots, node_id, version);
   ASSERT_TRUE(s.IsOK());
   version++;
   slots.clear();
 
   std::string empty_slots;
-  s = redis::CommanderHelper::ParseSlotRanges(empty_slots, slots);
+  s = redis::CommandTable::ParseSlotRanges(empty_slots, slots);
   ASSERT_FALSE(s.IsOK());
   ASSERT_TRUE(s.Msg() == "No slots to parse.");
   slots.clear();
 
   std::string space_slots = "    ";
-  s = redis::CommanderHelper::ParseSlotRanges(space_slots, slots);
+  s = redis::CommandTable::ParseSlotRanges(space_slots, slots);
   ASSERT_FALSE(s.IsOK());
   ASSERT_TRUE(s.Msg() == fmt::format("Invalid slots: `{}`. No slots to parse. "
                                      "Please use spaces to separate slots.",
@@ -278,44 +278,44 @@ TEST(Cluster, ClusterParseSlotRanges) {
       }
     }
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[0], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[0], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(s.Msg() == "Invalid slot id: encounter non-integer characters");
     slots.clear();
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[1], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[1], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(s.Msg() == "Invalid slot id: out of numeric range");
     slots.clear();
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[2], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[2], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(s.Msg() == fmt::format("Invalid slot range: `{}`. The character '-' can't appear "
                                        "in the first or last position.",
                                        front_slot_ranges));
     slots.clear();
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[3], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[3], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(s.Msg() ==
                 fmt::format("Invalid slot range: `{}`. The character '-' can't appear in the first or last position.",
                             back_slot_ranges));
     slots.clear();
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[4], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[4], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(s.Msg() ==
                 fmt::format("Invalid slot range: `{}`. The character '-' can't appear in the first or last position.",
                             f_single_slot));
     slots.clear();
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[5], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[5], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(s.Msg() == fmt::format("Invalid slot range: `{}`. The slot range should be of the form `int1-int2`.",
                                        overmuch_slot_ranges));
     slots.clear();
 
-    s = redis::CommanderHelper::ParseSlotRanges(error_slots[6], slots);
+    s = redis::CommandTable::ParseSlotRanges(error_slots[6], slots);
     ASSERT_FALSE(s.IsOK());
     ASSERT_TRUE(
         s.Msg() ==

tests/cppunit/config_test.cc

@@ -145,7 +145,7 @@ TEST(Config, GetRenameCommand) {
   output_file << "rename-command SET SET_NEW"
               << "\n";
   output_file.close();
-  redis::ResetCommands();
+  redis::CommandTable::Reset();
   Config config;
   ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
   std::vector<std::string> values;
@@ -171,12 +171,12 @@ TEST(Config, Rewrite) {
               << "\n";
   output_file.close();
 
-  redis::ResetCommands();
+  redis::CommandTable::Reset();
   Config config;
   ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
   ASSERT_TRUE(config.Rewrite({}).IsOK());
   // Need to re-populate the command table since it has renamed by the previous
-  redis::ResetCommands();
+  redis::CommandTable::Reset();
   Config new_config;
   ASSERT_TRUE(new_config.Load(CLIOptions(path)).IsOK());
   unlink(path);