SERVER-32484 CommandRegistry

7 files changed, 89 insertions, 59 deletions

diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp
index a95eed56437..25ee7429189 100644
--- a/src/mongo/db/commands.cpp
+++ b/src/mongo/db/commands.cpp
@@ -51,19 +51,13 @@
 #include "mongo/db/server_parameters.h"
 #include "mongo/rpc/write_concern_error_detail.h"
 #include "mongo/s/stale_exception.h"
+#include "mongo/util/invariant.h"
 #include "mongo/util/log.h"
 
 namespace mongo {
 
 using logger::LogComponent;
 
-Command::CommandMap* Command::_commandsByBestName = nullptr;
-Command::CommandMap* Command::_commands = nullptr;
-
-Counter64 Command::unknownCommands;
-static ServerStatusMetricField<Counter64> displayUnknownCommands("commands.<UNKNOWN>",
-                                                                 &Command::unknownCommands);
-
 namespace {
 
 ExportedServerParameter<bool, ServerParameterType::kStartupOnly> testCommandsParameter(
@@ -205,19 +199,7 @@ Command::Command(StringData name, StringData oldName)
     : _name(name.toString()),
       _commandsExecutedMetric("commands." + _name + ".total", &_commandsExecuted),
       _commandsFailedMetric("commands." + _name + ".failed", &_commandsFailed) {
-    // register ourself.
-    if (_commands == 0)
-        _commands = new CommandMap();
-    if (_commandsByBestName == 0)
-        _commandsByBestName = new CommandMap();
-    Command*& c = (*_commands)[name];
-    if (c)
-        log() << "warning: 2 commands with name: " << _name;
-    c = this;
-    (*_commandsByBestName)[name] = this;
-
-    if (!oldName.empty())
-        (*_commands)[oldName.toString()] = this;
+    globalCommandRegistry()->registerCommand(this, name, oldName);
 }
 
 void Command::help(std::stringstream& help) const {
@@ -233,29 +215,11 @@ Status Command::explain(OperationContext* opCtx,
 }
 
 BSONObj Command::runCommandDirectly(OperationContext* opCtx, const OpMsgRequest& request) {
-    auto command = Command::findCommand(request.getCommandName());
-    invariant(command);
-
-    BSONObjBuilder out;
-    try {
-        bool ok = command->publicRun(opCtx, request, out);
-        appendCommandStatus(out, ok);
-    } catch (const StaleConfigException&) {
-        // These exceptions are intended to be handled at a higher level and cannot losslessly
-        // round-trip through Status.
-        throw;
-    } catch (const DBException& ex) {
-        out.resetToEmpty();
-        appendCommandStatus(out, ex.toStatus());
-    }
-    return out.obj();
+    return CommandHelpers::runCommandDirectly(opCtx, request);
 }
 
 Command* Command::findCommand(StringData name) {
-    CommandMap::const_iterator i = _commands->find(name);
-    if (i == _commands->end())
-        return 0;
-    return i->second;
+    return globalCommandRegistry()->findCommand(name);
 }
 
 bool Command::appendCommandStatus(BSONObjBuilder& result, const Status& status) {
@@ -516,4 +480,47 @@ BSONObj Command::filterCommandReplyForPassthrough(const BSONObj& cmdObj) {
     return bob.obj();
 }
 
+void CommandRegistry::registerCommand(Command* command, StringData name, StringData oldName) {
+    for (StringData key : {name, oldName}) {
+        if (key.empty()) {
+            continue;
+        }
+        auto hashedKey = CommandMap::HashedKey(key);
+        auto iter = _commands.find(hashedKey);
+        invariant(iter == _commands.end(), str::stream() << "command name collision: " << key);
+        _commands[hashedKey] = command;
+    }
+}
+
+Command* CommandRegistry::findCommand(StringData name) const {
+    auto it = _commands.find(name);
+    if (it == _commands.end())
+        return nullptr;
+    return it->second;
+}
+
+BSONObj CommandHelpers::runCommandDirectly(OperationContext* opCtx, const OpMsgRequest& request) {
+    auto command = globalCommandRegistry()->findCommand(request.getCommandName());
+    invariant(command);
+
+    BSONObjBuilder out;
+    try {
+        bool ok = command->publicRun(opCtx, request, out);
+        Command::appendCommandStatus(out, ok);
+    } catch (const StaleConfigException&) {
+        // These exceptions are intended to be handled at a higher level and cannot losslessly
+        // round-trip through Status.
+        throw;
+    } catch (const DBException& ex) {
+        out.resetToEmpty();
+        Command::appendCommandStatus(out, ex.toStatus());
+    }
+    return out.obj();
+}
+
+CommandRegistry* globalCommandRegistry() {
+    static auto reg = new CommandRegistry();
+    return reg;
+}
+
 }  // namespace mongo
diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h
index 89a6d18a9e0..37693df5455 100644
--- a/src/mongo/db/commands.h
+++ b/src/mongo/db/commands.h
@@ -300,17 +300,6 @@ public:
      */
     bool publicRun(OperationContext* opCtx, const OpMsgRequest& request, BSONObjBuilder& result);
 
-    static const CommandMap& allCommands() {
-        return *_commands;
-    }
-
-    static const CommandMap& allCommandsByBestName() {
-        return *_commandsByBestName;
-    }
-
-    // Counter for unknown commands
-    static Counter64 unknownCommands;
-
     /**
      * Runs a command directly and returns the result. Does not do any other work normally handled
      * by command dispatch, such as checking auth, dealing with CurOp or waiting for write concern.
@@ -468,9 +457,6 @@ public:
     static BSONObj filterCommandReplyForPassthrough(const BSONObj& reply);
 
 private:
-    static CommandMap* _commands;
-    static CommandMap* _commandsByBestName;
-
     /**
      * Runs the command.
      *
@@ -589,4 +575,41 @@ class ErrmsgCommandDeprecated : public BasicCommand {
                            BSONObjBuilder& result) = 0;
 };
 
+// Struct as closed namespace. Nothing but statics.
+struct CommandHelpers {
+    static BSONObj runCommandDirectly(OperationContext* opCtx, const OpMsgRequest& request);
+};
+
+// See the 'globalCommandRegistry()' singleton accessor.
+class CommandRegistry {
+public:
+    using CommandMap = Command::CommandMap;
+
+    CommandRegistry() : _unknownsMetricField("commands.<UNKNOWN>", &_unknowns) {}
+
+    CommandRegistry(const CommandRegistry&) = delete;
+    CommandRegistry& operator=(const CommandRegistry&) = delete;
+
+    const CommandMap& allCommands() const {
+        return _commands;
+    }
+
+    void registerCommand(Command* command, StringData name, StringData oldName);
+
+    Command* findCommand(StringData name) const;
+
+    void incrementUnknownCommands() {
+        _unknowns.increment();
+    }
+
+private:
+    Counter64 _unknowns;
+    ServerStatusMetricField<Counter64> _unknownsMetricField;
+
+    CommandMap _commands;
+};
+
+// Accessor to the command registry, an always-valid singleton.
+CommandRegistry* globalCommandRegistry();
+
 }  // namespace mongo
diff --git a/src/mongo/db/commands/generic.cpp b/src/mongo/db/commands/generic.cpp
index 1cf3d51814a..f047d9fd70d 100644
--- a/src/mongo/db/commands/generic.cpp
+++ b/src/mongo/db/commands/generic.cpp
@@ -270,7 +270,7 @@ public:
                      BSONObjBuilder& result) {
         // sort the commands before building the result BSON
         std::vector<Command*> commands;
-        for (const auto command : allCommands()) {
+        for (const auto command : globalCommandRegistry()->allCommands()) {
             // don't show oldnames
             if (command.first == command.second->getName())
                 commands.push_back(command.second);
diff --git a/src/mongo/db/service_entry_point_mongod.cpp b/src/mongo/db/service_entry_point_mongod.cpp
index 1dfa4570fd4..c6acd32c214 100644
--- a/src/mongo/db/service_entry_point_mongod.cpp
+++ b/src/mongo/db/service_entry_point_mongod.cpp
@@ -857,7 +857,7 @@ DbResponse runCommands(OperationContext* opCtx, const Message& message) {
             // we restrict the log message to the name of the unrecognized command.
             // However, the complete command object will still be echoed to the client.
             if (!(c = Command::findCommand(request.getCommandName()))) {
-                Command::unknownCommands.increment();
+                globalCommandRegistry()->incrementUnknownCommands();
                 std::string msg = str::stream() << "no such command: '" << request.getCommandName()
                                                 << "'";
                 LOG(2) << msg;
diff --git a/src/mongo/s/commands/cluster_fsync_cmd.cpp b/src/mongo/s/commands/cluster_fsync_cmd.cpp
index 691b6c41caf..4b1a4a469e5 100644
--- a/src/mongo/s/commands/cluster_fsync_cmd.cpp
+++ b/src/mongo/s/commands/cluster_fsync_cmd.cpp
@@ -40,7 +40,7 @@ namespace {
 
 class FsyncCommand : public ErrmsgCommandDeprecated {
 public:
-    FsyncCommand() : ErrmsgCommandDeprecated("fsync", "fsync") {}
+    FsyncCommand() : ErrmsgCommandDeprecated("fsync") {}
 
     virtual bool slaveOk() const {
         return true;
diff --git a/src/mongo/s/commands/cluster_split_cmd.cpp b/src/mongo/s/commands/cluster_split_cmd.cpp
index 325333852a4..e29e1a7c412 100644
--- a/src/mongo/s/commands/cluster_split_cmd.cpp
+++ b/src/mongo/s/commands/cluster_split_cmd.cpp
@@ -86,7 +86,7 @@ BSONObj selectMedianKey(OperationContext* opCtx,
 
 class SplitCollectionCmd : public ErrmsgCommandDeprecated {
 public:
-    SplitCollectionCmd() : ErrmsgCommandDeprecated("split", "split") {}
+    SplitCollectionCmd() : ErrmsgCommandDeprecated("split") {}
 
     bool slaveOk() const override {
         return true;
diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp
index 9b05cad2a4d..0a66edcbcb0 100644
--- a/src/mongo/s/commands/strategy.cpp
+++ b/src/mongo/s/commands/strategy.cpp
@@ -251,7 +251,7 @@ void runCommand(OperationContext* opCtx, const OpMsgRequest& request, BSONObjBui
         Command::appendCommandStatus(
             builder,
             {ErrorCodes::CommandNotFound, str::stream() << "no such cmd: " << commandName});
-        Command::unknownCommands.increment();
+        globalCommandRegistry()->incrementUnknownCommands();
         return;
     }