feat(commands): introduce subcommand as a primary abstraction

.github/workflows/kvrocks.yaml

@@ -326,8 +326,8 @@ jobs:
           curl -O https://download.redis.io/releases/redis-6.2.14.tar.gz
           tar -xzvf redis-6.2.14.tar.gz
           mkdir -p $HOME/local/bin
-          pushd redis-6.2.14 && BUILD_TLS=yes make -j$NPROC redis-cli && mv src/redis-cli $HOME/local/bin/ && popd
-          pushd redis-6.2.14 && BUILD_TLS=yes make -j$NPROC redis-server && mv src/redis-server $HOME/local/bin/ && popd
+          pushd redis-6.2.14 && USE_JEMALLOC=no BUILD_TLS=yes make -j$NPROC redis-cli && mv src/redis-cli $HOME/local/bin/ && popd
+          pushd redis-6.2.14 && USE_JEMALLOC=no BUILD_TLS=yes make -j$NPROC redis-server && mv src/redis-server $HOME/local/bin/ && popd
 
       - uses: actions/checkout@v6
         with:

src/commands/cmd_server.cc

@@ -40,6 +40,17 @@
 
 namespace redis {
 
+namespace {
+
+bool IsNamespaceCommandDisabled(Server *srv) { return srv->GetConfig()->redis_databases > 0; }
+
+bool IsNamespaceReadOnlyOnSlave(Server *srv) {
+  Config *config = srv->GetConfig();
+  return config->repl_namespace_enabled && config->IsSlave();
+}
+
+}  // namespace
+
 class CommandAuth : public Commander {
  public:
   Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override {
@@ -65,63 +76,113 @@ class CommandAuth : public Commander {
 };
 
 class CommandNamespace : public Commander {
+ public:
+  Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, [[maybe_unused]] Connection *conn,
+                 [[maybe_unused]] std::string *output) override {
+    if (IsNamespaceCommandDisabled(srv)) {
+      return {Status::RedisExecErr, "namespace command is not allowed when redis-databases > 0"};
+    }
+
+    return {Status::RedisExecErr, "NAMESPACE subcommand must be one of GET, SET, DEL, ADD and CURRENT"};
+  }
+};
+
+class CommandNamespaceGet : public Commander {
  public:
   Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override {
     Config *config = srv->GetConfig();
-    std::string sub_command = util::ToLower(args_[1]);
-    if (config->repl_namespace_enabled && config->IsSlave() && sub_command != "get") {
-      return {Status::RedisExecErr, "namespace is read-only for slave"};
-    }
-    if (config->redis_databases > 0) {
+    if (IsNamespaceCommandDisabled(srv)) {
       return {Status::RedisExecErr, "namespace command is not allowed when redis-databases > 0"};
     }
-    if (args_.size() == 3 && sub_command == "get") {
-      if (args_[2] == "*") {
-        std::vector<std::string> namespaces;
-        auto tokens = srv->GetNamespace()->List();
-        for (auto &token : tokens) {
-          namespaces.emplace_back(token.second);  // namespace
-          namespaces.emplace_back(token.first);   // token
-        }
-        namespaces.emplace_back(kDefaultNamespace);
-        namespaces.emplace_back(config->requirepass);
-        *output = ArrayOfBulkStrings(namespaces);
-      } else {
-        auto token = srv->GetNamespace()->Get(args_[2]);
-        if (token.Is<Status::NotFound>()) {
-          *output = conn->NilString();
-        } else {
-          *output = redis::BulkString(token.GetValue());
-        }
+
+    if (args_[2] == "*") {
+      std::vector<std::string> namespaces;
+      auto tokens = srv->GetNamespace()->List();
+      for (auto &token : tokens) {
+        namespaces.emplace_back(token.second);
+        namespaces.emplace_back(token.first);
       }
-    } else if (args_.size() == 4 && sub_command == "set") {
-      Status s = srv->GetNamespace()->Set(args_[2], args_[3]);
-      *output = s.IsOK() ? redis::RESP_OK : redis::Error(s);
-      WARN("Updated namespace: {} with token: {}, addr: {}, result: {}", args_[2], args_[3], conn->GetAddr(), s.Msg());
-    } else if (args_.size() == 4 && sub_command == "add") {
-      Status s = srv->GetNamespace()->Add(args_[2], args_[3]);
-      *output = s.IsOK() ? redis::RESP_OK : redis::Error(s);
-      WARN("New namespace: {} with token: {}, addr: {}, result: {}", args_[2], args_[3], conn->GetAddr(), s.Msg());
-    } else if (args_.size() == 3 && sub_command == "del") {
-      Status s = srv->GetNamespace()->Del(args_[2]);
-      *output = s.IsOK() ? redis::RESP_OK : redis::Error(s);
-      WARN("Deleted namespace: {}, addr: {}, result: {}", args_[2], conn->GetAddr(), s.Msg());
-    } else if (args_.size() == 2 && sub_command == "current") {
-      *output = redis::BulkString(conn->GetNamespace());
+      namespaces.emplace_back(kDefaultNamespace);
+      namespaces.emplace_back(config->requirepass);
+      *output = ArrayOfBulkStrings(namespaces);
+      return Status::OK();
+    }
+
+    auto token = srv->GetNamespace()->Get(args_[2]);
+    if (token.Is<Status::NotFound>()) {
+      *output = conn->NilString();
     } else {
-      return {Status::RedisExecErr, "NAMESPACE subcommand must be one of GET, SET, DEL, ADD and CURRENT"};
+      *output = redis::BulkString(token.GetValue());
     }
     return Status::OK();
   }
 };
 
-static uint64_t GenerateNamespaceFlag(uint64_t flags, const std::vector<std::string> &args) {
-  if (args.size() >= 2 && util::EqualICase(args[1], "current")) {
-    return flags & ~kCmdAdmin;
+class CommandNamespaceSet : public Commander {
+ public:
+  Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override {
+    if (IsNamespaceReadOnlyOnSlave(srv)) {
+      return {Status::RedisExecErr, "namespace is read-only for slave"};
+    }
+    if (IsNamespaceCommandDisabled(srv)) {
+      return {Status::RedisExecErr, "namespace command is not allowed when redis-databases > 0"};
+    }
+
+    auto s = srv->GetNamespace()->Set(args_[2], args_[3]);
+    *output = s.IsOK() ? redis::RESP_OK : redis::Error(s);
+    WARN("Updated namespace: {} with token: {}, addr: {}, result: {}", args_[2], args_[3], conn->GetAddr(), s.Msg());
+    return Status::OK();
   }
+};
 
-  return flags;
-}
+class CommandNamespaceAdd : public Commander {
+ public:
+  Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override {
+    if (IsNamespaceReadOnlyOnSlave(srv)) {
+      return {Status::RedisExecErr, "namespace is read-only for slave"};
+    }
+    if (IsNamespaceCommandDisabled(srv)) {
+      return {Status::RedisExecErr, "namespace command is not allowed when redis-databases > 0"};
+    }
+
+    auto s = srv->GetNamespace()->Add(args_[2], args_[3]);
+    *output = s.IsOK() ? redis::RESP_OK : redis::Error(s);
+    WARN("New namespace: {} with token: {}, addr: {}, result: {}", args_[2], args_[3], conn->GetAddr(), s.Msg());
+    return Status::OK();
+  }
+};
+
+class CommandNamespaceDel : public Commander {
+ public:
+  Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override {
+    if (IsNamespaceReadOnlyOnSlave(srv)) {
+      return {Status::RedisExecErr, "namespace is read-only for slave"};
+    }
+    if (IsNamespaceCommandDisabled(srv)) {
+      return {Status::RedisExecErr, "namespace command is not allowed when redis-databases > 0"};
+    }
+
+    auto s = srv->GetNamespace()->Del(args_[2]);
+    *output = s.IsOK() ? redis::RESP_OK : redis::Error(s);
+    WARN("Deleted namespace: {}, addr: {}, result: {}", args_[2], conn->GetAddr(), s.Msg());
+    return Status::OK();
+  }
+};
+
+class CommandNamespaceCurrent : public Commander {
+ public:
+  Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override {
+    if (IsNamespaceReadOnlyOnSlave(srv)) {
+      return {Status::RedisExecErr, "namespace is read-only for slave"};
+    }
+    if (IsNamespaceCommandDisabled(srv)) {
+      return {Status::RedisExecErr, "namespace command is not allowed when redis-databases > 0"};
+    }
+
+    *output = redis::BulkString(conn->GetNamespace());
+    return Status::OK();
+  }
+};
 
 class CommandKeys : public Commander {
  public:
@@ -829,13 +890,13 @@ class CommandCommand : public Commander {
       } else if (sub_command == "info") {
         CommandTable::GetCommandsInfo(output, std::vector<std::string>(args_.begin() + 2, args_.end()));
       } else if (sub_command == "getkeys") {
-        auto cmd_iter = CommandTable::GetOriginal()->find(util::ToLower(args_[2]));
-        if (cmd_iter == CommandTable::GetOriginal()->end()) {
+        std::vector<std::string> cmd_tokens(args_.begin() + 2, args_.end());
+        auto resolved = CommandTable::Resolve(cmd_tokens);
+        if (!resolved) {
           return {Status::RedisUnknownCmd, "Invalid command specified"};
         }
 
-        auto key_indexes = GET_OR_RET(CommandTable::GetKeysFromCommand(
-            cmd_iter->second, std::vector<std::string>(args_.begin() + 2, args_.end())));
+        auto key_indexes = GET_OR_RET(CommandTable::GetKeysFromCommand(resolved->attributes, cmd_tokens));
 
         if (key_indexes.size() == 0) {
           return {Status::RedisExecErr, "Invalid arguments specified for command"};
@@ -844,7 +905,7 @@ class CommandCommand : public Commander {
         std::vector<std::string> keys;
         keys.reserve(key_indexes.size());
         for (const auto &key_index : key_indexes) {
-          keys.emplace_back(args_[key_index + 2]);
+          keys.emplace_back(cmd_tokens[key_index]);
         }
         *output = conn->MultiBulkString(keys);
       } else {
@@ -1634,7 +1695,7 @@ REDIS_REGISTER_COMMANDS(
     MakeCmdAttr<CommandInfo>("info", -1, "read-only ok-loading", NO_KEY),
     MakeCmdAttr<CommandRole>("role", 1, "read-only ok-loading", NO_KEY),
     MakeCmdAttr<CommandConfig>("config", -2, "read-only admin skip-monitor", NO_KEY, GenerateConfigFlag),
-    MakeCmdAttr<CommandNamespace>("namespace", -2, "read-only admin skip-monitor", NO_KEY, GenerateNamespaceFlag),
+    MakeCmdAttr<CommandNamespace>("namespace", -2, "read-only admin skip-monitor", NO_KEY),
     MakeCmdAttr<CommandKeys>("keys", 2, "read-only slow", NO_KEY),
     MakeCmdAttr<CommandFlushDB>("flushdb", 1, "write no-dbsize-check exclusive", NO_KEY),
     MakeCmdAttr<CommandFlushAll>("flushall", 1, "write no-dbsize-check exclusive admin", NO_KEY),
@@ -1670,5 +1731,11 @@ REDIS_REGISTER_COMMANDS(
     MakeCmdAttr<CommandPollUpdates>("pollupdates", -2, "read-only admin", NO_KEY),
     MakeCmdAttr<CommandSST>("sst", -3, "write exclusive admin", 1, 1, 1),
     MakeCmdAttr<CommandFlushMemTable>("flushmemtable", -1, "exclusive write", NO_KEY),
-    MakeCmdAttr<CommandFlushBlockCache>("flushblockcache", 1, "exclusive write", NO_KEY), )
+    MakeCmdAttr<CommandFlushBlockCache>("flushblockcache", 1, "exclusive write", NO_KEY),
+    MakeSubCmdAttr<CommandNamespaceGet>("namespace", "get", 3, "read-only admin skip-monitor", NO_KEY),
+    MakeSubCmdAttr<CommandNamespaceSet>("namespace", "set", 4, "read-only admin skip-monitor", NO_KEY),
+    MakeSubCmdAttr<CommandNamespaceAdd>("namespace", "add", 4, "read-only admin skip-monitor", NO_KEY),
+    MakeSubCmdAttr<CommandNamespaceDel>("namespace", "del", 3, "read-only admin skip-monitor", NO_KEY),
+    MakeSubCmdAttr<CommandNamespaceCurrent>("namespace", "current", 2, "read-only skip-monitor", NO_KEY))
+
 }  // namespace redis

src/commands/commander.cc

@@ -20,22 +20,88 @@
 
 #include "commander.h"
 
+#include <cstdlib>
+
 #include "cluster/cluster_defs.h"
 #include "server/redis_reply.h"
 
 namespace redis {
 
+bool CommandTable::isSubcommandName(const std::string &name) { return name.find('|') != std::string::npos; }
+
+std::pair<std::string, std::string> CommandTable::parseSubcommandName(const std::string &name) {
+  auto delimiter = name.find('|');
+  if (delimiter == std::string::npos || delimiter == 0 || delimiter + 1 >= name.size()) {
+    std::cout << fmt::format("Encountered invalid subcommand name '{}'", name) << std::endl;
+    std::abort();
+  }
+
+  auto normalized_parent = util::ToLower(name.substr(0, delimiter));
+  auto normalized_sub = util::ToLower(name.substr(delimiter + 1));
+  if (normalized_sub.empty()) {
+    std::cout << fmt::format("Encountered invalid subcommand name '{}'", name) << std::endl;
+    std::abort();
+  }
+
+  return {normalized_parent, normalized_sub};
+}
+
+const CommandAttributes *CommandTable::registerCommand(CommandAttributes attr, CommandCategory category) {
+  if (original_commands.contains(attr.name) || commands.contains(attr.name)) {
+    std::cout << fmt::format("Duplicate command registration for '{}'", attr.name) << std::endl;
+    std::abort();
+  }
+
+  attr.category = category;
+  redis_command_table.emplace_back(std::move(attr));
+  auto *registered_attr = &redis_command_table.back();
+  original_commands[registered_attr->name] = registered_attr;
+  commands[registered_attr->name] = registered_attr;
+  return registered_attr;
+}
+
+const CommandAttributes *CommandTable::registerSubCommand(CommandAttributes attr, CommandCategory category) {
+  auto [parent, sub] = parseSubcommandName(attr.name);
+  auto &subcommand_family = sub_commands[parent];
+  if (subcommand_family.contains(sub)) {
+    std::cout << fmt::format("Duplicate subcommand registration for '{}|{}'", parent, sub) << std::endl;
+    std::abort();
+  }
+
+  attr.category = category;
+  attr.name = fmt::format("{}|{}", parent, sub);
+  redis_subcommand_table.emplace_back(std::move(attr));
+  auto *registered_attr = &redis_subcommand_table.back();
+  subcommand_family[sub] = registered_attr;
+  return registered_attr;
+}
+
+const CommandAttributes *CommandTable::findSubCommand(const std::string &parent, const std::string &sub) {
+  auto family_iter = sub_commands.find(util::ToLower(parent));
+  if (family_iter == sub_commands.end()) {
+    return nullptr;
+  }
+
+  auto subcommand_iter = family_iter->second.find(util::ToLower(sub));
+  if (subcommand_iter == family_iter->second.end()) {
+    return nullptr;
+  }
+
+  return subcommand_iter->second;
+}
+
 RegisterToCommandTable::RegisterToCommandTable(CommandCategory category,
                                                std::initializer_list<CommandAttributes> list) {
   if (category == CommandCategory::Disabled) {
     return;
   }
 
   for (auto attr : list) {
-    attr.category = category;
-    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();
+    if (CommandTable::isSubcommandName(attr.name)) {
+      CommandTable::registerSubCommand(std::move(attr), category);
+      continue;
+    }
+    CommandTable::registerCommand(std::move(attr), category);
   }
 }
 
@@ -83,6 +149,32 @@ void CommandTable::GetCommandsInfo(std::string *info, const std::vector<std::str
   }
 }
 
+StatusOr<ResolvedCommand> CommandTable::Resolve(const std::vector<std::string> &cmd_tokens) {
+  if (cmd_tokens.empty()) {
+    return {Status::RedisUnknownCmd};
+  }
+
+  auto cmd_iter = commands.find(util::ToLower(cmd_tokens.front()));
+  if (cmd_iter == commands.end()) {
+    return {Status::RedisUnknownCmd};
+  }
+
+  const auto *root_attributes = cmd_iter->second;
+  ResolvedCommand resolved{root_attributes->name, root_attributes};
+
+  if (cmd_tokens.size() <= 1) {
+    return resolved;
+  }
+
+  auto subcommand_attributes = findSubCommand(root_attributes->name, cmd_tokens[1]);
+  if (subcommand_attributes == nullptr) {
+    return resolved;
+  }
+
+  resolved.attributes = subcommand_attributes;
+  return resolved;
+}
+
 StatusOr<std::vector<int>> CommandTable::GetKeysFromCommand(const CommandAttributes *attributes,
                                                             const std::vector<std::string> &cmd_tokens) {
   int argc = static_cast<int>(cmd_tokens.size());
@@ -92,7 +184,9 @@ StatusOr<std::vector<int>> CommandTable::GetKeysFromCommand(const CommandAttribu
   }
 
   auto cmd = attributes->factory();
-  if (auto s = cmd->Parse(cmd_tokens); !s) {
+  cmd->SetAttributes(attributes);
+  cmd->SetArgs(cmd_tokens);
+  if (auto s = cmd->Parse(); !s) {
     return {Status::NotOK, "Invalid syntax found in this command arguments: " + s.Msg()};
   }