From 3cd3a6da3fe0b6f022b721094bda0b97c3527d23 Mon Sep 17 00:00:00 2001 From: Amirsaman Memaripour Date: Thu, 23 Jan 2020 17:14:41 +0000 Subject: SERVER-45202 Improve command alias infrastructure --- jstests/core/failcommand_failpoint.js | 18 ++++++++++++++ src/mongo/db/commands.cpp | 38 ++++++++++++++++++----------- src/mongo/db/commands.h | 28 +++++++++++++++------ src/mongo/db/service_entry_point_common.cpp | 7 ++---- src/mongo/s/commands/strategy.cpp | 4 +-- 5 files changed, 66 insertions(+), 29 deletions(-) diff --git a/jstests/core/failcommand_failpoint.js b/jstests/core/failcommand_failpoint.js index 88d9f5d2400..901387c995d 100644 --- a/jstests/core/failcommand_failpoint.js +++ b/jstests/core/failcommand_failpoint.js @@ -19,6 +19,24 @@ const getThreadName = function() { let threadName = getThreadName(); +// Test failpoint for command aliases +assert.commandWorked(adminDB.runCommand({ + configureFailPoint: "failCommand", + mode: "alwaysOn", + data: { + errorCode: ErrorCodes.BadValue, + failCommands: ["dropIndexes"], + threadName: threadName, + } +})); +assert.commandFailedWithCode(testDB.runCommand({dropIndexes: 'collection', index: '*'}), + ErrorCodes.BadValue); +assert.commandWorked(testDB.runCommand({buildInfo: 1})); +assert.commandFailedWithCode(testDB.runCommand({deleteIndexes: 'collection', index: '*'}), + ErrorCodes.BadValue); +assert.commandWorked(testDB.runCommand({buildinfo: 1})); +assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"})); + // Test failing with a particular error code. assert.commandWorked(adminDB.runCommand({ configureFailPoint: "failCommand", diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index b2c5fb809f8..8fc57f0746a 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -500,10 +500,11 @@ const OperationContext::Decoration> errorLabelsOverri OperationContext::declareDecoration>(); bool CommandHelpers::shouldActivateFailCommandFailPoint(const BSONObj& data, - StringData cmdName, - Client* client, - const NamespaceString& nss) { - if (cmdName == "configureFailPoint"_sd) // Banned even if in failCommands. + const CommandInvocation* invocation, + Client* client) { + const Command* cmd = invocation->definition(); + const NamespaceString& nss = invocation->ns(); + if (cmd->getName() == "configureFailPoint"_sd) // Banned even if in failCommands. return false; if (data.hasField("threadName") && @@ -528,7 +529,7 @@ bool CommandHelpers::shouldActivateFailCommandFailPoint(const BSONObj& data, } for (auto&& failCommand : data.getObjectField("failCommands")) { - if (failCommand.type() == String && failCommand.valueStringData() == cmdName) { + if (failCommand.type() == String && cmd->hasAlias(failCommand.valueStringData())) { return true; } } @@ -537,11 +538,11 @@ bool CommandHelpers::shouldActivateFailCommandFailPoint(const BSONObj& data, } void CommandHelpers::evaluateFailCommandFailPoint(OperationContext* opCtx, - StringData commandName, - const NamespaceString& nss) { + const CommandInvocation* invocation) { bool closeConnection; bool hasErrorCode; long long errorCode; + const Command* cmd = invocation->definition(); failCommand.executeIf( [&](const BSONObj& data) { if (data.hasField(kErrorLabelsFieldName) && @@ -555,13 +556,13 @@ void CommandHelpers::evaluateFailCommandFailPoint(OperationContext* opCtx, if (closeConnection) { opCtx->getClient()->session()->end(); - log() << "Failing command '" << commandName + log() << "Failing command '" << cmd->getName() << "' via 'failCommand' failpoint. Action: closing connection."; uasserted(50985, "Failing command due to 'failCommand' failpoint"); } if (hasErrorCode) { - log() << "Failing command '" << commandName + log() << "Failing command '" << cmd->getName() << "' via 'failCommand' failpoint. Action: returning error code " << errorCode << "."; uasserted(ErrorCodes::Error(errorCode), @@ -574,7 +575,7 @@ void CommandHelpers::evaluateFailCommandFailPoint(OperationContext* opCtx, closeConnection; hasErrorCode = data.hasField("errorCode") && bsonExtractIntegerField(data, "errorCode", &errorCode).isOK(); - return shouldActivateFailCommandFailPoint(data, commandName, opCtx->getClient(), nss) && + return shouldActivateFailCommandFailPoint(data, invocation, opCtx->getClient()) && (closeConnection || hasErrorCode); }); } @@ -718,11 +719,16 @@ std::unique_ptr BasicCommandWithReplyBuilderInterface::parse( return std::make_unique(opCtx, request, this); } -Command::Command(StringData name, StringData oldName) +Command::Command(StringData name, std::vector aliases) : _name(name.toString()), + _aliases(std::move(aliases)), _commandsExecutedMetric("commands." + _name + ".total", &_commandsExecuted), _commandsFailedMetric("commands." + _name + ".failed", &_commandsFailed) { - globalCommandRegistry()->registerCommand(this, name, oldName); + globalCommandRegistry()->registerCommand(this, _name, _aliases); +} + +bool Command::hasAlias(const StringData& alias) const { + return globalCommandRegistry()->findCommand(alias) == this; } Status BasicCommandWithReplyBuilderInterface::explain(OperationContext* opCtx, @@ -772,8 +778,12 @@ bool ErrmsgCommandDeprecated::run(OperationContext* opCtx, ////////////////////////////////////////////////////////////// // CommandRegistry -void CommandRegistry::registerCommand(Command* command, StringData name, StringData oldName) { - for (StringData key : {name, oldName}) { +void CommandRegistry::registerCommand(Command* command, + StringData name, + std::vector aliases) { + aliases.push_back(name); + + for (auto key : aliases) { if (key.empty()) { continue; } diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index b993a6b737c..8590d3b9f16 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -228,16 +228,14 @@ struct CommandHelpers { * Checks if the command passed in is in the list of failCommands defined in the fail point. */ static bool shouldActivateFailCommandFailPoint(const BSONObj& data, - StringData cmdName, - Client* client, - const NamespaceString& nss); + const CommandInvocation* invocation, + Client* client); /** * Possibly uasserts according to the "failCommand" fail point. */ static void evaluateFailCommandFailPoint(OperationContext* opCtx, - StringData commandName, - const NamespaceString& nss); + const CommandInvocation* invocation); /** * Handles marking kill on client disconnect. @@ -258,9 +256,15 @@ public: * Constructs a new command and causes it to be registered with the global commands list. It is * not safe to construct commands other than when the server is starting up. * - * @param oldName an optional old, deprecated name for the command + * @param oldName an old, deprecated name for the command */ - Command(StringData name, StringData oldName = StringData()); + Command(StringData name, StringData oldName) + : Command(name, std::vector({oldName})) {} + + /** + * @param aliases the optional list of aliases (e.g., old names) for the command + */ + Command(StringData name, std::vector aliases = {}); Command(const Command&) = delete; Command& operator=(const Command&) = delete; @@ -430,10 +434,18 @@ public: return true; } + /** + * Checks if the command is also known by the provided alias. + */ + bool hasAlias(const StringData& alias) const; + private: // The full name of the command const std::string _name; + // The list of aliases for the command + const std::vector _aliases; + // Counters for how many times this command has been executed and failed mutable Counter64 _commandsExecuted; mutable Counter64 _commandsFailed; @@ -884,7 +896,7 @@ public: return _commands; } - void registerCommand(Command* command, StringData name, StringData oldName); + void registerCommand(Command* command, StringData name, std::vector aliases); Command* findCommand(StringData name) const; diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 3af8ccda71c..05577a81323 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -687,10 +687,7 @@ bool runCommandImpl(OperationContext* opCtx, }, [&](const BSONObj& data) { return CommandHelpers::shouldActivateFailCommandFailPoint( - data, - request.getCommandName(), - opCtx->getClient(), - invocation->ns()) && + data, invocation, opCtx->getClient()) && data.hasField("writeConcernError"); }); if (reallyWait) { @@ -845,7 +842,7 @@ void execCommandDatabase(OperationContext* opCtx, replCoord->getReplicationMode() == repl::ReplicationCoordinator::modeReplSet, opCtx->getServiceContext()->getStorageEngine()->supportsDocLocking()); - CommandHelpers::evaluateFailCommandFailPoint(opCtx, command->getName(), invocation->ns()); + CommandHelpers::evaluateFailCommandFailPoint(opCtx, invocation.get()); const auto dbname = request.getDatabase().toString(); uassert( diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index eef03a45db3..e0e2221ce3c 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -283,7 +283,7 @@ void execCommandClient(OperationContext* opCtx, }, [&](const BSONObj& data) { return CommandHelpers::shouldActivateFailCommandFailPoint( - data, request.getCommandName(), opCtx->getClient(), invocation->ns()) && + data, invocation, opCtx->getClient()) && data.hasField("writeConcernError"); }); } @@ -397,7 +397,7 @@ void runCommand(OperationContext* opCtx, try { rpc::readRequestMetadata(opCtx, request.body, command->requiresAuth()); - CommandHelpers::evaluateFailCommandFailPoint(opCtx, commandName, invocation->ns()); + CommandHelpers::evaluateFailCommandFailPoint(opCtx, invocation.get()); bool startTransaction = false; if (osi.getAutocommit()) { routerSession.emplace(opCtx); -- cgit v1.2.1