diff options
author | Anthony Roy <anthony.roy@10gen.com> | 2018-06-19 17:53:48 -0400 |
---|---|---|
committer | Anthony Roy <anthony.roy@10gen.com> | 2018-07-05 17:43:25 -0400 |
commit | f78056a8f1f5ea6af23bd68123659b714233b370 (patch) | |
tree | ca9bf843fec69e4ba687bdee55dade490b5ac246 /src/mongo/db | |
parent | 173ac70346990e4cf9b2720f86697785b8795967 (diff) | |
download | mongo-f78056a8f1f5ea6af23bd68123659b714233b370.tar.gz |
SERVER-35460 Replaced CommandReplyBuilder with direct usage of ReplyBuilderInterface
This is to provide access to DocumentSequence returns.
Diffstat (limited to 'src/mongo/db')
-rw-r--r-- | src/mongo/db/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/commands.cpp | 34 | ||||
-rw-r--r-- | src/mongo/db/commands.h | 54 | ||||
-rw-r--r-- | src/mongo/db/commands/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/commands/explain_cmd.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/commands/generic.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/commands/mr_test.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/commands/write_commands/write_commands.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/commands_test.cpp | 15 | ||||
-rw-r--r-- | src/mongo/db/s/get_database_version_command.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/service_entry_point_common.cpp | 22 |
11 files changed, 51 insertions, 104 deletions
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 527e2ce7af6..67d31359f66 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -594,6 +594,7 @@ env.CppUnitTest( ], LIBDEPS=[ '$BUILD_DIR/mongo/idl/idl_parser', + '$BUILD_DIR/mongo/rpc/rpc', "commands", "auth/authmocks", "repl/replmocks", diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index 87f02341918..b020d29eee0 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -50,6 +50,9 @@ #include "mongo/db/jsobj.h" #include "mongo/db/namespace_string.h" #include "mongo/db/server_parameters.h" +#include "mongo/rpc/factory.h" +#include "mongo/rpc/op_msg_rpc_impls.h" +#include "mongo/rpc/protocol.h" #include "mongo/rpc/write_concern_error_detail.h" #include "mongo/s/stale_exception.h" #include "mongo/util/invariant.h" @@ -109,22 +112,21 @@ bool checkAuthorizationImplPreParse(OperationContext* opCtx, BSONObj CommandHelpers::runCommandDirectly(OperationContext* opCtx, const OpMsgRequest& request) { auto command = globalCommandRegistry()->findCommand(request.getCommandName()); invariant(command); - BufBuilder bb; - CommandReplyBuilder crb(BSONObjBuilder{bb}); + rpc::OpMsgReplyBuilder replyBuilder; try { auto invocation = command->parse(opCtx, request); - invocation->run(opCtx, &crb); - auto body = crb.getBodyBuilder(); + invocation->run(opCtx, &replyBuilder); + auto body = replyBuilder.getBodyBuilder(); CommandHelpers::extractOrAppendOk(body); } catch (const StaleConfigException&) { // These exceptions are intended to be handled at a higher level. throw; } catch (const DBException& ex) { - auto body = crb.getBodyBuilder(); + auto body = replyBuilder.getBodyBuilder(); body.resetToEmpty(); appendCommandStatusNoThrow(body, ex.toStatus()); } - return BSONObj(bb.release()); + return replyBuilder.releaseBody(); } void CommandHelpers::auditLogAuthEvent(OperationContext* opCtx, @@ -380,24 +382,6 @@ bool CommandHelpers::uassertShouldAttemptParse(OperationContext* opCtx, constexpr StringData CommandHelpers::kHelpFieldName; ////////////////////////////////////////////////////////////// -// CommandReplyBuilder - -CommandReplyBuilder::CommandReplyBuilder(BSONObjBuilder bodyObj) - : _bodyBuf(&bodyObj.bb()), _bodyOffset(bodyObj.offset()) { - // CommandReplyBuilder requires that bodyObj build into an externally-owned buffer. - invariant(!bodyObj.owned()); - bodyObj.doneFast(); -} - -BSONObjBuilder CommandReplyBuilder::getBodyBuilder() const { - return BSONObjBuilder(BSONObjBuilder::ResumeBuildingTag{}, *_bodyBuf, _bodyOffset); -} - -void CommandReplyBuilder::reset() { - getBodyBuilder().resetToEmpty(); -} - -////////////////////////////////////////////////////////////// // CommandInvocation CommandInvocation::~CommandInvocation() = default; @@ -443,7 +427,7 @@ public: _dbName(_request->getDatabase().toString()) {} private: - void run(OperationContext* opCtx, CommandReplyBuilder* result) override { + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* result) override { try { BSONObjBuilder bob = result->getBodyBuilder(); bool ok = _command->run(opCtx, _dbName, _request->body, bob); diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index 966a0d640c7..5907fb65990 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -414,52 +414,6 @@ private: ServerStatusMetricField<Counter64> _commandsFailedMetric; }; -class CommandReplyBuilder { -public: - explicit CommandReplyBuilder(BSONObjBuilder bodyObj); - - CommandReplyBuilder(const CommandReplyBuilder&) = delete; - CommandReplyBuilder& operator=(const CommandReplyBuilder&) = delete; - - /** - * Returns a BSONObjBuilder that can be used to build the reply in-place. The returned - * builder (or an object into which it has been moved) must be completed before calling - * any more methods on this object. A builder is completed by a call to `done()` or by - * its destructor. Can be called repeatedly to append multiple things to the reply, as - * long as each returned builder must be completed between calls. - */ - BSONObjBuilder getBodyBuilder() const; - - void reset(); - - /** - * Appends a key:object field to this reply. - */ - template <typename T> - void append(StringData key, const T& object) { - getBodyBuilder() << key << object; - } - - /** - * The specified 'object' must be BSON-serializable. - * - * BSONSerializable 'x' means 'x.serialize(bob)' appends a representation of 'x' - * into 'BSONObjBuilder* bob'. - */ - template <typename T> - void fillFrom(const T& object) { - static_assert(!isStatusOrStatusWith<std::decay_t<T>>, - "Status and StatusWith<T> aren't supported by TypedCommand and fillFrom(). " - "Use uassertStatusOK() instead."); - auto bob = getBodyBuilder(); - object.serialize(&bob); - } - -private: - BufBuilder* const _bodyBuf; - const std::size_t _bodyOffset; -}; - /** * Represents a single invocation of a given command. */ @@ -476,7 +430,7 @@ public: * indicated either by throwing (preferred), or by calling * `CommandHelpers::extractOrAppendOk`. */ - virtual void run(OperationContext* opCtx, CommandReplyBuilder* result) = 0; + virtual void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* result) = 0; virtual void explain(OperationContext* opCtx, ExplainOptions::Verbosity verbosity, @@ -808,14 +762,14 @@ private: decltype(auto) _callTypedRun(OperationContext* opCtx) { return static_cast<Invocation*>(this)->typedRun(opCtx); } - void _runImpl(std::true_type, OperationContext* opCtx, CommandReplyBuilder*) { + void _runImpl(std::true_type, OperationContext* opCtx, rpc::ReplyBuilderInterface*) { _callTypedRun(opCtx); } - void _runImpl(std::false_type, OperationContext* opCtx, CommandReplyBuilder* reply) { + void _runImpl(std::false_type, OperationContext* opCtx, rpc::ReplyBuilderInterface* reply) { reply->fillFrom(_callTypedRun(opCtx)); } - void run(OperationContext* opCtx, CommandReplyBuilder* reply) final { + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* reply) final { using VoidResultTag = std::is_void<decltype(_callTypedRun(opCtx))>; _runImpl(VoidResultTag{}, opCtx, reply); } diff --git a/src/mongo/db/commands/SConscript b/src/mongo/db/commands/SConscript index 7f16897f022..908214a6e2f 100644 --- a/src/mongo/db/commands/SConscript +++ b/src/mongo/db/commands/SConscript @@ -109,6 +109,7 @@ env.Library( '$BUILD_DIR/mongo/db/mongohasher', '$BUILD_DIR/mongo/db/server_options_core', '$BUILD_DIR/mongo/logger/parse_log_component_settings', + '$BUILD_DIR/mongo/rpc/protocol', ], ) diff --git a/src/mongo/db/commands/explain_cmd.cpp b/src/mongo/db/commands/explain_cmd.cpp index 003047168db..46b0c6eac3f 100644 --- a/src/mongo/db/commands/explain_cmd.cpp +++ b/src/mongo/db/commands/explain_cmd.cpp @@ -92,7 +92,7 @@ public: _innerRequest{std::move(innerRequest)}, _innerInvocation{std::move(innerInvocation)} {} - void run(OperationContext* opCtx, CommandReplyBuilder* result) override { + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* result) override { uassert(50746, "Explain's child command cannot run on this node. " "Are you explaining a write command on a secondary?", diff --git a/src/mongo/db/commands/generic.cpp b/src/mongo/db/commands/generic.cpp index 277b22aba99..a572b94747c 100644 --- a/src/mongo/db/commands/generic.cpp +++ b/src/mongo/db/commands/generic.cpp @@ -108,8 +108,16 @@ public: return NamespaceString(request().request.getDatabase()); } - void run(OperationContext* opCtx, CommandReplyBuilder* result) override { - result->append("echo", request().request.body); + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* result) override { + auto sequences = request().request.sequences; + for (auto& docSeq : sequences) { + auto docBuilder = result->getDocSequenceBuilder(docSeq.name); + for (auto& bson : docSeq.objs) { + docBuilder.append(bson); + } + } + + result->getBodyBuilder().append("echo", request().request.body); } }; diff --git a/src/mongo/db/commands/mr_test.cpp b/src/mongo/db/commands/mr_test.cpp index f8c81c473a5..6a1f5d67302 100644 --- a/src/mongo/db/commands/mr_test.cpp +++ b/src/mongo/db/commands/mr_test.cpp @@ -47,6 +47,7 @@ #include "mongo/db/repl/replication_coordinator_mock.h" #include "mongo/db/repl/storage_interface_impl.h" #include "mongo/db/service_context_d_test_fixture.h" +#include "mongo/rpc/factory.h" #include "mongo/rpc/get_status_from_command_result.h" #include "mongo/scripting/dbdirectclient_factory.h" #include "mongo/scripting/engine.h" @@ -425,10 +426,9 @@ Status MapReduceCommandTest::_runCommand(StringData mapCode, StringData reduceCo ASSERT(command) << "Unable to look up mapReduce command"; auto request = OpMsgRequest::fromDBAndBody(inputNss.db(), _makeCmdObj(mapCode, reduceCode)); - BufBuilder bb; - CommandReplyBuilder crb(BSONObjBuilder{bb}); - command->parse(_opCtx.get(), request)->run(_opCtx.get(), &crb); - auto status = getStatusFromCommandResult(crb.getBodyBuilder().asTempObj()); + auto replyBuilder = rpc::makeReplyBuilder(rpc::Protocol::kOpMsg); + command->parse(_opCtx.get(), request)->run(_opCtx.get(), replyBuilder.get()); + auto status = getStatusFromCommandResult(replyBuilder->getBodyBuilder().asTempObj()); if (!status.isOK()) { return status.withContext(str::stream() << "mapReduce command failed: " << request.body); } diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index edbd9a6d315..46ebb7d0e9c 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -225,7 +225,7 @@ private: // Customization point for 'run'. virtual void runImpl(OperationContext* opCtx, BSONObjBuilder& result) const = 0; - void run(OperationContext* opCtx, CommandReplyBuilder* result) final { + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* result) final { try { try { _transactionChecks(opCtx); diff --git a/src/mongo/db/commands_test.cpp b/src/mongo/db/commands_test.cpp index bf03dd61a19..c92f72617c3 100644 --- a/src/mongo/db/commands_test.cpp +++ b/src/mongo/db/commands_test.cpp @@ -33,6 +33,8 @@ #include "mongo/db/commands_test_example_gen.h" #include "mongo/db/dbmessage.h" #include "mongo/db/service_context_test_fixture.h" +#include "mongo/rpc/factory.h" +#include "mongo/rpc/op_msg_rpc_impls.h" #include "mongo/unittest/unittest.h" namespace mongo { @@ -203,7 +205,7 @@ public: /** * Reply with an incremented 'request.i'. */ - void run(OperationContext* opCtx, CommandReplyBuilder* reply) override { + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* reply) override { commands_test_example::ExampleIncrementReply r; r.setIPlusOne(request().getI() + 1); reply->fillFrom(r); @@ -345,17 +347,16 @@ protected: ASSERT_EQ(invocation->ns(), ns); const BSONObj reply = [&] { - BufBuilder bb; - CommandReplyBuilder crb{BSONObjBuilder{bb}}; + rpc::OpMsgReplyBuilder replyBuilder; try { - invocation->run(opCtx.get(), &crb); - auto bob = crb.getBodyBuilder(); + invocation->run(opCtx.get(), &replyBuilder); + auto bob = replyBuilder.getBodyBuilder(); CommandHelpers::extractOrAppendOk(bob); } catch (const DBException& e) { - auto bob = crb.getBodyBuilder(); + auto bob = replyBuilder.getBodyBuilder(); CommandHelpers::appendCommandStatusNoThrow(bob, e.toStatus()); } - return BSONObj(bb.release()); + return replyBuilder.releaseBody(); }(); postAssert(i, reply); diff --git a/src/mongo/db/s/get_database_version_command.cpp b/src/mongo/db/s/get_database_version_command.cpp index 8d7f3446db6..b6925c705f2 100644 --- a/src/mongo/db/s/get_database_version_command.cpp +++ b/src/mongo/db/s/get_database_version_command.cpp @@ -72,7 +72,7 @@ public: ActionType::getDatabaseVersion)); } - void run(OperationContext* opCtx, CommandReplyBuilder* result) override { + void run(OperationContext* opCtx, rpc::ReplyBuilderInterface* result) override { uassert(ErrorCodes::IllegalOperation, str::stream() << definition()->getName() << " can only be run on shard servers", serverGlobalParams.clusterRole == ClusterRole::ShardServer); @@ -83,7 +83,7 @@ public: versionObj = dbVersion->toBSON(); } } - result->append("dbVersion", versionObj); + result->getBodyBuilder().append("dbVersion", versionObj); } StringData _targetDb() const { diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 11a73d8b421..287be590179 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -460,7 +460,7 @@ void appendClusterAndOperationTime(OperationContext* opCtx, void invokeInTransaction(OperationContext* opCtx, CommandInvocation* invocation, - CommandReplyBuilder* replyBuilder) { + rpc::ReplyBuilderInterface* replyBuilder) { auto session = OperationContextSession::get(opCtx); if (!session) { // Run the command directly if we're not in a transaction. @@ -495,7 +495,6 @@ bool runCommandImpl(OperationContext* opCtx, const boost::optional<OperationSessionInfoFromClient>& sessionOptions) { const Command* command = invocation->definition(); auto bytesToReserve = command->reserveBytesForReply(); - // SERVER-22100: In Windows DEBUG builds, the CRT heap debugging overhead, in conjunction with the // additional memory pressure introduced by reply buffer pre-allocation, causes the concurrency // suite to run extremely slowly. As a workaround we do not pre-allocate in Windows DEBUG builds. @@ -503,12 +502,11 @@ bool runCommandImpl(OperationContext* opCtx, if (kDebugBuild) bytesToReserve = 0; #endif - - CommandReplyBuilder crb(replyBuilder->getInPlaceReplyBuilder(bytesToReserve)); + replyBuilder->reserveBytes(bytesToReserve); if (!invocation->supportsWriteConcern()) { behaviors.uassertCommandDoesNotSpecifyWriteConcern(request.body); - invokeInTransaction(opCtx, invocation, &crb); + invokeInTransaction(opCtx, invocation, replyBuilder); } else { auto wcResult = uassertStatusOK(extractWriteConcern(opCtx, request.body)); auto session = OperationContextSession::get(opCtx); @@ -539,13 +537,13 @@ bool runCommandImpl(OperationContext* opCtx, }; try { - invokeInTransaction(opCtx, invocation, &crb); + invokeInTransaction(opCtx, invocation, replyBuilder); } catch (const DBException&) { waitForWriteConcern(*extraFieldsBuilder); throw; } - waitForWriteConcern(crb.getBodyBuilder()); + waitForWriteConcern(replyBuilder->getBodyBuilder()); // Nothing in run() should change the writeConcern. dassert(SimpleBSONObjComparator::kInstance.evaluate(opCtx->getWriteConcern().toBSON() == @@ -555,20 +553,20 @@ bool runCommandImpl(OperationContext* opCtx, behaviors.waitForLinearizableReadConcern(opCtx); const bool ok = [&] { - auto body = crb.getBodyBuilder(); + auto body = replyBuilder->getBodyBuilder(); return CommandHelpers::extractOrAppendOk(body); }(); - behaviors.attachCurOpErrInfo(opCtx, crb.getBodyBuilder().asTempObj()); + behaviors.attachCurOpErrInfo(opCtx, replyBuilder->getBodyBuilder().asTempObj()); if (!ok) { - auto response = crb.getBodyBuilder().asTempObj(); + auto response = replyBuilder->getBodyBuilder().asTempObj(); auto codeField = response["code"]; if (codeField.isNumber()) { auto code = ErrorCodes::Error(codeField.numberInt()); // Append the error labels for transient transaction errors. auto errorLabels = getErrorLabels(sessionOptions, command->getName(), code); - crb.getBodyBuilder().appendElements(errorLabels); + replyBuilder->getBodyBuilder().appendElements(errorLabels); } } @@ -576,7 +574,7 @@ bool runCommandImpl(OperationContext* opCtx, appendReplyMetadata(opCtx, request, &metadataBob); { - auto commandBodyBob = crb.getBodyBuilder(); + auto commandBodyBob = replyBuilder->getBodyBuilder(); appendClusterAndOperationTime(opCtx, &commandBodyBob, &metadataBob, startOperationTime); } |