diff options
author | Adam Midvidy <amidvidy@gmail.com> | 2015-11-12 00:37:49 -0500 |
---|---|---|
committer | Adam Midvidy <amidvidy@gmail.com> | 2015-11-13 10:50:32 -0500 |
commit | 289d3fdf5c9d5b232b383c6cd871de1b75cf76b4 (patch) | |
tree | 58bcb98e92fb61e805674afbfb6db775959889dd /src | |
parent | d7511a9244ecab9ea894bb4e8767a289314a1e34 (diff) | |
download | mongo-289d3fdf5c9d5b232b383c6cd871de1b75cf76b4.tar.gz |
SERVER-20884 build command replies in-place to avoid copies
Diffstat (limited to 'src')
38 files changed, 232 insertions, 554 deletions
diff --git a/src/mongo/bson/bsonobjbuilder.h b/src/mongo/bson/bsonobjbuilder.h index 3713cf8c313..5b744a62120 100644 --- a/src/mongo/bson/bsonobjbuilder.h +++ b/src/mongo/bson/bsonobjbuilder.h @@ -97,6 +97,23 @@ public: _b.reserveBytes(1); } + // Tag for a special overload of BSONObjBuilder that allows the user to continue + // building in to an existing BufBuilder that has already been built in to. Use with caution. + struct ResumeBuildingTag {}; + + BSONObjBuilder(ResumeBuildingTag, BufBuilder& existingBuilder, std::size_t offset = 0) + : _b(existingBuilder), + _buf(0), + _offset(offset), + _s(this), + _tracker(nullptr), + _doneCalled(false) { + invariant(_b.len() >= BSONObj::kMinBSONLength); + _b.setlen(_b.len() - 1); // get rid of the previous EOO. + // Reserve space for our EOO. + _b.reserveBytes(1); + } + BSONObjBuilder(const BSONSizeTracker& tracker) : _b(_buf), _buf(sizeof(BSONObj::Holder) + tracker.getSize()), diff --git a/src/mongo/bson/bsonobjbuilder_test.cpp b/src/mongo/bson/bsonobjbuilder_test.cpp index 0f87d966fd8..d92d48d6098 100644 --- a/src/mongo/bson/bsonobjbuilder_test.cpp +++ b/src/mongo/bson/bsonobjbuilder_test.cpp @@ -38,12 +38,13 @@ namespace { -using std::string; -using std::stringstream; using mongo::BSONElement; using mongo::BSONObj; using mongo::BSONObjBuilder; using mongo::BSONType; +using mongo::BufBuilder; +using std::string; +using std::stringstream; const long long maxEncodableInt = (1 << 30) - 1; const long long minEncodableInt = -maxEncodableInt; @@ -264,4 +265,42 @@ TEST(BSONObjBuilderTest, AppendMaxTimestampConversion) { ASSERT_FALSE(timestamp.isNull()); } +TEST(BSONObjBuilderTest, ResumeBuilding) { + BufBuilder b; + { + BSONObjBuilder firstBuilder(b); + firstBuilder.append("a", "b"); + } + { + BSONObjBuilder secondBuilder(BSONObjBuilder::ResumeBuildingTag(), b); + secondBuilder.append("c", "d"); + } + auto obj = BSONObj(b.buf()); + ASSERT_EQ(obj, + BSON("a" + << "b" + << "c" + << "d")); +} + +TEST(BSONObjBuilderTest, ResumeBuildingWithNesting) { + BufBuilder b; + // build a trivial object. + { + BSONObjBuilder firstBuilder(b); + firstBuilder.append("ll", + BSON("f" << BSON("cc" + << "dd"))); + } + // add a complex field + { + BSONObjBuilder secondBuilder(BSONObjBuilder::ResumeBuildingTag(), b); + secondBuilder.append("a", BSON("c" << 3)); + } + auto obj = BSONObj(b.buf()); + ASSERT_EQ(obj, + BSON("ll" << BSON("f" << BSON("cc" + << "dd")) << "a" << BSON("c" << 3))); +} + } // unnamed namespace diff --git a/src/mongo/client/dbclient.cpp b/src/mongo/client/dbclient.cpp index e881e161c29..66a7abba021 100644 --- a/src/mongo/client/dbclient.cpp +++ b/src/mongo/client/dbclient.cpp @@ -307,8 +307,8 @@ rpc::UniqueReply DBClientWithCommands::runCommandWithMetadata(StringData databas requestBuilder->setDatabase(database); requestBuilder->setCommandName(command); - requestBuilder->setMetadata(metadataBob.done()); requestBuilder->setCommandArgs(commandArgs); + requestBuilder->setMetadata(metadataBob.done()); auto requestMsg = requestBuilder->done(); Message replyMsg; diff --git a/src/mongo/client/dbclientcursor.cpp b/src/mongo/client/dbclientcursor.cpp index 00eb939da30..208d1c01a6d 100644 --- a/src/mongo/client/dbclientcursor.cpp +++ b/src/mongo/client/dbclientcursor.cpp @@ -85,8 +85,8 @@ Message assembleCommandRequest(DBClientWithCommands* cli, // We need to get the command name from the upconverted command as it may have originally // been wrapped. requestBuilder->setCommandName(upconvertedCommand.firstElementFieldName()); - requestBuilder->setMetadata(metadataBob.done()); requestBuilder->setCommandArgs(std::move(upconvertedCommand)); + requestBuilder->setMetadata(metadataBob.done()); return requestBuilder->done(); } diff --git a/src/mongo/client/scoped_db_conn_test.cpp b/src/mongo/client/scoped_db_conn_test.cpp index bbc805dfbf2..a3494225648 100644 --- a/src/mongo/client/scoped_db_conn_test.cpp +++ b/src/mongo/client/scoped_db_conn_test.cpp @@ -119,9 +119,8 @@ public: commandResponse.append("minWireVersion", WireVersion::RELEASE_2_4_AND_BEFORE); } - - auto response = reply->setMetadata(rpc::makeEmptyMetadata()) - .setCommandReply(commandResponse.done()) + auto response = reply->setCommandReply(commandResponse.done()) + .setMetadata(rpc::makeEmptyMetadata()) .done(); port->reply(m, response); diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index fa601fae613..65560607e50 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -400,8 +400,8 @@ void Command::generateHelpResponse(OperationContext* txn, helpBuilder.append("help", ss.str()); helpBuilder.append("lockType", command.isWriteCommandForConfigServer() ? 1 : 0); - replyBuilder->setMetadata(rpc::makeEmptyMetadata()); replyBuilder->setCommandReply(helpBuilder.done()); + replyBuilder->setMetadata(rpc::makeEmptyMetadata()); } namespace { @@ -416,8 +416,6 @@ void _generateErrorResponse(OperationContext* txn, // so we need to reset it to a clean state just to be sure. replyBuilder->reset(); - replyBuilder->setMetadata(metadata); - // We need to include some extra information for SendStaleConfig. if (exception.getCode() == ErrorCodes::SendStaleConfig) { const SendStaleConfigException& scex = @@ -430,6 +428,8 @@ void _generateErrorResponse(OperationContext* txn, } else { replyBuilder->setCommandReply(exception.toStatus()); } + + replyBuilder->setMetadata(metadata); } } // namespace @@ -470,7 +470,7 @@ void runCommands(OperationContext* txn, const rpc::RequestInterface& request, rpc::ReplyBuilderInterface* replyBuilder) { try { - dassert(replyBuilder->getState() == rpc::ReplyBuilderInterface::State::kMetadata); + dassert(replyBuilder->getState() == rpc::ReplyBuilderInterface::State::kCommandReply); Command* c = nullptr; // In the absence of a Command object, no redaction is possible. Therefore diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp index d9bba0008ee..09075791f12 100644 --- a/src/mongo/db/dbcommands.cpp +++ b/src/mongo/db/dbcommands.cpp @@ -1200,7 +1200,7 @@ void Command::execCommand(OperationContext* txn, // see SERVER-18515 for details. uassertStatusOK(rpc::readRequestMetadata(txn, request.getMetadata())); - dassert(replyBuilder->getState() == rpc::ReplyBuilderInterface::State::kMetadata); + dassert(replyBuilder->getState() == rpc::ReplyBuilderInterface::State::kCommandReply); std::string dbname = request.getDatabase().toString(); unique_ptr<MaintenanceModeSetter> mmSetter; @@ -1337,7 +1337,7 @@ void Command::execCommand(OperationContext* txn, bool Command::run(OperationContext* txn, const rpc::RequestInterface& request, rpc::ReplyBuilderInterface* replyBuilder) { - BSONObjBuilder replyBuilderBob; + BSONObjBuilder inPlaceReplyBob(replyBuilder->getInPlaceReplyBuilder()); repl::ReplicationCoordinator* replCoord = repl::getGlobalReplicationCoordinator(); @@ -1346,9 +1346,10 @@ bool Command::run(OperationContext* txn, // parse and validate ReadConcernArgs auto readConcernParseStatus = readConcernArgs.initialize(request.getCommandArgs()); if (!readConcernParseStatus.isOK()) { - replyBuilder->setMetadata(rpc::makeEmptyMetadata()) - .setCommandReply(readConcernParseStatus); - return false; + auto result = appendCommandStatus(inPlaceReplyBob, readConcernParseStatus); + inPlaceReplyBob.doneFast(); + replyBuilder->setMetadata(rpc::makeEmptyMetadata()); + return result; } if (!supportsReadConcern()) { @@ -1356,23 +1357,27 @@ bool Command::run(OperationContext* txn, // readConcern regardless. if (!readConcernArgs.getOpTime().isNull() || readConcernArgs.getLevel() != repl::ReadConcernLevel::kLocalReadConcern) { - replyBuilder->setMetadata(rpc::makeEmptyMetadata()) - .setCommandReply({ErrorCodes::InvalidOptions, - str::stream() - << "Command " << name << " does not support " - << repl::ReadConcernArgs::kReadConcernFieldName}); - return false; + auto result = appendCommandStatus( + inPlaceReplyBob, + {ErrorCodes::InvalidOptions, + str::stream() << "Command " << name << " does not support " + << repl::ReadConcernArgs::kReadConcernFieldName}); + inPlaceReplyBob.doneFast(); + replyBuilder->setMetadata(rpc::makeEmptyMetadata()); + return result; } } else { // Skip waiting for the OpTime when testing snapshot behavior. if (!testingSnapshotBehaviorInIsolation) { // Wait for readConcern to be satisfied. auto readConcernResult = replCoord->waitUntilOpTime(txn, readConcernArgs); - readConcernResult.appendInfo(&replyBuilderBob); + readConcernResult.appendInfo(&inPlaceReplyBob); if (!readConcernResult.getStatus().isOK()) { - replyBuilder->setMetadata(rpc::makeEmptyMetadata()) - .setCommandReply(readConcernResult.getStatus(), replyBuilderBob.done()); - return false; + auto result = + appendCommandStatus(inPlaceReplyBob, readConcernResult.getStatus()); + inPlaceReplyBob.doneFast(); + replyBuilder->setMetadata(rpc::makeEmptyMetadata()); + return result; } } @@ -1389,8 +1394,10 @@ bool Command::run(OperationContext* txn, } if (!status.isOK()) { - replyBuilder->setMetadata(rpc::makeEmptyMetadata()).setCommandReply(status); - return false; + auto result = appendCommandStatus(inPlaceReplyBob, status); + inPlaceReplyBob.doneFast(); + replyBuilder->setMetadata(rpc::makeEmptyMetadata()); + return result; } } } @@ -1404,8 +1411,11 @@ bool Command::run(OperationContext* txn, // run expects const db std::string (can't bind to temporary) const std::string db = request.getDatabase().toString(); + // TODO: remove queryOptions parameter from command's run method. - bool result = this->run(txn, db, cmd, 0, errmsg, replyBuilderBob); + bool result = this->run(txn, db, cmd, 0, errmsg, inPlaceReplyBob); + appendCommandStatus(inPlaceReplyBob, result, errmsg); + inPlaceReplyBob.doneFast(); BSONObjBuilder metadataBob; @@ -1429,18 +1439,8 @@ bool Command::run(OperationContext* txn, rpc::ConfigServerMetadata(opTime).writeToMetadata(&metadataBob); } - auto cmdResponse = replyBuilderBob.done(); replyBuilder->setMetadata(metadataBob.done()); - if (result) { - replyBuilder->setCommandReply(std::move(cmdResponse)); - } else { - // maintain existing behavior of returning all data appended to builder - // even if command returned false - replyBuilder->setCommandReply(Status(ErrorCodes::CommandFailed, errmsg), - std::move(cmdResponse)); - } - return result; } diff --git a/src/mongo/db/dbwebserver.cpp b/src/mongo/db/dbwebserver.cpp index b60e1dd77c3..bee10b95a5b 100644 --- a/src/mongo/db/dbwebserver.cpp +++ b/src/mongo/db/dbwebserver.cpp @@ -274,10 +274,8 @@ public: rpc::CommandRequestBuilder requestBuilder{}; - requestBuilder.setDatabase("admin") - .setCommandName(cmd) - .setMetadata(rpc::makeEmptyMetadata()) - .setCommandArgs(cmdObj); + requestBuilder.setDatabase("admin").setCommandName(cmd).setCommandArgs(cmdObj).setMetadata( + rpc::makeEmptyMetadata()); auto cmdRequestMsg = requestBuilder.done(); rpc::CommandRequest cmdRequest{&cmdRequestMsg}; diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp index 4506a1f6a14..3a11fc79e5a 100644 --- a/src/mongo/db/instance.cpp +++ b/src/mongo/db/instance.cpp @@ -1096,8 +1096,8 @@ static void insertSystemIndexes(OperationContext* txn, DbMessage& d, CurOp& curO auto indexNs = NamespaceString(d.getns()); auto cmdRequestMsg = requestBuilder.setDatabase(indexNs.db()) .setCommandName("createIndexes") - .setMetadata(rpc::makeEmptyMetadata()) .setCommandArgs(cmdObj) + .setMetadata(rpc::makeEmptyMetadata()) .done(); rpc::LegacyRequest cmdRequest{&cmdRequestMsg}; rpc::LegacyReplyBuilder cmdReplyBuilder{}; diff --git a/src/mongo/dbtests/mock/mock_remote_db_server.cpp b/src/mongo/dbtests/mock/mock_remote_db_server.cpp index d4949e171d6..3b9b472012d 100644 --- a/src/mongo/dbtests/mock/mock_remote_db_server.cpp +++ b/src/mongo/dbtests/mock/mock_remote_db_server.cpp @@ -169,8 +169,8 @@ rpc::UniqueReply MockRemoteDBServer::runCommandWithMetadata(MockRemoteDBServer:: // We need to construct a reply message - it will always be read through a view so it // doesn't matter whether we use CommandReplBuilder or LegacyReplyBuilder auto message = rpc::CommandReplyBuilder{} - .setMetadata(rpc::makeEmptyMetadata()) .setCommandReply(reply) + .setMetadata(rpc::makeEmptyMetadata()) .done(); auto replyView = stdx::make_unique<rpc::CommandReply>(&message); return rpc::UniqueReply(std::move(message), std::move(replyView)); diff --git a/src/mongo/executor/async_mock_stream_factory.cpp b/src/mongo/executor/async_mock_stream_factory.cpp index c47f04f388a..2a2d8322b59 100644 --- a/src/mongo/executor/async_mock_stream_factory.cpp +++ b/src/mongo/executor/async_mock_stream_factory.cpp @@ -302,8 +302,8 @@ void AsyncMockStreamFactory::MockStream::simulateServer( } auto replyBuilder = rpc::makeReplyBuilder(proto); - replyBuilder->setMetadata(resp.metadata); replyBuilder->setCommandReply(resp.data); + replyBuilder->setMetadata(resp.metadata); auto replyMsg = replyBuilder->done(); replyMsg.header().setResponseTo(messageId); diff --git a/src/mongo/executor/connection_pool_asio.cpp b/src/mongo/executor/connection_pool_asio.cpp index 2b475016057..b2344b54ffa 100644 --- a/src/mongo/executor/connection_pool_asio.cpp +++ b/src/mongo/executor/connection_pool_asio.cpp @@ -167,8 +167,8 @@ Message ASIOConnection::makeIsMasterRequest(ASIOConnection* conn) { rpc::LegacyRequestBuilder requestBuilder{}; requestBuilder.setDatabase("admin"); requestBuilder.setCommandName("isMaster"); - requestBuilder.setMetadata(rpc::makeEmptyMetadata()); requestBuilder.setCommandArgs(BSON("isMaster" << 1)); + requestBuilder.setMetadata(rpc::makeEmptyMetadata()); return requestBuilder.done(); } diff --git a/src/mongo/executor/network_interface_asio_auth.cpp b/src/mongo/executor/network_interface_asio_auth.cpp index 42a3b8e7780..8a7eddad2fa 100644 --- a/src/mongo/executor/network_interface_asio_auth.cpp +++ b/src/mongo/executor/network_interface_asio_auth.cpp @@ -57,7 +57,6 @@ void NetworkInterfaceASIO::_runIsMaster(AsyncOp* op) { rpc::LegacyRequestBuilder requestBuilder{}; requestBuilder.setDatabase("admin"); requestBuilder.setCommandName("isMaster"); - requestBuilder.setMetadata(rpc::makeEmptyMetadata()); BSONObjBuilder bob; bob.append("isMaster", 1); @@ -72,6 +71,7 @@ void NetworkInterfaceASIO::_runIsMaster(AsyncOp* op) { } requestBuilder.setCommandArgs(bob.done()); + requestBuilder.setMetadata(rpc::makeEmptyMetadata()); // Set current command to ismaster request and run auto beginStatus = op->beginCommand( diff --git a/src/mongo/executor/network_interface_asio_operation.cpp b/src/mongo/executor/network_interface_asio_operation.cpp index 21929559e5c..00395e25f59 100644 --- a/src/mongo/executor/network_interface_asio_operation.cpp +++ b/src/mongo/executor/network_interface_asio_operation.cpp @@ -81,8 +81,8 @@ StatusWith<Message> messageFromRequest(const RemoteCommandRequest& request, auto toSend = rpc::makeRequestBuilder(protocol) ->setDatabase(request.dbname) .setCommandName(request.cmdObj.firstElementFieldName()) - .setMetadata(maybeAugmented) .setCommandArgs(request.cmdObj) + .setMetadata(maybeAugmented) .done(); return std::move(toSend); } diff --git a/src/mongo/executor/network_interface_asio_test.cpp b/src/mongo/executor/network_interface_asio_test.cpp index 33eb9e71482..21f8be0e280 100644 --- a/src/mongo/executor/network_interface_asio_test.cpp +++ b/src/mongo/executor/network_interface_asio_test.cpp @@ -420,8 +420,8 @@ public: // Build a mock reply message auto replyBuilder = rpc::makeReplyBuilder(rpc::Protocol::kOpCommandV1); - replyBuilder->setMetadata(BSONObj()); replyBuilder->setCommandReply(BSON("hello!" << 1)); + replyBuilder->setMetadata(BSONObj()); auto message = replyBuilder->done(); message.header().setResponseTo(messageId); diff --git a/src/mongo/rpc/command_reply.cpp b/src/mongo/rpc/command_reply.cpp index b2472230e52..b6d3a98586f 100644 --- a/src/mongo/rpc/command_reply.cpp +++ b/src/mongo/rpc/command_reply.cpp @@ -52,8 +52,8 @@ CommandReply::CommandReply(const Message* message) : _message(message) { const char* messageEnd = begin + length; ConstDataRangeCursor cur(begin, messageEnd); - _metadata = uassertStatusOK(cur.readAndAdvance<Validated<BSONObj>>()).val; _commandReply = uassertStatusOK(cur.readAndAdvance<Validated<BSONObj>>()).val; + _metadata = uassertStatusOK(cur.readAndAdvance<Validated<BSONObj>>()).val; _outputDocs = DocumentRange(cur.data(), messageEnd); } diff --git a/src/mongo/rpc/command_reply_builder.cpp b/src/mongo/rpc/command_reply_builder.cpp index 65744efa49f..b1b128e198c 100644 --- a/src/mongo/rpc/command_reply_builder.cpp +++ b/src/mongo/rpc/command_reply_builder.cpp @@ -46,44 +46,39 @@ CommandReplyBuilder::CommandReplyBuilder(Message&& message) : _message{std::move _builder.skip(mongo::MsgData::MsgDataHeaderSize); } -CommandReplyBuilder& CommandReplyBuilder::setMetadata(const BSONObj& metadata) { - invariant(_state == State::kMetadata); +CommandReplyBuilder& CommandReplyBuilder::setRawCommandReply(const BSONObj& commandReply) { + invariant(_state == State::kCommandReply); - metadata.appendSelfToBufBuilder(_builder); - _state = State::kCommandReply; + commandReply.appendSelfToBufBuilder(_builder); + _state = State::kMetadata; return *this; } -CommandReplyBuilder& CommandReplyBuilder::setRawCommandReply(const BSONObj& commandReply) { +BufBuilder& CommandReplyBuilder::getInPlaceReplyBuilder() { invariant(_state == State::kCommandReply); + _state = State::kMetadata; + return _builder; +} - commandReply.appendSelfToBufBuilder(_builder); +CommandReplyBuilder& CommandReplyBuilder::setMetadata(const BSONObj& metadata) { + invariant(_state == State::kMetadata); + metadata.appendSelfToBufBuilder(_builder); _state = State::kOutputDocs; return *this; } + Status CommandReplyBuilder::addOutputDocs(DocumentRange outputDocs) { invariant(_state == State::kOutputDocs); auto rangeData = outputDocs.data(); auto dataSize = rangeData.length(); - auto hasSpace = _hasSpaceFor(dataSize); - if (!hasSpace.isOK()) { - return hasSpace; - } - + invariant(_state == State::kOutputDocs); _builder.appendBuf(rangeData.data(), dataSize); return Status::OK(); } Status CommandReplyBuilder::addOutputDoc(const BSONObj& outputDoc) { invariant(_state == State::kOutputDocs); - - auto dataSize = static_cast<std::size_t>(outputDoc.objsize()); - auto hasSpace = _hasSpaceFor(dataSize); - if (!hasSpace.isOK()) { - return hasSpace; - } - outputDoc.appendSelfToBufBuilder(_builder); return Status::OK(); } @@ -97,15 +92,15 @@ Protocol CommandReplyBuilder::getProtocol() const { } void CommandReplyBuilder::reset() { - // If we are in State::kMetadata, we are already in the 'start' state, so by + // If we are in State::kCommandReply, we are already in the 'start' state, so by // immediately returning, we save a heap allocation. - if (_state == State::kMetadata) { + if (_state == State::kCommandReply) { return; } _builder.reset(); _builder.skip(mongo::MsgData::MsgDataHeaderSize); _message.reset(); - _state = State::kMetadata; + _state = State::kCommandReply; } Message CommandReplyBuilder::done() { @@ -119,23 +114,5 @@ Message CommandReplyBuilder::done() { return std::move(_message); } -std::size_t CommandReplyBuilder::availableBytes() const { - int intLen = _builder.len(); - invariant(0 <= intLen); - std::size_t len = static_cast<std::size_t>(intLen); - invariant(len <= mongo::MaxMessageSizeBytes); - return mongo::MaxMessageSizeBytes - len; -} - -Status CommandReplyBuilder::_hasSpaceFor(std::size_t dataSize) const { - size_t availBytes = availableBytes(); - if (availBytes < dataSize) { - return Status(ErrorCodes::Overflow, - str::stream() << "Not enough space to store " << dataSize << " bytes. Only " - << availBytes << " bytes are available."); - } - return Status::OK(); -} - } // rpc } // mongo diff --git a/src/mongo/rpc/command_reply_builder.h b/src/mongo/rpc/command_reply_builder.h index b91b335fcdc..2079ae69790 100644 --- a/src/mongo/rpc/command_reply_builder.h +++ b/src/mongo/rpc/command_reply_builder.h @@ -56,8 +56,11 @@ public: */ CommandReplyBuilder(Message&& message); - CommandReplyBuilder& setMetadata(const BSONObj& metadata) final; + CommandReplyBuilder& setRawCommandReply(const BSONObj& commandReply) final; + BufBuilder& getInPlaceReplyBuilder() final; + + CommandReplyBuilder& setMetadata(const BSONObj& metadata) final; Status addOutputDocs(DocumentRange outputDocs) final; Status addOutputDoc(const BSONObj& outputDoc) final; @@ -75,19 +78,11 @@ public: */ Message done() final; - std::size_t availableBytes() const final; - private: - /** - * Checks if there is enough space in the buffer to store dataSize bytes - * and computes error message if not. - */ - Status _hasSpaceFor(std::size_t dataSize) const; - // Default values are all empty. BufBuilder _builder{}; Message _message; - State _state{State::kMetadata}; + State _state{State::kCommandReply}; }; } // namespace rpc diff --git a/src/mongo/rpc/command_reply_test.cpp b/src/mongo/rpc/command_reply_test.cpp index b5d372b42be..ba12fbe9748 100644 --- a/src/mongo/rpc/command_reply_test.cpp +++ b/src/mongo/rpc/command_reply_test.cpp @@ -77,16 +77,16 @@ protected: }; TEST_F(ReplyTest, ParseAllFields) { - BSONObjBuilder metadataBob{}; - metadataBob.append("foo", "bar"); - auto metadata = metadataBob.done(); - writeObj(metadata); - BSONObjBuilder commandReplyBob{}; commandReplyBob.append("baz", "garply"); auto commandReply = commandReplyBob.done(); writeObj(commandReply); + BSONObjBuilder metadataBob{}; + metadataBob.append("foo", "bar"); + auto metadata = metadataBob.done(); + writeObj(metadata); + BSONObjBuilder outputDoc1Bob{}; outputDoc1Bob.append("meep", "boop").append("meow", "chirp"); auto outputDoc1 = outputDoc1Bob.done(); diff --git a/src/mongo/rpc/command_request.cpp b/src/mongo/rpc/command_request.cpp index 80f0cfbe59e..ad9316b1c4d 100644 --- a/src/mongo/rpc/command_request.cpp +++ b/src/mongo/rpc/command_request.cpp @@ -94,9 +94,9 @@ CommandRequest::CommandRequest(const Message* message) : _message(message) { Validated<BSONObj> obj; uassertStatusOK(cur.readAndAdvance<>(&obj)); - _metadata = std::move(obj.val); - uassertStatusOK(cur.readAndAdvance<>(&obj)); _commandArgs = std::move(obj.val); + uassertStatusOK(cur.readAndAdvance<>(&obj)); + _metadata = std::move(obj.val); _inputDocs = DocumentRange{cur.data(), messageEnd}; } diff --git a/src/mongo/rpc/command_request.h b/src/mongo/rpc/command_request.h index bd29ca30597..a39d7374533 100644 --- a/src/mongo/rpc/command_request.h +++ b/src/mongo/rpc/command_request.h @@ -42,8 +42,6 @@ namespace rpc { /** * An immutable view of an OP_COMMAND message. The underlying bytes are owned * by a mongo::Message, which must outlive any Reply instances created from it. - * - * TODO: BSON validation. See SERVER-18167 for details. */ class CommandRequest : public RequestInterface { public: @@ -97,8 +95,8 @@ private: const Message* _message; StringData _database; StringData _commandName; - BSONObj _metadata; BSONObj _commandArgs; + BSONObj _metadata; DocumentRange _inputDocs; }; diff --git a/src/mongo/rpc/command_request_builder.cpp b/src/mongo/rpc/command_request_builder.cpp index 60a013d218f..3945b6219a4 100644 --- a/src/mongo/rpc/command_request_builder.cpp +++ b/src/mongo/rpc/command_request_builder.cpp @@ -57,13 +57,6 @@ CommandRequestBuilder& CommandRequestBuilder::setDatabase(StringData database) { CommandRequestBuilder& CommandRequestBuilder::setCommandName(StringData commandName) { invariant(_state == State::kCommandName); _builder.appendStr(commandName); - _state = State::kMetadata; - return *this; -} - -CommandRequestBuilder& CommandRequestBuilder::setMetadata(BSONObj metadata) { - invariant(_state == State::kMetadata); - metadata.appendSelfToBufBuilder(_builder); _state = State::kCommandArgs; return *this; } @@ -71,6 +64,13 @@ CommandRequestBuilder& CommandRequestBuilder::setMetadata(BSONObj metadata) { CommandRequestBuilder& CommandRequestBuilder::setCommandArgs(BSONObj commandArgs) { invariant(_state == State::kCommandArgs); commandArgs.appendSelfToBufBuilder(_builder); + _state = State::kMetadata; + return *this; +} + +CommandRequestBuilder& CommandRequestBuilder::setMetadata(BSONObj metadata) { + invariant(_state == State::kMetadata); + metadata.appendSelfToBufBuilder(_builder); _state = State::kInputDocs; return *this; } diff --git a/src/mongo/rpc/command_request_builder.h b/src/mongo/rpc/command_request_builder.h index 50b3f69fab2..113bf691e14 100644 --- a/src/mongo/rpc/command_request_builder.h +++ b/src/mongo/rpc/command_request_builder.h @@ -62,10 +62,10 @@ public: CommandRequestBuilder& setCommandName(StringData commandName) final; - CommandRequestBuilder& setMetadata(BSONObj metadata) final; - CommandRequestBuilder& setCommandArgs(BSONObj commandArgs) final; + CommandRequestBuilder& setMetadata(BSONObj metadata) final; + CommandRequestBuilder& addInputDocs(DocumentRange inputDocs) final; CommandRequestBuilder& addInputDoc(BSONObj inputDoc) final; diff --git a/src/mongo/rpc/command_request_builder_test.cpp b/src/mongo/rpc/command_request_builder_test.cpp index 5972a41752f..d5bcb62669f 100644 --- a/src/mongo/rpc/command_request_builder_test.cpp +++ b/src/mongo/rpc/command_request_builder_test.cpp @@ -73,8 +73,8 @@ TEST(RequestBuilder, RoundTrip) { auto msg = r.setDatabase(databaseName) .setCommandName(commandName) - .setMetadata(metadata) .setCommandArgs(commandArgs) + .setMetadata(metadata) .addInputDocs(inputDocRange) .done(); diff --git a/src/mongo/rpc/command_request_test.cpp b/src/mongo/rpc/command_request_test.cpp index 18dbc2edc66..f22923d954d 100644 --- a/src/mongo/rpc/command_request_test.cpp +++ b/src/mongo/rpc/command_request_test.cpp @@ -64,16 +64,16 @@ TEST(CommandRequest, ParseAllFields) { auto commandName = std::string{"abababa"}; writeString(commandName); - BSONObjBuilder metadataBob{}; - metadataBob.append("foo", "bar"); - auto metadata = metadataBob.done(); - writeObj(metadata); - BSONObjBuilder commandArgsBob{}; commandArgsBob.append("baz", "garply"); auto commandArgs = commandArgsBob.done(); writeObj(commandArgs); + BSONObjBuilder metadataBob{}; + metadataBob.append("foo", "bar"); + auto metadata = metadataBob.done(); + writeObj(metadata); + BSONObjBuilder inputDoc1Bob{}; inputDoc1Bob.append("meep", "boop").append("meow", "chirp"); auto inputDoc1 = inputDoc1Bob.done(); @@ -112,8 +112,8 @@ TEST(CommandRequest, InvalidNSThrows) { rpc::CommandRequestBuilder crb; crb.setDatabase("foo////!!!!<><><>"); crb.setCommandName("foo"); - crb.setMetadata(BSONObj()); crb.setCommandArgs(BSON("ping" << 1)); + crb.setMetadata(BSONObj()); auto msg = crb.done(); ASSERT_THROWS(rpc::CommandRequest{&msg}, AssertionException); } diff --git a/src/mongo/rpc/legacy_reply.cpp b/src/mongo/rpc/legacy_reply.cpp index 60d68008558..e298a97814f 100644 --- a/src/mongo/rpc/legacy_reply.cpp +++ b/src/mongo/rpc/legacy_reply.cpp @@ -67,27 +67,7 @@ LegacyReply::LegacyReply(const Message* message) : _message(std::move(message)) std::tie(_commandReply, _metadata) = uassertStatusOK(rpc::upconvertReplyMetadata(BSONObj(qr.data()))); - // Copy the bson array of documents from the message into - // a contiguous area of memory owned by _docBuffer so - // DocumentRange can be used to iterate over documents - auto cursorElem = _commandReply[LegacyReplyBuilder::kCursorTag]; - if (cursorElem.eoo()) - return; - - BSONObj cursorObj = cursorElem.Obj(); - auto firstBatchElem = cursorObj[LegacyReplyBuilder::kFirstBatchTag]; - if (firstBatchElem.eoo()) - return; - - for (BSONObjIterator it(firstBatchElem.Obj()); it.more(); it.next()) { - invariant((*it).isABSONObj()); - BSONObj doc = (*it).Obj(); - doc.appendSelfToBufBuilder(_docBuffer); - } - const char* dataBegin = _docBuffer.buf(); - const char* dataEnd = dataBegin + _docBuffer.len(); - _outputDocs = DocumentRange(dataBegin, dataEnd); - + _outputDocs = DocumentRange{}; return; } diff --git a/src/mongo/rpc/legacy_reply_builder.cpp b/src/mongo/rpc/legacy_reply_builder.cpp index e4830c10f2a..efa50d8f82c 100644 --- a/src/mongo/rpc/legacy_reply_builder.cpp +++ b/src/mongo/rpc/legacy_reply_builder.cpp @@ -35,6 +35,7 @@ #include "mongo/db/dbmessage.h" #include "mongo/db/jsobj.h" #include "mongo/rpc/metadata.h" +#include "mongo/rpc/metadata/sharding_metadata.h" #include "mongo/stdx/memory.h" #include "mongo/util/assert_util.h" #include "mongo/util/mongoutils/str.h" @@ -42,111 +43,54 @@ namespace mongo { namespace rpc { -namespace { -// Margin of error for availableBytes size estimate -std::size_t kReservedSize = 1024; - -bool isEmptyCommandReply(const BSONObj& bson) { - auto cursorElem = bson[LegacyReplyBuilder::kCursorTag]; - if (cursorElem.eoo()) - return true; - - BSONObj cursorObj = cursorElem.Obj(); - auto firstBatchElem = cursorObj[LegacyReplyBuilder::kFirstBatchTag]; - if (firstBatchElem.eoo()) - return true; - - BSONObjIterator it(firstBatchElem.Obj()); - - return !it.more(); -} -} // namespace - -const char LegacyReplyBuilder::kCursorTag[] = "cursor"; -const char LegacyReplyBuilder::kFirstBatchTag[] = "firstBatch"; - LegacyReplyBuilder::LegacyReplyBuilder() : LegacyReplyBuilder(Message()) {} -LegacyReplyBuilder::LegacyReplyBuilder(Message&& message) - : _currentLength{kReservedSize}, _message{std::move(message)} {} +LegacyReplyBuilder::LegacyReplyBuilder(Message&& message) : _message{std::move(message)} { + _builder.skip(sizeof(QueryResult::Value)); +} LegacyReplyBuilder::~LegacyReplyBuilder() {} -LegacyReplyBuilder& LegacyReplyBuilder::setMetadata(const BSONObj& metadata) { - invariant(_state == State::kMetadata); - - auto dataSize = static_cast<std::size_t>(metadata.objsize()); - - _currentLength += dataSize; - _metadata = metadata.getOwned(); - _state = State::kCommandReply; +LegacyReplyBuilder& LegacyReplyBuilder::setRawCommandReply(const BSONObj& commandReply) { + invariant(_state == State::kCommandReply); + commandReply.appendSelfToBufBuilder(_builder); + _state = State::kMetadata; return *this; } -LegacyReplyBuilder& LegacyReplyBuilder::setRawCommandReply(const BSONObj& commandReply) { +BufBuilder& LegacyReplyBuilder::getInPlaceReplyBuilder() { invariant(_state == State::kCommandReply); + _state = State::kMetadata; + return _builder; +} - auto dataSize = static_cast<std::size_t>(commandReply.objsize()); - - _currentLength += dataSize; - _commandReply = commandReply.getOwned(); - _allowAddingOutputDocs = isEmptyCommandReply(_commandReply); - +LegacyReplyBuilder& LegacyReplyBuilder::setMetadata(const BSONObj& metadata) { + invariant(_state == State::kMetadata); + // HACK: the only thing we need to downconvert is ShardingMetadata, which can go at the end of + // the object. So we do that in place to avoid copying the command reply. + auto shardingMetadata = rpc::ShardingMetadata::readFromMetadata(metadata); + invariant(shardingMetadata.isOK() || shardingMetadata.getStatus() == ErrorCodes::NoSuchKey); + + if (shardingMetadata.isOK()) { + // Write the sharding metadata in to the end of the object. The third parameter is needed + // because we already have skipped some bytes for the message header. + BSONObjBuilder resumedBuilder( + BSONObjBuilder::ResumeBuildingTag(), _builder, sizeof(QueryResult::Value)); + shardingMetadata.getValue().writeToMetadata(&resumedBuilder); + } _state = State::kOutputDocs; return *this; } Status LegacyReplyBuilder::addOutputDocs(DocumentRange docs) { invariant(_state == State::kOutputDocs); - invariant(_allowAddingOutputDocs); - - auto dataSize = docs.data().length(); - - auto hasSpace = _hasSpaceFor(dataSize); - if (!hasSpace.isOK()) { - return hasSpace; - } - - // The temporary obj is used to address the case when where is not enough space. - // BSONArray overhead can not be estimated upfront. - std::vector<BSONObj> docsTmp{}; - std::size_t lenTmp = 0; - std::size_t tmpIndex(_currentIndex); - for (auto&& it : docs) { - docsTmp.emplace_back(it.getOwned()); - lenTmp += BSONObjBuilder::numStr(++tmpIndex).length() + 2; // space for storing array index - } - - hasSpace = _hasSpaceFor(dataSize + lenTmp); - if (!hasSpace.isOK()) { - return hasSpace; - } - - // vector::insert instead of swap allows to call addOutputDoc(s) multiple times - _outputDocs.insert(_outputDocs.end(), - std::make_move_iterator(docsTmp.begin()), - std::make_move_iterator(docsTmp.end())); - - _currentIndex = tmpIndex; - _currentLength += lenTmp; - _currentLength += dataSize; + // no op return Status::OK(); } Status LegacyReplyBuilder::addOutputDoc(const BSONObj& bson) { invariant(_state == State::kOutputDocs); - invariant(_allowAddingOutputDocs); - - auto dataSize = static_cast<std::size_t>(bson.objsize()); - auto hasSpace = _hasSpaceFor(dataSize); - if (!hasSpace.isOK()) { - return hasSpace; - } - - _outputDocs.emplace_back(bson.getOwned()); - _currentLength += dataSize; - _currentLength += BSONObjBuilder::numStr(++_currentIndex).length() + 2; // storing array index - + // no op return Status::OK(); } @@ -161,87 +105,34 @@ Protocol LegacyReplyBuilder::getProtocol() const { void LegacyReplyBuilder::reset() { // If we are in State::kMetadata, we are already in the 'start' state, so by // immediately returning, we save a heap allocation. - if (_state == State::kMetadata) { + if (_state == State::kCommandReply) { return; } + _builder.reset(); + _builder.skip(sizeof(QueryResult::Value)); _message.reset(); - _currentLength = kReservedSize; - _currentIndex = 0U; - _state = State::kMetadata; + _state = State::kCommandReply; } Message LegacyReplyBuilder::done() { invariant(_state == State::kOutputDocs); - BSONObj reply = uassertStatusOK(rpc::downconvertReplyMetadata(_commandReply, _metadata)); - - BufBuilder bufBuilder; - bufBuilder.skip(sizeof(QueryResult::Value)); - - if (_allowAddingOutputDocs) { - BSONObjBuilder topBuilder(bufBuilder); - for (const auto& el : reply) { - if (kCursorTag != el.fieldNameStringData()) { - topBuilder.append(el); - continue; - } - invariant(el.isABSONObj()); - BSONObjBuilder curBuilder(topBuilder.subobjStart(kCursorTag)); - for (const auto& insideEl : el.Obj()) { - if (kFirstBatchTag != insideEl.fieldNameStringData()) { - curBuilder.append(insideEl); - continue; - } - invariant(insideEl.isABSONObj()); - BSONArrayBuilder arrBuilder(curBuilder.subarrayStart(kFirstBatchTag)); - for (const auto& doc : _outputDocs) { - arrBuilder.append(doc); - } - arrBuilder.doneFast(); - } - curBuilder.doneFast(); - } - topBuilder.doneFast(); - } else { - reply.appendSelfToBufBuilder(bufBuilder); - } - - auto msgHeaderSz = static_cast<std::size_t>(MsgData::MsgDataHeaderSize); - - invariant(static_cast<std::size_t>(bufBuilder.len()) + msgHeaderSz <= - mongo::MaxMessageSizeBytes); - - QueryResult::View qr = bufBuilder.buf(); + QueryResult::View qr = _builder.buf(); qr.setResultFlagsToOk(); - qr.msgdata().setLen(bufBuilder.len()); + qr.msgdata().setLen(_builder.len()); qr.msgdata().setOperation(opReply); qr.setCursorId(0); qr.setStartingFrom(0); qr.setNReturned(1); _message.setData(qr.view2ptr(), true); - bufBuilder.decouple(); + _builder.decouple(); _state = State::kDone; return std::move(_message); } -std::size_t LegacyReplyBuilder::availableBytes() const { - std::size_t msgHeaderSz = static_cast<std::size_t>(MsgData::MsgDataHeaderSize); - return mongo::MaxMessageSizeBytes - _currentLength - msgHeaderSz; -} - -Status LegacyReplyBuilder::_hasSpaceFor(std::size_t dataSize) const { - size_t availBytes = availableBytes(); - if (availBytes < dataSize) { - return Status(ErrorCodes::Overflow, - str::stream() << "Not enough space to store " << dataSize << " bytes. Only " - << availBytes << " bytes are available."); - } - return Status::OK(); -} - } // namespace rpc } // namespace mongo diff --git a/src/mongo/rpc/legacy_reply_builder.h b/src/mongo/rpc/legacy_reply_builder.h index 814fb9096e1..de229970360 100644 --- a/src/mongo/rpc/legacy_reply_builder.h +++ b/src/mongo/rpc/legacy_reply_builder.h @@ -49,9 +49,13 @@ public: LegacyReplyBuilder(Message&&); ~LegacyReplyBuilder() final; - LegacyReplyBuilder& setMetadata(const BSONObj& metadata) final; + LegacyReplyBuilder& setRawCommandReply(const BSONObj& commandReply) final; + BufBuilder& getInPlaceReplyBuilder() final; + + LegacyReplyBuilder& setMetadata(const BSONObj& metadata) final; + Status addOutputDocs(DocumentRange outputDocs) final; Status addOutputDoc(const BSONObj& outputDoc) final; @@ -63,26 +67,10 @@ public: Protocol getProtocol() const final; - std::size_t availableBytes() const final; - private: - /** - * Checks if there is enough space in the buffer to store dataSize bytes - * and computes error message if not. - */ - Status _hasSpaceFor(std::size_t dataSize) const; - - // If _allowAddingOutputDocs is false it enforces the "legacy" semantic - // where command results are returned inside commandReply. - // In this case calling addOutputDoc(s) will break the invariant. - bool _allowAddingOutputDocs{true}; - BSONObj _commandReply{}; - std::size_t _currentLength; - std::size_t _currentIndex = 0U; + BufBuilder _builder{}; Message _message; - BSONObj _metadata{}; - std::vector<BSONObj> _outputDocs{}; - State _state{State::kMetadata}; + State _state{State::kCommandReply}; }; } // namespace rpc diff --git a/src/mongo/rpc/legacy_request_builder.cpp b/src/mongo/rpc/legacy_request_builder.cpp index 33f66a4811f..c3a78803db3 100644 --- a/src/mongo/rpc/legacy_request_builder.cpp +++ b/src/mongo/rpc/legacy_request_builder.cpp @@ -60,25 +60,24 @@ LegacyRequestBuilder& LegacyRequestBuilder::setDatabase(StringData database) { LegacyRequestBuilder& LegacyRequestBuilder::setCommandName(StringData commandName) { invariant(_state == State::kCommandName); // no op, as commandName is the first element of commandArgs - _state = State::kMetadata; - return *this; -} - -LegacyRequestBuilder& LegacyRequestBuilder::setMetadata(BSONObj metadata) { - invariant(_state == State::kMetadata); - _metadata = std::move(metadata); _state = State::kCommandArgs; return *this; } LegacyRequestBuilder& LegacyRequestBuilder::setCommandArgs(BSONObj commandArgs) { invariant(_state == State::kCommandArgs); + _commandArgs = std::move(commandArgs); + _state = State::kMetadata; + return *this; +} +LegacyRequestBuilder& LegacyRequestBuilder::setMetadata(BSONObj metadata) { + invariant(_state == State::kMetadata); BSONObj legacyCommandArgs; int queryOptions; std::tie(legacyCommandArgs, queryOptions) = uassertStatusOK( - rpc::downconvertRequestMetadata(std::move(commandArgs), std::move(_metadata))); + rpc::downconvertRequestMetadata(std::move(_commandArgs), std::move(metadata))); _builder.appendNum(queryOptions); // queryOptions _builder.appendStr(_ns); diff --git a/src/mongo/rpc/legacy_request_builder.h b/src/mongo/rpc/legacy_request_builder.h index d20011cc443..7b738098c41 100644 --- a/src/mongo/rpc/legacy_request_builder.h +++ b/src/mongo/rpc/legacy_request_builder.h @@ -64,9 +64,9 @@ private: Message _message; BufBuilder _builder{}; - // we need to stash this as we need commandArgs to + // we need to stash this as we need metadata to // upconvert. - BSONObj _metadata; + BSONObj _commandArgs; std::string _ns{}; // copied to in setDatabase diff --git a/src/mongo/rpc/legacy_request_test.cpp b/src/mongo/rpc/legacy_request_test.cpp index 1413a139d71..82e89cb8641 100644 --- a/src/mongo/rpc/legacy_request_test.cpp +++ b/src/mongo/rpc/legacy_request_test.cpp @@ -43,8 +43,8 @@ TEST(LegacyRequest, InvalidNSThrows) { rpc::LegacyRequestBuilder crb; crb.setDatabase("foo////!!!!<><><>"); crb.setCommandName("foo"); - crb.setMetadata(BSONObj()); crb.setCommandArgs(BSON("ping" << 1)); + crb.setMetadata(BSONObj()); auto msg = crb.done(); ASSERT_THROWS(rpc::LegacyRequest{&msg}, AssertionException); } diff --git a/src/mongo/rpc/reply_builder_interface.cpp b/src/mongo/rpc/reply_builder_interface.cpp index 5949b405a54..63e147aab05 100644 --- a/src/mongo/rpc/reply_builder_interface.cpp +++ b/src/mongo/rpc/reply_builder_interface.cpp @@ -48,13 +48,16 @@ const char kErrorField[] = "errmsg"; // similar to appendCommandStatus (duplicating logic here to avoid cyclic library // dependency) BSONObj augmentReplyWithStatus(const Status& status, const BSONObj& reply) { + auto okField = reply.getField(kOKField); + if (!okField.eoo() && okField.trueValue()) { + return reply; + } + BSONObjBuilder bob; bob.appendElements(reply); - - if (!reply.hasField(kOKField)) { + if (okField.eoo()) { bob.append(kOKField, status.isOK() ? 1.0 : 0.0); } - if (status.isOK()) { return bob.obj(); } diff --git a/src/mongo/rpc/reply_builder_interface.h b/src/mongo/rpc/reply_builder_interface.h index 5f3f2621746..ec0195df32e 100644 --- a/src/mongo/rpc/reply_builder_interface.h +++ b/src/mongo/rpc/reply_builder_interface.h @@ -32,6 +32,7 @@ #include "mongo/base/disallow_copying.h" #include "mongo/base/status.h" +#include "mongo/bson/util/builder.h" #include "mongo/rpc/protocol.h" namespace mongo { @@ -59,7 +60,6 @@ public: virtual ~ReplyBuilderInterface() = default; - virtual ReplyBuilderInterface& setMetadata(const BSONObj& metadata) = 0; /** * Sets the raw command reply. This should probably not be used in favor of the @@ -68,6 +68,13 @@ public: virtual ReplyBuilderInterface& setRawCommandReply(const BSONObj& reply) = 0; /** + * Returns a BufBuilder suitable for building a command reply in place. + */ + virtual BufBuilder& getInPlaceReplyBuilder() = 0; + + virtual ReplyBuilderInterface& setMetadata(const BSONObj& metadata) = 0; + + /** * Sets the reply for this command. If an engaged StatusWith<BSONObj> is passed, the command * reply will be set to the contained BSONObj, augmented with the element {ok, 1.0} if it * does not already have an "ok" field. If a disengaged StatusWith<BSONObj> is passed, the @@ -122,12 +129,6 @@ public: virtual void reset() = 0; /** - * Returns available space in bytes, should be used to verify that the message have enough - * space for ouput documents. - */ - virtual std::size_t availableBytes() const = 0; - - /** * Writes data then transfers ownership of the message to the caller. The behavior of * calling any methods on the builder is subsequently undefined. */ diff --git a/src/mongo/rpc/reply_builder_test.cpp b/src/mongo/rpc/reply_builder_test.cpp index 9ae3544aba5..fb7e8c1ed9e 100644 --- a/src/mongo/rpc/reply_builder_test.cpp +++ b/src/mongo/rpc/reply_builder_test.cpp @@ -43,8 +43,6 @@ namespace { using namespace mongo; -void testMaxCommandReply(rpc::ReplyBuilderInterface& replyBuilder); - template <typename T> void testRoundTrip(rpc::ReplyBuilderInterface& replyBuilder); @@ -92,8 +90,8 @@ TEST(CommandReplyBuilder, MemAccess) { BSONObj metadata = buildMetadata(); BSONObj commandReply = buildCommand(); rpc::CommandReplyBuilder replyBuilder; - replyBuilder.setMetadata(metadata); replyBuilder.setCommandReply(commandReply); + replyBuilder.setMetadata(metadata); auto msg = replyBuilder.done(); rpc::CommandReply parsed(&msg); @@ -106,8 +104,8 @@ TEST(LegacyReplyBuilder, MemAccess) { BSONObj metadata = buildMetadata(); BSONObj commandReply = buildEmptyCommand(); rpc::LegacyReplyBuilder replyBuilder; - replyBuilder.setMetadata(metadata); replyBuilder.setCommandReply(commandReply); + replyBuilder.setMetadata(metadata); auto msg = replyBuilder.done(); rpc::LegacyReply parsed(&msg); @@ -116,32 +114,13 @@ TEST(LegacyReplyBuilder, MemAccess) { ASSERT_EQUALS(parsed.getCommandReply(), commandReply); } -DEATH_TEST(LegacyReplyBuilder, FailureAddingDoc, "Invariant failure _allowAddingOutputDocs") { - BSONObj metadata = buildMetadata(); - BSONObj commandReply = buildCommand(); - rpc::LegacyReplyBuilder replyBuilder; - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - replyBuilder.addOutputDoc(BSONObj()); -} - -DEATH_TEST(LegacyReplyBuilder, FailureAddingDocs, "Invariant failure _allowAddingOutputDocs") { - BSONObj metadata = buildMetadata(); - BSONObj commandReply = buildCommand(); - rpc::LegacyReplyBuilder replyBuilder; - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - rpc::DocumentRange range; - replyBuilder.addOutputDocs(range); -} - template <typename T> void testRoundTrip(rpc::ReplyBuilderInterface& replyBuilder) { auto metadata = buildMetadata(); auto commandReply = buildEmptyCommand(); - replyBuilder.setMetadata(metadata); replyBuilder.setCommandReply(commandReply); + replyBuilder.setMetadata(metadata); BSONObjBuilder outputDoc1Bob{}; outputDoc1Bob.append("z", "t"); @@ -160,207 +139,18 @@ void testRoundTrip(rpc::ReplyBuilderInterface& replyBuilder) { outputDoc2.appendSelfToBufBuilder(outputDocs); outputDoc3.appendSelfToBufBuilder(outputDocs); rpc::DocumentRange outputDocRange{outputDocs.buf(), outputDocs.buf() + outputDocs.len()}; - replyBuilder.addOutputDocs(outputDocRange); + if (replyBuilder.getProtocol() != rpc::Protocol::kOpQuery) { + replyBuilder.addOutputDocs(outputDocRange); + } auto msg = replyBuilder.done(); T parsed(&msg); ASSERT_EQUALS(parsed.getMetadata(), metadata); - ASSERT_TRUE(parsed.getOutputDocs() == outputDocRange); -} - -TEST(CommandReplyBuilder, MaxCommandReply) { - rpc::CommandReplyBuilder replyBuilder; - testMaxCommandReply(replyBuilder); -} - -TEST(LegacyReplyBuilder, MaxCommandReply) { - rpc::LegacyReplyBuilder replyBuilder; - testMaxCommandReply(replyBuilder); -} - -TEST(LegacyReplyBuilderSpaceTest, DocSize) { - rpc::LegacyReplyBuilder replyBuilder; - auto metadata = buildMetadata(); - auto commandReply = buildEmptyCommand(); - - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - - auto sizeBefore = replyBuilder.availableBytes(); - - for (int i = 0; i < 100000; ++i) { - BSONObjBuilder docBuilder; - docBuilder.append("foo" + std::to_string(i), "bar" + std::to_string(i)); - auto statusAfter = replyBuilder.addOutputDoc(docBuilder.done()); - ASSERT_TRUE(statusAfter.isOK()); - } - - auto sizeAfter = replyBuilder.availableBytes(); - auto msg = replyBuilder.done(); - - // construct an empty message to compare the estimated size difference with - // the actual difference - rpc::LegacyReplyBuilder replyBuilder0; - replyBuilder0.setMetadata(metadata); - replyBuilder0.setCommandReply(commandReply); - auto msg0 = replyBuilder0.done(); - - QueryResult::View qr0 = msg0.singleData().view2ptr(); - auto dataLen0 = static_cast<std::size_t>(qr0.msgdata().dataLen()); - - QueryResult::View qr = msg.singleData().view2ptr(); - auto dataLen = static_cast<std::size_t>(qr.msgdata().dataLen()); - - // below tests the array space estimates - // due to the inaccuracy in size estimation algo the actual size is off by up to 6 bytes - // on the large # of documents - ASSERT_EQUALS(sizeBefore - sizeAfter, dataLen - dataLen0 + 5); -} - -class CommandReplyBuilderSpaceTest : public mongo::unittest::Test { -protected: - // compute an empty doc size to use in follow up tests for payload size computation - virtual void setUp() override { - BSONObjBuilder docBuilder1{}; - docBuilder1.append("x", ""); - auto emptyDoc = docBuilder1.done(); - emptyDocSize = emptyDoc.objsize(); - } - - virtual void tearDown() override {} - - std::size_t emptyDocSize = 0u; -}; - -TEST_F(CommandReplyBuilderSpaceTest, DocSizeEq) { - rpc::CommandReplyBuilder replyBuilder; - auto metadata = buildMetadata(); - auto commandReply = buildEmptyCommand(); - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - - std::size_t spaceBefore = replyBuilder.availableBytes(); - - BSONObjBuilder docBuilder{}; - docBuilder.append("foo", "bar"); - auto doc = docBuilder.done(); - std::size_t docSize = doc.objsize(); - - replyBuilder.addOutputDoc(doc); - std::size_t spaceAfter = replyBuilder.availableBytes(); - ASSERT_EQUALS(spaceBefore - docSize, spaceAfter); -} - -// multiple calls to addOutputDoc, no metadata -TEST_F(CommandReplyBuilderSpaceTest, MaxDocSize1) { - rpc::CommandReplyBuilder replyBuilder; - - auto metadata = buildMetadata(); - auto commandReply = buildEmptyCommand(); - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - - std::size_t availSpace = replyBuilder.availableBytes(); - - while (availSpace > 0u) { - std::size_t payloadSz = - std::min(availSpace, static_cast<std::size_t>(mongo::BSONObjMaxUserSize)) - - emptyDocSize; - BSONObjBuilder docBuilder{}; - std::string payload = std::string(payloadSz, 'y'); - docBuilder.append("x", payload); - auto doc = docBuilder.done(); - replyBuilder.addOutputDoc(doc); - availSpace = replyBuilder.availableBytes(); - } - auto msg = replyBuilder.done(); - auto sizeUInt = static_cast<std::size_t>(msg.size()); - - ASSERT_EQUALS(sizeUInt, mongo::MaxMessageSizeBytes); -} - -// multiple calls to addOutputDoc, some metadata -TEST_F(CommandReplyBuilderSpaceTest, MaxDocSize2) { - rpc::CommandReplyBuilder replyBuilder; - - auto metadata = buildMetadata(); - auto commandReply = buildEmptyCommand(); - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - - std::size_t availSpace = replyBuilder.availableBytes(); - - while (availSpace > 0u) { - std::size_t payloadSz = - std::min(availSpace, static_cast<std::size_t>(mongo::BSONObjMaxUserSize)) - - emptyDocSize; - BSONObjBuilder docBuilder{}; - std::string payload = std::string(payloadSz, 'y'); - docBuilder.append("x", payload); - auto doc = docBuilder.done(); - replyBuilder.addOutputDoc(doc); - availSpace = replyBuilder.availableBytes(); + if (replyBuilder.getProtocol() != rpc::Protocol::kOpQuery) { + ASSERT_TRUE(parsed.getOutputDocs() == outputDocRange); } - auto msg = replyBuilder.done(); - auto sizeUInt = static_cast<std::size_t>(msg.size()); - - ASSERT_EQUALS(sizeUInt, mongo::MaxMessageSizeBytes); -} - - -// single call to addOutputDocs -TEST_F(CommandReplyBuilderSpaceTest, MaxDocSize3) { - rpc::CommandReplyBuilder replyBuilder; - - auto metadata = buildMetadata(); - auto commandReply = buildEmptyCommand(); - replyBuilder.setMetadata(metadata); - replyBuilder.setCommandReply(commandReply); - - std::size_t availSpace = replyBuilder.availableBytes(); - - BufBuilder docs; - while (availSpace > 0u) { - std::size_t payloadSz = - std::min(availSpace, static_cast<std::size_t>(mongo::BSONObjMaxUserSize)) - - emptyDocSize; - BSONObjBuilder docBuilder{}; - std::string payload = std::string(payloadSz, 'y'); - docBuilder.append("x", payload); - auto doc = docBuilder.done(); - availSpace -= doc.objsize(); - doc.appendSelfToBufBuilder(docs); - } - rpc::DocumentRange docRange{docs.buf(), docs.buf() + docs.len()}; - replyBuilder.addOutputDocs(docRange); - - auto msg = replyBuilder.done(); - - auto sizeUInt = static_cast<std::size_t>(msg.size()); - - ASSERT_EQUALS(sizeUInt, mongo::MaxMessageSizeBytes); -} - -// call to addCommandReply -void testMaxCommandReply(rpc::ReplyBuilderInterface& replyBuilder) { - BSONObjBuilder docBuilder1{}; - docBuilder1.append("x", ""); - auto emptyDoc = docBuilder1.done(); - std::size_t emptyDocSize = emptyDoc.objsize(); - - auto metadata = buildMetadata(); - replyBuilder.setMetadata(metadata); - - auto payloadSz = static_cast<std::size_t>(mongo::BSONObjMaxUserSize) - emptyDocSize; - - BSONObjBuilder commandReplyBuilder{}; - std::string payload = std::string(payloadSz, 'y'); - commandReplyBuilder.append("x", payload); - auto commandReply = commandReplyBuilder.obj(); - ASSERT_EQUALS(commandReply.objsize(), mongo::BSONObjMaxUserSize); - replyBuilder.setCommandReply(commandReply); } } // namespace diff --git a/src/mongo/s/client/dbclient_multi_command.cpp b/src/mongo/s/client/dbclient_multi_command.cpp index 41097339346..2f3e6ae67f8 100644 --- a/src/mongo/s/client/dbclient_multi_command.cpp +++ b/src/mongo/s/client/dbclient_multi_command.cpp @@ -93,8 +93,9 @@ static void sayAsCmd(DBClientBase* conn, StringData dbName, const BSONObj& cmdOb requestBuilder->setDatabase(dbName); requestBuilder->setCommandName(upconvertedCmd.firstElementFieldName()); - requestBuilder->setMetadata(metadataBob.done()); requestBuilder->setCommandArgs(upconvertedCmd); + requestBuilder->setMetadata(metadataBob.done()); + // Send our command auto requestMsg = requestBuilder->done(); conn->say(requestMsg); diff --git a/src/mongo/s/s_only.cpp b/src/mongo/s/s_only.cpp index 277d653cbde..a718c31c22d 100644 --- a/src/mongo/s/s_only.cpp +++ b/src/mongo/s/s_only.cpp @@ -88,7 +88,7 @@ void Command::execCommand(OperationContext* txn, cmdObj, result); - replyBuilder->setMetadata(rpc::makeEmptyMetadata()).setCommandReply(result.done()); + replyBuilder->setCommandReply(result.done()).setMetadata(rpc::makeEmptyMetadata()); } void Command::execCommandClientBasic(OperationContext* txn, diff --git a/src/mongo/tools/bridge.cpp b/src/mongo/tools/bridge.cpp index 53def3153bd..375473dee44 100644 --- a/src/mongo/tools/bridge.cpp +++ b/src/mongo/tools/bridge.cpp @@ -165,7 +165,7 @@ public: commandReply = StatusWith<BSONObj>(*status); } auto cmdResponse = - replyBuilder->setMetadata(metadata).setCommandReply(commandReply).done(); + replyBuilder->setCommandReply(commandReply).setMetadata(metadata).done(); _mp->say(cmdResponse, requestId); continue; } diff --git a/src/mongo/util/net/message.h b/src/mongo/util/net/message.h index eb1afc098aa..d0f66b7fec8 100644 --- a/src/mongo/util/net/message.h +++ b/src/mongo/util/net/message.h @@ -66,8 +66,10 @@ enum NetworkOp { dbGetMore = 2005, dbDelete = 2006, dbKillCursors = 2007, - dbCommand = 2008, - dbCommandReply = 2009, + // dbCommand_DEPRECATED = 2008, // + // dbCommandReply_DEPRECATED = 2009, // + dbCommand = 2010, + dbCommandReply = 2011, }; enum class LogicalOp { |