diff options
58 files changed, 247 insertions, 350 deletions
diff --git a/jstests/replsets/last_op_visible.js b/jstests/replsets/last_op_visible.js index 743b1b1c4c1..471669b9853 100644 --- a/jstests/replsets/last_op_visible.js +++ b/jstests/replsets/last_op_visible.js @@ -26,13 +26,13 @@ load("jstests/replsets/rslib.js"); var res = primary.getDB(name).runCommandWithMetadata({insert: name, documents: [{x: 1}]}, {"$replData": 1}); assert.commandWorked(res.commandReply); - var last_op_visible = res.metadata["$replData"].lastOpVisible; + var last_op_visible = res.commandReply["$replData"].lastOpVisible; // A find should return the same lastVisibleOp. res = primary.getDB(name).runCommandWithMetadata({find: name, readConcern: {level: "local"}}, {"$replData": 1}); assert.commandWorked(res.commandReply); - assert.eq(last_op_visible, res.metadata["$replData"].lastOpVisible); + assert.eq(last_op_visible, res.commandReply["$replData"].lastOpVisible); // A majority readConcern with afterOpTime: lastOpVisible should also return the same // lastVisibleOp. @@ -40,18 +40,18 @@ load("jstests/replsets/rslib.js"); {find: name, readConcern: {level: "majority", afterOpTime: last_op_visible}}, {"$replData": 1}); assert.commandWorked(res.commandReply); - assert.eq(last_op_visible, res.metadata["$replData"].lastOpVisible); + assert.eq(last_op_visible, res.commandReply["$replData"].lastOpVisible); // Do an insert without writeConcern. res = primary.getDB(name).runCommandWithMetadata( {insert: name, documents: [{x: 1}], writeConcern: {w: "majority"}}, {"$replData": 1}); assert.commandWorked(res.commandReply); - last_op_visible = res.metadata["$replData"].lastOpVisible; + last_op_visible = res.commandReply["$replData"].lastOpVisible; // A majority readConcern should return the same lastVisibleOp. res = primary.getDB(name).runCommandWithMetadata({find: name, readConcern: {level: "majority"}}, {"$replData": 1}); assert.commandWorked(res.commandReply); - assert.eq(last_op_visible, res.metadata["$replData"].lastOpVisible); + assert.eq(last_op_visible, res.commandReply["$replData"].lastOpVisible); replTest.stopSet(); }()); diff --git a/jstests/replsets/read_committed_no_snapshots.js b/jstests/replsets/read_committed_no_snapshots.js index 58d6095b60c..59524c24bd2 100644 --- a/jstests/replsets/read_committed_no_snapshots.js +++ b/jstests/replsets/read_committed_no_snapshots.js @@ -55,7 +55,7 @@ load("jstests/replsets/rslib.js"); // For reconfig and startSetIfSupportsReadMa // We need to propagate the lastOpVisible from the primary as afterOpTime in the secondaries to // ensure we wait for the write to be in the majority committed view. - var lastOp = res.metadata["$replData"].lastOpVisible; + var lastOp = res.commandReply["$replData"].lastOpVisible; // Timeout is based on heartbeat timeout. assert.commandWorked(healthySecondary.getDB(name).foo.runCommand( diff --git a/jstests/replsets/read_committed_on_secondary.js b/jstests/replsets/read_committed_on_secondary.js index df4394f112f..824a0f2e0bd 100644 --- a/jstests/replsets/read_committed_on_secondary.js +++ b/jstests/replsets/read_committed_on_secondary.js @@ -73,8 +73,8 @@ load("jstests/replsets/rslib.js"); // For startSetIfSupportsReadMajority. {"$replData": 1}); assert.commandWorked(res.commandReply); assert.eq(res.commandReply.writeErrors, undefined); - log("done saving doc: optime " + tojson(res.metadata.$replData.lastOpVisible)); - return res.metadata.$replData.lastOpVisible; + log("done saving doc: optime " + tojson(res.commandReply.$replData.lastOpVisible)); + return res.commandReply.$replData.lastOpVisible; } function doDirtyRead(lastOp) { diff --git a/src/mongo/client/authenticate.cpp b/src/mongo/client/authenticate.cpp index fe13099bd2a..f86791dd3af 100644 --- a/src/mongo/client/authenticate.cpp +++ b/src/mongo/client/authenticate.cpp @@ -169,8 +169,7 @@ void auth(RunCommandHook runCommand, "authentication."; // We need to mock a successful AuthResponse. - return handler( - AuthResponse(RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(0)))); + return handler(AuthResponse(RemoteCommandResponse(BSON("ok" << 1), Milliseconds(0)))); } // otherwise, call handler diff --git a/src/mongo/client/authenticate_test.cpp b/src/mongo/client/authenticate_test.cpp index a486b37f131..54cad29b9be 100644 --- a/src/mongo/client/authenticate_test.cpp +++ b/src/mongo/client/authenticate_test.cpp @@ -103,7 +103,7 @@ public: } void pushResponse(const BSONObj& cmd) { - _responses.emplace(cmd, BSONObj(), _millis); + _responses.emplace(cmd, _millis); } void pushRequest(StringData dbname, const BSONObj& cmd) { diff --git a/src/mongo/client/connection_pool.cpp b/src/mongo/client/connection_pool.cpp index 3050f281c73..c02ab68ed5d 100644 --- a/src/mongo/client/connection_pool.cpp +++ b/src/mongo/client/connection_pool.cpp @@ -207,7 +207,6 @@ ConnectionPool::ConnectionList::iterator ConnectionPool::acquireConnection( postConnectRequest->metadata)); auto rcr = executor::RemoteCommandResponse(reply->getCommandReply().getOwned(), - reply->getMetadata().getOwned(), Date_t::now() - start); uassertStatusOK(_hook->handleReply(target, std::move(rcr))); diff --git a/src/mongo/client/dbclient_base.cpp b/src/mongo/client/dbclient_base.cpp index d0ef3e0ea12..5332e631759 100644 --- a/src/mongo/client/dbclient_base.cpp +++ b/src/mongo/client/dbclient_base.cpp @@ -168,7 +168,7 @@ rpc::UniqueReply DBClientBase::parseCommandReplyMessage(const std::string& host, if (_metadataReader) { auto opCtx = haveClient() ? cc().getOperationContext() : nullptr; - uassertStatusOK(_metadataReader(opCtx, commandReply->getMetadata(), host)); + uassertStatusOK(_metadataReader(opCtx, commandReply->getCommandReply(), host)); } auto status = getStatusFromCommandResult(commandReply->getCommandReply()); @@ -470,11 +470,10 @@ void DBClientBase::_auth(const BSONObj& params) { OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj, request.metadata)); BSONObj data = reply->getCommandReply().getOwned(); - BSONObj metadata = reply->getMetadata().getOwned(); Milliseconds millis(Date_t::now() - start); // Hand control back to authenticateClient() - handler({data, metadata, millis}); + handler({data, millis}); } catch (...) { handler(exceptionToStatus()); diff --git a/src/mongo/client/dbclient_connection.cpp b/src/mongo/client/dbclient_connection.cpp index d0940a873e4..1903f5eaa3a 100644 --- a/src/mongo/client/dbclient_connection.cpp +++ b/src/mongo/client/dbclient_connection.cpp @@ -153,8 +153,7 @@ executor::RemoteCommandResponse initWireVersion(DBClientConnection* conn, conn->getCompressorManager().clientFinish(isMasterObj); - return executor::RemoteCommandResponse{ - std::move(isMasterObj), result->getMetadata().getOwned(), finish - start}; + return executor::RemoteCommandResponse{std::move(isMasterObj), finish - start}; } catch (...) { return exceptionToStatus(); diff --git a/src/mongo/client/dbclient_cursor.cpp b/src/mongo/client/dbclient_cursor.cpp index 321d19d0eef..2d6ea826c42 100644 --- a/src/mongo/client/dbclient_cursor.cpp +++ b/src/mongo/client/dbclient_cursor.cpp @@ -262,7 +262,7 @@ BSONObj DBClientCursor::commandDataReceived(const Message& reply) { auto opCtx = haveClient() ? cc().getOperationContext() : nullptr; if (_client->getReplyMetadataReader()) { uassertStatusOK(_client->getReplyMetadataReader()( - opCtx, commandReply->getMetadata(), _client->getServerAddress())); + opCtx, commandReply->getCommandReply(), _client->getServerAddress())); } return commandReply->getCommandReply().getOwned(); diff --git a/src/mongo/client/fetcher.cpp b/src/mongo/client/fetcher.cpp index 24eecd6d8b7..facd7aef852 100644 --- a/src/mongo/client/fetcher.cpp +++ b/src/mongo/client/fetcher.cpp @@ -361,7 +361,7 @@ void Fetcher::_callback(const RemoteCommandCallbackArgs& rcbd, const char* batch return; } - batchData.otherFields.metadata = std::move(rcbd.response.metadata); + batchData.otherFields.metadata = std::move(rcbd.response.data); batchData.elapsedMillis = rcbd.response.elapsedMillis.value_or(Milliseconds{0}); { stdx::lock_guard<stdx::mutex> lk(_mutex); diff --git a/src/mongo/client/fetcher_test.cpp b/src/mongo/client/fetcher_test.cpp index 1da6d23f20d..a50b03ab0aa 100644 --- a/src/mongo/client/fetcher_test.cpp +++ b/src/mongo/client/fetcher_test.cpp @@ -143,7 +143,7 @@ void FetcherTest::processNetworkResponse(const BSONObj& obj, ReadyQueueState readyQueueStateAfterProcessing, FetcherState fetcherStateAfterProcessing) { executor::NetworkInterfaceMock::InNetworkGuard guard(getNet()); - getNet()->scheduleSuccessfulResponse({obj, {}, elapsed}); + getNet()->scheduleSuccessfulResponse({obj, elapsed}); finishProcessingNetworkResponse(readyQueueStateAfterProcessing, fetcherStateAfterProcessing); } @@ -947,7 +947,7 @@ TEST_F(FetcherTest, UpdateNextActionAfterSecondBatch) { ASSERT_EQUALS(cursorId, cursors.front().numberLong()); // Failed killCursors command response should be logged. - getNet()->scheduleSuccessfulResponse(noi, {BSON("ok" << false), {}, Milliseconds(0)}); + getNet()->scheduleSuccessfulResponse(noi, {BSON("ok" << false), Milliseconds(0)}); getNet()->runReadyNetworkOperations(); } diff --git a/src/mongo/client/remote_command_retry_scheduler_test.cpp b/src/mongo/client/remote_command_retry_scheduler_test.cpp index a3c23c6b05d..783ab23d4c0 100644 --- a/src/mongo/client/remote_command_retry_scheduler_test.cpp +++ b/src/mongo/client/remote_command_retry_scheduler_test.cpp @@ -378,7 +378,7 @@ TEST_F(RemoteCommandRetrySchedulerTest, SchedulerInvokesCallbackOnFirstSuccessfu start(&scheduler); // Elapsed time in response is ignored on successful responses. - ResponseStatus response(BSON("ok" << 1 << "x" << 123), BSON("z" << 456), Milliseconds(100)); + ResponseStatus response(BSON("ok" << 1 << "x" << 123 << "z" << 456), Milliseconds(100)); processNetworkResponse(response); checkCompletionStatus(&scheduler, callback, response); @@ -400,8 +400,9 @@ TEST_F(RemoteCommandRetrySchedulerTest, SchedulerIgnoresEmbeddedErrorInSuccessfu // This is the case with some commands (e.g. find) which do not always return errors using the // wire protocol. ResponseStatus response(BSON("ok" << 0 << "code" << int(ErrorCodes::FailedToParse) << "errmsg" - << "injected error"), - BSON("z" << 456), + << "injected error" + << "z" + << 456), Milliseconds(100)); processNetworkResponse(response); @@ -458,7 +459,7 @@ TEST_F(RemoteCommandRetrySchedulerTest, SchedulerShouldRetryUntilSuccessfulRespo processNetworkResponse({ErrorCodes::HostNotFound, "first", Milliseconds(0)}); - ResponseStatus response(BSON("ok" << 1 << "x" << 123), BSON("z" << 456), Milliseconds(100)); + ResponseStatus response(BSON("ok" << 1 << "x" << 123 << "z" << 456), Milliseconds(100)); processNetworkResponse(response); checkCompletionStatus(&scheduler, callback, response); } diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index 57e82e49067..63500fe2af4 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -519,7 +519,6 @@ void Command::generateHelpResponse(OperationContext* opCtx, helpBuilder.append("help", str::stream() << "help for: " << command.getName() << " " << command.help()); replyBuilder->setCommandReply(helpBuilder.obj()); - replyBuilder->setMetadata(rpc::makeEmptyMetadata()); } bool ErrmsgCommandDeprecated::run(OperationContext* opCtx, diff --git a/src/mongo/db/repl/abstract_oplog_fetcher_test.cpp b/src/mongo/db/repl/abstract_oplog_fetcher_test.cpp index 388e1492461..13446ed9ff5 100644 --- a/src/mongo/db/repl/abstract_oplog_fetcher_test.cpp +++ b/src/mongo/db/repl/abstract_oplog_fetcher_test.cpp @@ -451,8 +451,7 @@ TEST_F(AbstractOplogFetcherTest, OplogFetcherTimesOutCorrectlyOnInitialFindReque net->enterNetwork(); auto when = net->now() + initialFindMaxTime + kNetworkTimeoutBufferMS + Milliseconds(10); auto noi = getNet()->getNextReadyRequest(); - RemoteCommandResponse response = { - {makeCursorResponse(1, {ops[0], ops[1]})}, rpc::makeEmptyMetadata(), Milliseconds(0)}; + RemoteCommandResponse response = {{makeCursorResponse(1, {ops[0], ops[1]})}, Milliseconds(0)}; auto request = net->scheduleSuccessfulResponse(noi, when, response); net->runUntil(when); net->runReadyNetworkOperations(); @@ -492,8 +491,7 @@ TEST_F(AbstractOplogFetcherTest, OplogFetcherTimesOutCorrectlyOnRetriedFindReque net->enterNetwork(); auto when = net->now() + initialFindMaxTime + kNetworkTimeoutBufferMS + Milliseconds(10); auto noi = getNet()->getNextReadyRequest(); - RemoteCommandResponse response = { - {makeCursorResponse(1, {ops[0], ops[1]})}, rpc::makeEmptyMetadata(), Milliseconds(0)}; + RemoteCommandResponse response = {{makeCursorResponse(1, {ops[0], ops[1]})}, Milliseconds(0)}; auto request = net->scheduleSuccessfulResponse(noi, when, response); net->runUntil(when); net->runReadyNetworkOperations(); @@ -503,8 +501,7 @@ TEST_F(AbstractOplogFetcherTest, OplogFetcherTimesOutCorrectlyOnRetriedFindReque net->enterNetwork(); when = net->now() + retriedFindMaxTime + kNetworkTimeoutBufferMS + Milliseconds(10); noi = getNet()->getNextReadyRequest(); - response = { - {makeCursorResponse(1, {ops[0], ops[1]})}, rpc::makeEmptyMetadata(), Milliseconds(0)}; + response = {{makeCursorResponse(1, {ops[0], ops[1]})}, Milliseconds(0)}; request = net->scheduleSuccessfulResponse(noi, when, response); net->runUntil(when); net->runReadyNetworkOperations(); diff --git a/src/mongo/db/repl/abstract_oplog_fetcher_test_fixture.cpp b/src/mongo/db/repl/abstract_oplog_fetcher_test_fixture.cpp index 2b0696c8531..ed0e0f9105a 100644 --- a/src/mongo/db/repl/abstract_oplog_fetcher_test_fixture.cpp +++ b/src/mongo/db/repl/abstract_oplog_fetcher_test_fixture.cpp @@ -139,8 +139,7 @@ executor::RemoteCommandRequest AbstractOplogFetcherTest::processNetworkResponse( executor::RemoteCommandRequest AbstractOplogFetcherTest::processNetworkResponse( BSONObj obj, bool expectReadyRequestsAfterProcessing) { - return processNetworkResponse({obj, rpc::makeEmptyMetadata(), Milliseconds(0)}, - expectReadyRequestsAfterProcessing); + return processNetworkResponse({obj, Milliseconds(0)}, expectReadyRequestsAfterProcessing); } } // namespace repl diff --git a/src/mongo/db/repl/base_cloner_test_fixture.cpp b/src/mongo/db/repl/base_cloner_test_fixture.cpp index 8fb69472329..88d0a84d996 100644 --- a/src/mongo/db/repl/base_cloner_test_fixture.cpp +++ b/src/mongo/db/repl/base_cloner_test_fixture.cpp @@ -162,7 +162,7 @@ const Status& BaseClonerTest::getStatus() const { void BaseClonerTest::scheduleNetworkResponse(NetworkOperationIterator noi, const BSONObj& obj) { auto net = getNet(); Milliseconds millis(0); - RemoteCommandResponse response(obj, BSONObj(), millis); + RemoteCommandResponse response(obj, millis); log() << "Scheduling response to request:" << noi->getDiagnosticString() << " -- resp:" << obj; net->scheduleResponse(noi, net->now(), response); } diff --git a/src/mongo/db/repl/check_quorum_for_config_change.cpp b/src/mongo/db/repl/check_quorum_for_config_change.cpp index 67204eed65c..f5c967163d0 100644 --- a/src/mongo/db/repl/check_quorum_for_config_change.cpp +++ b/src/mongo/db/repl/check_quorum_for_config_change.cpp @@ -226,7 +226,7 @@ void QuorumChecker::_tabulateHeartbeatResponse(const RemoteCommandRequest& reque if (_rsConfig->hasReplicaSetId()) { StatusWith<rpc::ReplSetMetadata> replMetadata = - rpc::ReplSetMetadata::readFromMetadata(response.metadata); + rpc::ReplSetMetadata::readFromMetadata(response.data); if (replMetadata.isOK() && replMetadata.getValue().getReplicaSetId().isSet() && _rsConfig->getReplicaSetId() != replMetadata.getValue().getReplicaSetId()) { std::string message = str::stream() diff --git a/src/mongo/db/repl/check_quorum_for_config_change_test.cpp b/src/mongo/db/repl/check_quorum_for_config_change_test.cpp index 2d4c0416b3e..7829fd72ec5 100644 --- a/src/mongo/db/repl/check_quorum_for_config_change_test.cpp +++ b/src/mongo/db/repl/check_quorum_for_config_change_test.cpp @@ -233,7 +233,7 @@ const BSONObj makeHeartbeatRequest(const ReplSetConfig& rsConfig, int myConfigIn executor::RemoteCommandResponse makeHeartbeatResponse(const ReplSetConfig& rsConfig, const stdx::chrono::milliseconds millis, const long long configVersion = 0, - const BSONObj& metadata = {}) { + const BSONObj& extraFields = {}) { ReplSetHeartbeatResponse hbResp; hbResp.setSetName(rsConfig.getReplSetName()); hbResp.setConfigVersion(configVersion); @@ -241,7 +241,9 @@ executor::RemoteCommandResponse makeHeartbeatResponse(const ReplSetConfig& rsCon OpTime opTime(Timestamp(), 0); hbResp.setAppliedOpTime(opTime); hbResp.setDurableOpTime(opTime); - return RemoteCommandResponse(hbResp.toBSON(), metadata, duration_cast<Milliseconds>(millis)); + auto bob = BSONObjBuilder(hbResp.toBSON()); + bob.appendElements(extraFields); + return RemoteCommandResponse(bob.obj(), duration_cast<Milliseconds>(millis)); } TEST_F(CheckQuorumForInitiate, QuorumCheckSuccessForFiveNodes) { @@ -399,13 +401,10 @@ TEST_F(CheckQuorumForInitiate, QuorumCheckFailedDueToSetNameMismatch) { (RemoteCommandResponse( BSON("ok" << 0 << "code" << ErrorCodes::InconsistentReplicaSetNames << "errmsg" << "replica set name doesn't match."), - BSONObj(), Milliseconds(8)))); } else { getNet()->scheduleResponse( - noi, - startDate + 10ms, - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(8)))); + noi, startDate + 10ms, RemoteCommandResponse(BSON("ok" << 1), Milliseconds(8))); } } getNet()->runUntil(startDate + 10ms); @@ -473,14 +472,14 @@ TEST_F(CheckQuorumForInitiate, QuorumCheckFailedDueToSetIdMismatch) { unexpectedId, rpc::ReplSetMetadata::kNoPrimary, -1); - BSONObjBuilder metadataBuilder; - metadata.writeToMetadata(&metadataBuilder).transitional_ignore(); + BSONObjBuilder bob; + uassertStatusOK(metadata.writeToMetadata(&bob)); long long configVersion = 0; getNet()->scheduleResponse( noi, startDate + 10ms, - makeHeartbeatResponse(rsConfig, 8ms, configVersion, metadataBuilder.obj())); + makeHeartbeatResponse(rsConfig, 8ms, configVersion, bob.obj())); } else { getNet()->scheduleResponse(noi, startDate + 10ms, makeHeartbeatResponse(rsConfig, 8ms)); } @@ -713,7 +712,6 @@ TEST_F(CheckQuorumForReconfig, QuorumCheckVetoedDueToIncompatibleSetName) { (RemoteCommandResponse( BSON("ok" << 0 << "code" << ErrorCodes::InconsistentReplicaSetNames << "errmsg" << "replica set name doesn't match."), - BSONObj(), Milliseconds(8)))); } else { getNet()->scheduleResponse( @@ -949,10 +947,9 @@ TEST_F(CheckQuorumForReconfig, QuorumCheckProcessesCallbackCanceledResponse) { startDate + 10ms, (RemoteCommandResponse(ErrorCodes::CallbackCanceled, "Testing canceled callback"))); } else { - getNet()->scheduleResponse( - noi, - startDate + 10ms, - (RemoteCommandResponse(BSON("ok" << 0), BSONObj(), Milliseconds(8)))); + getNet()->scheduleResponse(noi, + startDate + Milliseconds(10), + RemoteCommandResponse(BSON("ok" << 0), Milliseconds(8))); } } getNet()->runUntil(startDate + 10ms); diff --git a/src/mongo/db/repl/databases_cloner_test.cpp b/src/mongo/db/repl/databases_cloner_test.cpp index f32c83c756a..66f2138ceaa 100644 --- a/src/mongo/db/repl/databases_cloner_test.cpp +++ b/src/mongo/db/repl/databases_cloner_test.cpp @@ -104,7 +104,7 @@ public: const BSONObj& obj) { NetworkInterfaceMock* net = getNet(); Milliseconds millis(0); - RemoteCommandResponse response(obj, BSONObj(), millis); + RemoteCommandResponse response(obj, millis); net->scheduleResponse(noi, net->now(), response); } @@ -242,11 +242,10 @@ protected: log() << "Sending response for network request:"; log() << " req: " << noi->getRequest().dbname << "." << noi->getRequest().cmdObj; log() << " resp:" << responses[processedRequests].second; - net->scheduleResponse(noi, - net->now(), - RemoteCommandResponse(responses[processedRequests].second, - BSONObj(), - Milliseconds(10))); + net->scheduleResponse( + noi, + net->now(), + RemoteCommandResponse(responses[processedRequests].second, Milliseconds(10))); if ((Date_t::now() - lastLog) > Seconds(1)) { lastLog = Date_t(); diff --git a/src/mongo/db/repl/initial_syncer_test.cpp b/src/mongo/db/repl/initial_syncer_test.cpp index 2948a08ff01..fe3483cb792 100644 --- a/src/mongo/db/repl/initial_syncer_test.cpp +++ b/src/mongo/db/repl/initial_syncer_test.cpp @@ -162,7 +162,7 @@ public: const BSONObj& obj) { NetworkInterfaceMock* net = getNet(); Milliseconds millis(0); - RemoteCommandResponse response(obj, BSONObj(), millis); + RemoteCommandResponse response(obj, millis); log() << "Sending response for network request:"; log() << " req: " << noi->getRequest().dbname << "." << noi->getRequest().cmdObj; log() << " resp:" << response; @@ -501,9 +501,6 @@ RemoteCommandResponse makeCursorResponse(CursorId cursorId, int rbid = 1) { OpTime futureOpTime(Timestamp(1000, 1000), 1000); rpc::OplogQueryMetadata oqMetadata(futureOpTime, futureOpTime, rbid, 0, 0); - BSONObjBuilder metadataBob; - ASSERT_OK(oqMetadata.writeToMetadata(&metadataBob)); - auto metadataObj = metadataBob.obj(); BSONObjBuilder bob; { @@ -518,8 +515,9 @@ RemoteCommandResponse makeCursorResponse(CursorId cursorId, } } } + ASSERT_OK(oqMetadata.writeToMetadata(&bob)); bob.append("ok", 1); - return {bob.obj(), metadataObj, Milliseconds(0)}; + return {bob.obj(), Milliseconds()}; } /** diff --git a/src/mongo/db/repl/oplog_fetcher_test.cpp b/src/mongo/db/repl/oplog_fetcher_test.cpp index 5209ea0c4e3..8f03c6c50cb 100644 --- a/src/mongo/db/repl/oplog_fetcher_test.cpp +++ b/src/mongo/db/repl/oplog_fetcher_test.cpp @@ -192,8 +192,7 @@ std::unique_ptr<ShutdownState> OplogFetcherTest::processSingleBatch(RemoteComman std::unique_ptr<ShutdownState> OplogFetcherTest::processSingleBatch(BSONObj obj, bool requireFresherSyncSource) { - return processSingleBatch({obj, rpc::makeEmptyMetadata(), Milliseconds(0)}, - requireFresherSyncSource); + return processSingleBatch({obj, Milliseconds(0)}, requireFresherSyncSource); } void _checkDefaultCommandObjectFields(BSONObj cmdObj) { @@ -219,6 +218,12 @@ std::unique_ptr<OplogFetcher> OplogFetcherTest::makeOplogFetcher(ReplSetConfig c defaultBatchSize); } +BSONObj concatenate(BSONObj a, const BSONObj& b) { + auto bob = BSONObjBuilder(std::move(a)); + bob.appendElements(b); + return bob.obj(); +} + TEST_F( OplogFetcherTest, FindQueryContainsTermAndStartTimestampIfGetCurrentTermAndLastCommittedOpTimeReturnsValidTerm) { @@ -255,19 +260,21 @@ TEST_F(OplogFetcherTest, AwaitDataTimeoutShouldEqualHalfElectionTimeoutUnderProt } TEST_F(OplogFetcherTest, InvalidReplSetMetadataInResponseStopsTheOplogFetcher) { - auto shutdownState = processSingleBatch( - {makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - BSON(rpc::kReplSetMetadataFieldName << BSON("invalid_repl_metadata_field" << 1)), - Milliseconds(0)}); + auto shutdownState = + processSingleBatch({concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), + BSON(rpc::kReplSetMetadataFieldName + << BSON("invalid_repl_metadata_field" << 1))), + Milliseconds(0)}); ASSERT_EQUALS(ErrorCodes::NoSuchKey, shutdownState->getStatus()); } TEST_F(OplogFetcherTest, InvalidOplogQueryMetadataInResponseStopsTheOplogFetcher) { - auto shutdownState = processSingleBatch( - {makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - BSON(rpc::kOplogQueryMetadataFieldName << BSON("invalid_oq_metadata_field" << 1)), - Milliseconds(0)}); + auto shutdownState = + processSingleBatch({concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), + BSON(rpc::kOplogQueryMetadataFieldName + << BSON("invalid_oq_metadata_field" << 1))), + Milliseconds(0)}); ASSERT_EQUALS(ErrorCodes::NoSuchKey, shutdownState->getStatus()); } @@ -279,10 +286,11 @@ TEST_F(OplogFetcherTest, ASSERT_OK(metadata.writeToMetadata(&bob)); auto metadataObj = bob.obj(); - ASSERT_OK(processSingleBatch({makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_OK( + processSingleBatch( + {concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), metadataObj), + Milliseconds(0)}) + ->getStatus()); ASSERT_TRUE(dataReplicatorExternalState->metadataWasProcessed); ASSERT_EQUALS(metadata.getPrimaryIndex(), dataReplicatorExternalState->replMetadataProcessed.getPrimaryIndex()); @@ -296,10 +304,11 @@ TEST_F(OplogFetcherTest, ValidMetadataWithInResponseShouldBeForwardedToProcessMe ASSERT_OK(replMetadata.writeToMetadata(&bob)); ASSERT_OK(oqMetadata.writeToMetadata(&bob)); auto metadataObj = bob.obj(); - ASSERT_OK(processSingleBatch({makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_OK( + processSingleBatch( + {concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), metadataObj), + Milliseconds(0)}) + ->getStatus()); ASSERT_TRUE(dataReplicatorExternalState->metadataWasProcessed); ASSERT_EQUALS(replMetadata.getPrimaryIndex(), dataReplicatorExternalState->replMetadataProcessed.getPrimaryIndex()); @@ -315,11 +324,12 @@ TEST_F(OplogFetcherTest, MetadataAndBatchAreNotProcessedWhenSyncSourceRollsBack) ASSERT_OK(oqMetadata.writeToMetadata(&bob)); auto metadataObj = bob.obj(); - ASSERT_EQUALS(ErrorCodes::InvalidSyncSource, - processSingleBatch({makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_EQUALS( + ErrorCodes::InvalidSyncSource, + processSingleBatch( + {concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), metadataObj), + Milliseconds(0)}) + ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); ASSERT(lastEnqueuedDocuments.empty()); } @@ -332,11 +342,12 @@ TEST_F(OplogFetcherTest, MetadataAndBatchAreNotProcessedWhenSyncSourceIsBehind) ASSERT_OK(oqMetadata.writeToMetadata(&bob)); auto metadataObj = bob.obj(); - ASSERT_EQUALS(ErrorCodes::InvalidSyncSource, - processSingleBatch({makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_EQUALS( + ErrorCodes::InvalidSyncSource, + processSingleBatch( + {concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), metadataObj), + Milliseconds(0)}) + ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); ASSERT(lastEnqueuedDocuments.empty()); } @@ -349,11 +360,12 @@ TEST_F(OplogFetcherTest, MetadataAndBatchAreNotProcessedWhenSyncSourceIsNotAhead ASSERT_OK(oqMetadata.writeToMetadata(&bob)); auto metadataObj = bob.obj(); - ASSERT_EQUALS(ErrorCodes::InvalidSyncSource, - processSingleBatch({makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_EQUALS( + ErrorCodes::InvalidSyncSource, + processSingleBatch( + {concatenate(makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), metadataObj), + Milliseconds(0)}) + ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); ASSERT(lastEnqueuedDocuments.empty()); } @@ -370,7 +382,8 @@ TEST_F(OplogFetcherTest, auto entry = makeNoopOplogEntry({123LL, staleOpTime}); ASSERT_EQUALS( ErrorCodes::InvalidSyncSource, - processSingleBatch({makeCursorResponse(0, {entry}), metadataObj, Milliseconds(0)}, false) + processSingleBatch( + {concatenate(makeCursorResponse(0, {entry}), metadataObj), Milliseconds(0)}, false) ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); ASSERT(lastEnqueuedDocuments.empty()); @@ -388,8 +401,8 @@ TEST_F(OplogFetcherTest, MetadataAndBatchAreProcessedWhenSyncSourceIsCurrentButM auto metadataObj = bob.obj(); auto entry = makeNoopOplogEntry(lastFetched); - auto shutdownState = - processSingleBatch({makeCursorResponse(0, {entry}), metadataObj, Milliseconds(0)}, false); + auto shutdownState = processSingleBatch( + {concatenate(makeCursorResponse(0, {entry}), metadataObj), Milliseconds(0)}, false); ASSERT_OK(shutdownState->getStatus()); ASSERT(dataReplicatorExternalState->metadataWasProcessed); } @@ -404,8 +417,8 @@ TEST_F(OplogFetcherTest, auto metadataObj = bob.obj(); auto entry = makeNoopOplogEntry(lastFetched); - auto shutdownState = - processSingleBatch({makeCursorResponse(0, {entry}), metadataObj, Milliseconds(0)}, false); + auto shutdownState = processSingleBatch( + {concatenate(makeCursorResponse(0, {entry}), metadataObj), Milliseconds(0)}, false); ASSERT_OK(shutdownState->getStatus()); ASSERT(dataReplicatorExternalState->metadataWasProcessed); } @@ -418,8 +431,9 @@ TEST_F(OplogFetcherTest, auto metadataObj = bob.obj(); ASSERT_EQUALS(ErrorCodes::OplogStartMissing, processSingleBatch( - {makeCursorResponse(0, {makeNoopOplogEntry(Seconds(456), lastFetched.value)}), - metadataObj, + {concatenate(makeCursorResponse( + 0, {makeNoopOplogEntry(Seconds(456), lastFetched.value)}), + metadataObj), Milliseconds(0)}) ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); @@ -434,17 +448,17 @@ TEST_F(OplogFetcherTest, MetadataIsNotProcessedOnBatchThatTriggersRollback) { auto metadataObj = bob.obj(); ASSERT_EQUALS(ErrorCodes::OplogStartMissing, processSingleBatch( - {makeCursorResponse(0, {makeNoopOplogEntry(Seconds(456), lastFetched.value)}), - metadataObj, + {concatenate(makeCursorResponse( + 0, {makeNoopOplogEntry(Seconds(456), lastFetched.value)}), + metadataObj), Milliseconds(0)}) ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); } TEST_F(OplogFetcherTest, EmptyMetadataIsNotProcessed) { - ASSERT_OK(processSingleBatch({makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), - rpc::makeEmptyMetadata(), - Milliseconds(0)}) + ASSERT_OK(processSingleBatch( + {makeCursorResponse(0, {makeNoopOplogEntry(lastFetched)}), Milliseconds(0)}) ->getStatus()); ASSERT_FALSE(dataReplicatorExternalState->metadataWasProcessed); } @@ -456,10 +470,10 @@ TEST_F(OplogFetcherTest, EmptyFirstBatchStopsOplogFetcherWithOplogStartMissingEr TEST_F(OplogFetcherTest, MissingOpTimeInFirstDocumentCausesOplogFetcherToStopWithInvalidBSONError) { auto metadataObj = makeOplogQueryMetadataObject(remoteNewerOpTime, rbid, 2, 2); - ASSERT_EQUALS( - ErrorCodes::InvalidBSON, - processSingleBatch({makeCursorResponse(0, {BSONObj()}), metadataObj, Milliseconds(0)}) - ->getStatus()); + ASSERT_EQUALS(ErrorCodes::InvalidBSON, + processSingleBatch({concatenate(makeCursorResponse(0, {BSONObj()}), metadataObj), + Milliseconds(0)}) + ->getStatus()); } TEST_F( @@ -468,8 +482,9 @@ TEST_F( auto metadataObj = makeOplogQueryMetadataObject(remoteNewerOpTime, rbid, 2, 2); ASSERT_EQUALS(ErrorCodes::OplogStartMissing, processSingleBatch( - {makeCursorResponse(0, {makeNoopOplogEntry(Seconds(456), lastFetched.value)}), - metadataObj, + {concatenate(makeCursorResponse( + 0, {makeNoopOplogEntry(Seconds(456), lastFetched.value)}), + metadataObj), Milliseconds(0)}) ->getStatus()); } @@ -480,8 +495,9 @@ TEST_F(OplogFetcherTest, ASSERT_EQUALS( ErrorCodes::OplogStartMissing, processSingleBatch( - {makeCursorResponse(0, {makeNoopOplogEntry(remoteNewerOpTime, lastFetched.value + 1)}), - metadataObj, + {concatenate(makeCursorResponse( + 0, {makeNoopOplogEntry(remoteNewerOpTime, lastFetched.value + 1)}), + metadataObj), Milliseconds(0)}) ->getStatus()); } @@ -489,28 +505,30 @@ TEST_F(OplogFetcherTest, TEST_F(OplogFetcherTest, MissingOpTimeInSecondDocumentOfFirstBatchCausesOplogFetcherToStopWithNoSuchKey) { auto metadataObj = makeOplogQueryMetadataObject(remoteNewerOpTime, rbid, 2, 2); - ASSERT_EQUALS(ErrorCodes::NoSuchKey, - processSingleBatch( - {makeCursorResponse(0, - {makeNoopOplogEntry(lastFetched), - BSON("o" << BSON("msg" - << "oplog entry without optime"))}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_EQUALS( + ErrorCodes::NoSuchKey, + processSingleBatch( + {concatenate(makeCursorResponse(0, + {makeNoopOplogEntry(lastFetched), + BSON("o" << BSON("msg" + << "oplog entry without optime"))}), + metadataObj), + Milliseconds(0)}) + ->getStatus()); } TEST_F(OplogFetcherTest, TimestampsNotAdvancingInBatchCausesOplogFetcherStopWithOplogOutOfOrder) { auto metadataObj = makeOplogQueryMetadataObject(remoteNewerOpTime, rbid, 2, 2); - ASSERT_EQUALS(ErrorCodes::OplogOutOfOrder, - processSingleBatch({makeCursorResponse(0, - {makeNoopOplogEntry(lastFetched), - makeNoopOplogEntry(Seconds(1000), 1), - makeNoopOplogEntry(Seconds(2000), 1), - makeNoopOplogEntry(Seconds(1500), 1)}), - metadataObj, - Milliseconds(0)}) - ->getStatus()); + ASSERT_EQUALS( + ErrorCodes::OplogOutOfOrder, + processSingleBatch({concatenate(makeCursorResponse(0, + {makeNoopOplogEntry(lastFetched), + makeNoopOplogEntry(Seconds(1000), 1), + makeNoopOplogEntry(Seconds(2000), 1), + makeNoopOplogEntry(Seconds(1500), 1)}), + metadataObj), + Milliseconds(0)}) + ->getStatus()); } TEST_F(OplogFetcherTest, OplogFetcherShouldExcludeFirstDocumentInFirstBatchWhenEnqueuingDocuments) { @@ -521,8 +539,8 @@ TEST_F(OplogFetcherTest, OplogFetcherShouldExcludeFirstDocumentInFirstBatchWhenE auto thirdEntry = makeNoopOplogEntry({{Seconds(789), 0}, lastFetched.opTime.getTerm()}, 300); Fetcher::Documents documents{firstEntry, secondEntry, thirdEntry}; - auto shutdownState = - processSingleBatch({makeCursorResponse(0, documents), metadataObj, Milliseconds(0)}); + auto shutdownState = processSingleBatch( + {concatenate(makeCursorResponse(0, documents), metadataObj), Milliseconds(0)}); ASSERT_EQUALS(2U, lastEnqueuedDocuments.size()); ASSERT_BSONOBJ_EQ(secondEntry, lastEnqueuedDocuments[0]); @@ -559,8 +577,8 @@ TEST_F(OplogFetcherTest, OplogFetcherShouldReportErrorsThrownFromCallback) { return Status(ErrorCodes::InternalError, "my custom error"); }; - auto shutdownState = - processSingleBatch({makeCursorResponse(0, documents), metadataObj, Milliseconds(0)}); + auto shutdownState = processSingleBatch( + {concatenate(makeCursorResponse(0, documents), metadataObj), Milliseconds(0)}); ASSERT_EQ(shutdownState->getStatus(), Status(ErrorCodes::InternalError, "my custom error")); } @@ -582,8 +600,8 @@ void OplogFetcherTest::testSyncSourceChecking(rpc::ReplSetMetadata* replMetadata dataReplicatorExternalState->shouldStopFetchingResult = true; - auto shutdownState = - processSingleBatch({makeCursorResponse(0, documents), metadataObj, Milliseconds(0)}); + auto shutdownState = processSingleBatch( + {concatenate(makeCursorResponse(0, documents), metadataObj), Milliseconds(0)}); // Sync source checking happens after we have successfully pushed the operations into // the buffer for the next replication phase (eg. applier). @@ -697,7 +715,8 @@ RemoteCommandRequest OplogFetcherTest::testTwoBatchHandling() { auto metadataObj = makeOplogQueryMetadataObject(remoteNewerOpTime, rbid, 2, 2); processNetworkResponse( - {makeCursorResponse(cursorId, {firstEntry, secondEntry}), metadataObj, Milliseconds(0)}, + {concatenate(makeCursorResponse(cursorId, {firstEntry, secondEntry}), metadataObj), + Milliseconds(0)}, true); ASSERT_EQUALS(1U, lastEnqueuedDocuments.size()); diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp index 479881621eb..2b06d004d0a 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp @@ -149,7 +149,7 @@ void ReplicationCoordinatorImpl::_handleHeartbeatResponse( resp = cbData.response.data; responseStatus = hbResponse.initialize(resp, _topCoord->getTerm()); StatusWith<rpc::ReplSetMetadata> replMetadata = - rpc::ReplSetMetadata::readFromMetadata(cbData.response.metadata); + rpc::ReplSetMetadata::readFromMetadata(cbData.response.data); LOG_FOR_HEARTBEATS(2) << "Received response to heartbeat (requestId: " << cbData.request.id << ") from " << target << ", " << resp; diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp index 61a63ca9225..5bb5c2a2ee2 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp @@ -394,10 +394,9 @@ TEST_F(ReplCoordHBV1Test, IgnoreTheContentsOfMetadataWhenItsReplicaSetIdDoesNotM rpc::ReplSetMetadata metadata( opTime.getTerm(), opTime, opTime, rsConfig.getConfigVersion(), unexpectedId, 1, -1); - BSONObjBuilder metadataBuilder; - metadata.writeToMetadata(&metadataBuilder).transitional_ignore(); + uassertStatusOK(metadata.writeToMetadata(&responseBuilder)); - heartbeatResponse = makeResponseStatus(responseBuilder.obj(), metadataBuilder.obj()); + heartbeatResponse = makeResponseStatus(responseBuilder.obj()); } // process heartbeat diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp index 9d64861a4ab..f703624e158 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp @@ -384,9 +384,8 @@ TEST_F(ReplCoordTest, InitiateSucceedsWhenQuorumCheckPasses) { hbResp.setConfigVersion(0); hbResp.setAppliedOpTime(OpTime(Timestamp(100, 1), 0)); hbResp.setDurableOpTime(OpTime(Timestamp(100, 1), 0)); - getNet()->scheduleResponse(noi, - startDate + Milliseconds(10), - RemoteCommandResponse(hbResp.toBSON(), BSONObj(), Milliseconds(8))); + getNet()->scheduleResponse( + noi, startDate + Milliseconds(10), RemoteCommandResponse(hbResp.toBSON(), Milliseconds(8))); getNet()->runUntil(startDate + Milliseconds(10)); getNet()->exitNetwork(); ASSERT_EQUALS(startDate + Milliseconds(10), getNet()->now()); @@ -4521,9 +4520,8 @@ TEST_F(ReplCoordTest, << 3 << "syncSourceIndex" << 1))); - BSONObjBuilder metadataBuilder; - ASSERT_OK(metadata.getValue().writeToMetadata(&metadataBuilder)); - auto metadataObj = metadataBuilder.obj(); + BSONObjBuilder responseBuilder; + ASSERT_OK(metadata.getValue().writeToMetadata(&responseBuilder)); auto net = getNet(); net->enterNetwork(); @@ -4538,7 +4536,8 @@ TEST_F(ReplCoordTest, hbResp.setConfigVersion(config.getConfigVersion()); hbResp.setSetName(config.getReplSetName()); hbResp.setState(MemberState::RS_SECONDARY); - net->scheduleResponse(noi, net->now(), makeResponseStatus(hbResp.toBSON(), metadataObj)); + responseBuilder.appendElements(hbResp.toBSON()); + net->scheduleResponse(noi, net->now(), makeResponseStatus(responseBuilder.obj())); net->runReadyNetworkOperations(); net->exitNetwork(); @@ -4647,9 +4646,8 @@ TEST_F(ReplCoordTest, TermAndLastCommittedOpTimeUpdatedFromHeartbeatWhenArbiter) << 3 << "syncSourceIndex" << 1))); - BSONObjBuilder metadataBuilder; - ASSERT_OK(metadata.getValue().writeToMetadata(&metadataBuilder)); - auto metadataObj = metadataBuilder.obj(); + BSONObjBuilder responseBuilder; + ASSERT_OK(metadata.getValue().writeToMetadata(&responseBuilder)); auto net = getNet(); net->enterNetwork(); @@ -4664,7 +4662,8 @@ TEST_F(ReplCoordTest, TermAndLastCommittedOpTimeUpdatedFromHeartbeatWhenArbiter) hbResp.setConfigVersion(config.getConfigVersion()); hbResp.setSetName(config.getReplSetName()); hbResp.setState(MemberState::RS_SECONDARY); - net->scheduleResponse(noi, net->now(), makeResponseStatus(hbResp.toBSON(), metadataObj)); + responseBuilder.appendElements(hbResp.toBSON()); + net->scheduleResponse(noi, net->now(), makeResponseStatus(responseBuilder.obj())); net->runReadyNetworkOperations(); net->exitNetwork(); diff --git a/src/mongo/db/repl/replication_coordinator_test_fixture.cpp b/src/mongo/db/repl/replication_coordinator_test_fixture.cpp index 456c9443276..10f1babe0f2 100644 --- a/src/mongo/db/repl/replication_coordinator_test_fixture.cpp +++ b/src/mongo/db/repl/replication_coordinator_test_fixture.cpp @@ -212,15 +212,8 @@ void ReplCoordTest::assertStartSuccess(const BSONObj& configDoc, const HostAndPo executor::RemoteCommandResponse ReplCoordTest::makeResponseStatus(const BSONObj& doc, Milliseconds millis) { - return makeResponseStatus(doc, BSONObj(), millis); -} - -executor::RemoteCommandResponse ReplCoordTest::makeResponseStatus(const BSONObj& doc, - const BSONObj& metadata, - Milliseconds millis) { - log() << "Responding with " << doc << " (metadata: " << metadata << "; elapsed: " << millis - << ")"; - return RemoteCommandResponse(doc, metadata, millis); + log() << "Responding with " << doc << " (elapsed: " << millis << ")"; + return RemoteCommandResponse(doc, millis); } void ReplCoordTest::simulateEnoughHeartbeatsForAllNodesUp() { diff --git a/src/mongo/db/repl/replication_coordinator_test_fixture.h b/src/mongo/db/repl/replication_coordinator_test_fixture.h index b5718a65a70..2e3732a4caa 100644 --- a/src/mongo/db/repl/replication_coordinator_test_fixture.h +++ b/src/mongo/db/repl/replication_coordinator_test_fixture.h @@ -65,13 +65,6 @@ public: const BSONObj& doc, Milliseconds millis = Milliseconds(0)); /** - * Makes a command response with the given "doc" response, metadata and optional elapsed time - * "millis". - */ - static executor::RemoteCommandResponse makeResponseStatus( - const BSONObj& doc, const BSONObj& metadata, Milliseconds millis = Milliseconds(0)); - - /** * Constructs a ReplSetConfig from the given BSON, or raises a test failure exception. */ static ReplSetConfig assertMakeRSConfig(const BSONObj& configBSON); diff --git a/src/mongo/db/repl/reporter_test.cpp b/src/mongo/db/repl/reporter_test.cpp index 5fb620d5252..8c6ac958c16 100644 --- a/src/mongo/db/repl/reporter_test.cpp +++ b/src/mongo/db/repl/reporter_test.cpp @@ -331,7 +331,7 @@ TEST_F(ReporterTestNoTriggerAtSetUp, IsNotActiveAfterUpdatePositionTimeoutExpire // Schedule a response to the updatePosition command at a time that exceeds the timeout. Then // make sure the reporter shut down due to a remote command timeout. auto updatePosRequest = net->getNextReadyRequest(); - RemoteCommandResponse response(BSON("ok" << 1), BSONObj(), Milliseconds(0)); + RemoteCommandResponse response(BSON("ok" << 1), Milliseconds(0)); executor::TaskExecutor::ResponseStatus responseStatus(response); net->scheduleResponse( updatePosRequest, net->now() + updatePositionTimeout + Milliseconds(1), responseStatus); diff --git a/src/mongo/db/repl/scatter_gather_test.cpp b/src/mongo/db/repl/scatter_gather_test.cpp index 1d5a32b4895..8d1c8f480fb 100644 --- a/src/mongo/db/repl/scatter_gather_test.cpp +++ b/src/mongo/db/repl/scatter_gather_test.cpp @@ -165,21 +165,18 @@ TEST_F(ScatterGatherTest, DeleteAlgorithmAfterItHasCompleted) { NetworkInterfaceMock* net = getNet(); net->enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(5), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(5), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); net->runUntil(net->now() + Seconds(2)); @@ -208,21 +205,19 @@ TEST_F(ScatterGatherTest, DeleteAlgorithmBeforeItCompletes) { net->enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); net->scheduleResponse( - noi, net->now(), (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + noi, net->now(), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); // Get and process the response from the first node immediately. net->runReadyNetworkOperations(); noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(5), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(5), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); sga.reset(); @@ -248,9 +243,8 @@ TEST_F(ScatterGatherTest, DeleteAlgorithmAfterCancel) { NetworkInterfaceMock* net = getNet(); net->enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); // Cancel the runner so following responses won't change the result. All pending requests @@ -347,21 +341,18 @@ TEST_F(ScatterGatherTest, DoNotProcessMoreThanSufficientResponses) { NetworkInterfaceMock* net = getNet(); net->enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(5), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(5), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); net->runUntil(net->now() + Seconds(2)); @@ -390,9 +381,8 @@ TEST_F(ScatterGatherTest, AlgorithmProcessesCallbackCanceledResponse) { NetworkInterfaceMock* net = getNet(); net->enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); - net->scheduleResponse(noi, - net->now() + Seconds(2), - (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + net->scheduleResponse( + noi, net->now() + Seconds(2), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); ASSERT_FALSE(ranCompletion); noi = net->getNextReadyRequest(); @@ -493,7 +483,7 @@ TEST_F(ScatterGatherTest, SuccessfulScatterGatherViaRun) { net->enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); net->scheduleResponse( - noi, net->now(), (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + noi, net->now(), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); net->runReadyNetworkOperations(); noi = net->getNextReadyRequest(); @@ -502,7 +492,7 @@ TEST_F(ScatterGatherTest, SuccessfulScatterGatherViaRun) { noi = net->getNextReadyRequest(); net->scheduleResponse( - noi, net->now(), (RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(10)))); + noi, net->now(), (RemoteCommandResponse(BSON("ok" << 1), Milliseconds(10)))); net->runReadyNetworkOperations(); net->exitNetwork(); diff --git a/src/mongo/db/repl/vote_requester_test.cpp b/src/mongo/db/repl/vote_requester_test.cpp index 74374d3ad36..9a11270e577 100644 --- a/src/mongo/db/repl/vote_requester_test.cpp +++ b/src/mongo/db/repl/vote_requester_test.cpp @@ -153,7 +153,7 @@ protected: ReplSetRequestVotesResponse response; response.setVoteGranted(true); response.setTerm(1); - return RemoteCommandResponse(response.toBSON(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(response.toBSON(), Milliseconds(10)); } RemoteCommandResponse votedYesStatusNotOkBecauseFailedToStoreLastVote() { @@ -164,7 +164,7 @@ protected: response.addToBSON(&result); auto status = Status(ErrorCodes::InterruptedDueToStepDown, "operation was interrupted"); CommandHelpers::appendCommandStatusNoThrow(result, status); - return RemoteCommandResponse(result.obj(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(result.obj(), Milliseconds(10)); } RemoteCommandResponse votedNoBecauseConfigVersionDoesNotMatch() { @@ -172,7 +172,7 @@ protected: response.setVoteGranted(false); response.setTerm(1); response.setReason("candidate's config version differs from mine"); - return RemoteCommandResponse(response.toBSON(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(response.toBSON(), Milliseconds(10)); } RemoteCommandResponse votedNoBecauseSetNameDiffers() { @@ -180,7 +180,7 @@ protected: response.setVoteGranted(false); response.setTerm(1); response.setReason("candidate's set name differs from mine"); - return RemoteCommandResponse(response.toBSON(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(response.toBSON(), Milliseconds(10)); } RemoteCommandResponse votedNoBecauseLastOpTimeIsGreater() { @@ -188,7 +188,7 @@ protected: response.setVoteGranted(false); response.setTerm(1); response.setReason("candidate's data is staler than mine"); - return RemoteCommandResponse(response.toBSON(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(response.toBSON(), Milliseconds(10)); } RemoteCommandResponse votedNoBecauseTermIsGreater() { @@ -196,7 +196,7 @@ protected: response.setVoteGranted(false); response.setTerm(3); response.setReason("candidate's term is lower than mine"); - return RemoteCommandResponse(response.toBSON(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(response.toBSON(), Milliseconds(10)); } RemoteCommandResponse votedNoBecauseAlreadyVoted() { @@ -204,7 +204,7 @@ protected: response.setVoteGranted(false); response.setTerm(2); response.setReason("already voted for another candidate this term"); - return RemoteCommandResponse(response.toBSON(), BSONObj(), Milliseconds(10)); + return RemoteCommandResponse(response.toBSON(), Milliseconds(10)); } std::unique_ptr<VoteRequester::Algorithm> _requester; diff --git a/src/mongo/db/s/config/sharding_catalog_manager_shard_operations.cpp b/src/mongo/db/s/config/sharding_catalog_manager_shard_operations.cpp index 9992f06acb5..7ef70b27738 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_shard_operations.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_shard_operations.cpp @@ -198,7 +198,6 @@ StatusWith<Shard::CommandResponse> ShardingCatalogManager::_runCommandForAddShar return Shard::CommandResponse(std::move(host), std::move(result), - response.metadata.getOwned(), std::move(commandStatus), std::move(writeConcernStatus)); } diff --git a/src/mongo/db/s/migration_source_manager.cpp b/src/mongo/db/s/migration_source_manager.cpp index d49a85ad625..d6bd884ba26 100644 --- a/src/mongo/db/s/migration_source_manager.cpp +++ b/src/mongo/db/s/migration_source_manager.cpp @@ -545,9 +545,7 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC severe() << "The current config server opTime is " << Grid::get(opCtx)->configOpTime(); severe() << "The commit response came from " << redact(commitChunkMigrationResponse.getValue().hostAndPort->toString()) - << " and contained:"; - severe() << " metadata: " - << redact(commitChunkMigrationResponse.getValue().metadata.toString()); + << " and contained"; severe() << " response: " << redact(commitChunkMigrationResponse.getValue().response.toString()); diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 16a4671961f..0f0581104e9 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -213,7 +213,7 @@ void generateErrorResponse(OperationContext* opCtx, // so we need to reset it to a clean state just to be sure. replyBuilder->reset(); replyBuilder->setCommandReply(exception.toStatus(), extraFields); - replyBuilder->setMetadata(replyMetadata); + replyBuilder->getBodyBuilder().appendElements(replyMetadata); } BSONObj getErrorLabels(const boost::optional<OperationSessionInfoFromClient>& sessionOptions, @@ -571,15 +571,10 @@ bool runCommandImpl(OperationContext* opCtx, } } - BSONObjBuilder metadataBob; - appendReplyMetadata(opCtx, request, &metadataBob); + auto commandBodyBob = replyBuilder->getBodyBuilder(); + appendReplyMetadata(opCtx, request, &commandBodyBob); + appendClusterAndOperationTime(opCtx, &commandBodyBob, &commandBodyBob, startOperationTime); - { - auto commandBodyBob = replyBuilder->getBodyBuilder(); - appendClusterAndOperationTime(opCtx, &commandBodyBob, &metadataBob, startOperationTime); - } - - replyBuilder->setMetadata(metadataBob.obj()); return ok; } diff --git a/src/mongo/dbtests/mock/mock_remote_db_server.cpp b/src/mongo/dbtests/mock/mock_remote_db_server.cpp index 0c18b18bfb1..baa35e0f8f6 100644 --- a/src/mongo/dbtests/mock/mock_remote_db_server.cpp +++ b/src/mongo/dbtests/mock/mock_remote_db_server.cpp @@ -163,10 +163,7 @@ rpc::UniqueReply MockRemoteDBServer::runCommand(InstanceID id, const OpMsgReques // We need to construct a reply message - it will always be read through a view so it // doesn't matter whether we use OpMsgReplyBuilder or LegacyReplyBuilder - auto message = rpc::OpMsgReplyBuilder{} - .setCommandReply(reply) - .setMetadata(rpc::makeEmptyMetadata()) - .done(); + auto message = rpc::OpMsgReplyBuilder{}.setCommandReply(reply).done(); auto replyView = stdx::make_unique<rpc::OpMsgReply>(&message); return rpc::UniqueReply(std::move(message), std::move(replyView)); } diff --git a/src/mongo/executor/network_interface_mock.cpp b/src/mongo/executor/network_interface_mock.cpp index 42cda1b4005..942d3795eea 100644 --- a/src/mongo/executor/network_interface_mock.cpp +++ b/src/mongo/executor/network_interface_mock.cpp @@ -326,7 +326,7 @@ void NetworkInterfaceMock::scheduleResponse(NetworkOperationIterator noi, if (_metadataHook && response.isOK()) { _metadataHook ->readReplyMetadata( - noi->getRequest().opCtx, noi->getRequest().target.toString(), response.metadata) + noi->getRequest().opCtx, noi->getRequest().target.toString(), response.data) .transitional_ignore(); } @@ -335,8 +335,7 @@ void NetworkInterfaceMock::scheduleResponse(NetworkOperationIterator noi, } RemoteCommandRequest NetworkInterfaceMock::scheduleSuccessfulResponse(const BSONObj& response) { - BSONObj metadata; - return scheduleSuccessfulResponse(RemoteCommandResponse(response, metadata, Milliseconds(0))); + return scheduleSuccessfulResponse(RemoteCommandResponse(response, Milliseconds(0))); } RemoteCommandRequest NetworkInterfaceMock::scheduleSuccessfulResponse( @@ -479,7 +478,7 @@ void NetworkInterfaceMock::_connectThenEnqueueOperation_inlock(const HostAndPort auto handshakeReply = (handshakeReplyIter != std::end(_handshakeReplies)) ? handshakeReplyIter->second - : RemoteCommandResponse(BSONObj(), BSONObj(), Milliseconds(0)); + : RemoteCommandResponse(BSONObj(), Milliseconds(0)); auto valid = _hook->validateHost(target, op.getRequest().cmdObj, handshakeReply); if (!valid.isOK()) { diff --git a/src/mongo/executor/network_interface_mock_test.cpp b/src/mongo/executor/network_interface_mock_test.cpp index a09ed4aeac0..ef7aebbdb9a 100644 --- a/src/mongo/executor/network_interface_mock_test.cpp +++ b/src/mongo/executor/network_interface_mock_test.cpp @@ -116,8 +116,8 @@ TEST_F(NetworkInterfaceMockTest, ConnectionHook) { RemoteCommandResponse expectedResponse{BSON("foo" << "bar" << "baz" - << "garply"), - BSON("bar" + << "garply" + << "bar" << "baz"), Milliseconds(30)}; @@ -125,8 +125,7 @@ TEST_F(NetworkInterfaceMockTest, ConnectionHook) { auto isMasterReplyData = BSON("iamyour" << "father"); - RemoteCommandResponse isMasterReply{ - isMasterReplyData.copy(), BSON("blah" << 2), Milliseconds(20)}; + RemoteCommandResponse isMasterReply{isMasterReplyData.copy(), Milliseconds(20)}; net().setHandshakeReplyForHost(testHost(), std::move(isMasterReply)); @@ -166,7 +165,6 @@ TEST_F(NetworkInterfaceMockTest, ConnectionHook) { testHost(), "testDB", BSON("test" << 1), rpc::makeEmptyMetadata(), nullptr}; RemoteCommandResponse actualResponseExpected{BSON("1212121212" << "12121212121212"), - BSONObj(), Milliseconds(0)}; ASSERT_OK(net().startCommand(cb, actualCommandExpected, [&](RemoteCommandResponse resp) { diff --git a/src/mongo/executor/network_interface_tl.cpp b/src/mongo/executor/network_interface_tl.cpp index 923d25f565e..3c885feab5f 100644 --- a/src/mongo/executor/network_interface_tl.cpp +++ b/src/mongo/executor/network_interface_tl.cpp @@ -360,7 +360,7 @@ Future<RemoteCommandResponse> NetworkInterfaceTL::_onAcquireConn( if (_metadataHook && response.status.isOK()) { auto target = state->conn->getHostAndPort().toString(); response.status = - _metadataHook->readReplyMetadata(nullptr, std::move(target), response.metadata); + _metadataHook->readReplyMetadata(nullptr, std::move(target), response.data); } return RemoteCommandResponse(std::move(response)); diff --git a/src/mongo/executor/network_test_env.cpp b/src/mongo/executor/network_test_env.cpp index bc782c8ce8c..d60bd0d4fa7 100644 --- a/src/mongo/executor/network_test_env.cpp +++ b/src/mongo/executor/network_test_env.cpp @@ -52,7 +52,7 @@ void NetworkTestEnv::onCommand(OnCommandFunction func) { if (resultStatus.isOK()) { BSONObjBuilder result(std::move(resultStatus.getValue())); CommandHelpers::appendCommandStatusNoThrow(result, resultStatus.getStatus()); - const RemoteCommandResponse response(result.obj(), BSONObj(), Milliseconds(1)); + const RemoteCommandResponse response(result.obj(), Milliseconds(1)); _mockNetwork->scheduleResponse(noi, _mockNetwork->now(), response); } else { @@ -75,8 +75,7 @@ void NetworkTestEnv::onCommandWithMetadata(OnCommandWithMetadataFunction func) { if (cmdResponseStatus.isOK()) { BSONObjBuilder result(std::move(cmdResponseStatus.data)); CommandHelpers::appendCommandStatusNoThrow(result, cmdResponseStatus.status); - const RemoteCommandResponse response( - result.obj(), cmdResponseStatus.metadata, Milliseconds(1)); + const RemoteCommandResponse response(result.obj(), Milliseconds(1)); _mockNetwork->scheduleResponse(noi, _mockNetwork->now(), response); } else { @@ -128,10 +127,10 @@ void NetworkTestEnv::onFindWithMetadataCommand(OnFindCommandWithMetadataFunction const NamespaceString nss = NamespaceString(request.dbname, request.cmdObj.firstElement().String()); - BSONObjBuilder resultBuilder; + BSONObjBuilder resultBuilder(std::move(metadata)); appendCursorResponseObject(0LL, nss.toString(), arr.arr(), &resultBuilder); - return RemoteCommandResponse(resultBuilder.obj(), metadata, Milliseconds(1)); + return RemoteCommandResponse(resultBuilder.obj(), Milliseconds(1)); }); } diff --git a/src/mongo/executor/remote_command_response.cpp b/src/mongo/executor/remote_command_response.cpp index 4fb8af3ccf2..e40917843cb 100644 --- a/src/mongo/executor/remote_command_response.cpp +++ b/src/mongo/executor/remote_command_response.cpp @@ -54,38 +54,29 @@ RemoteCommandResponse::RemoteCommandResponse(Status s, Milliseconds millis) invariant(!isOK()); }; -RemoteCommandResponse::RemoteCommandResponse(BSONObj dataObj, - BSONObj metadataObj, - Milliseconds millis) - : data(std::move(dataObj)), metadata(std::move(metadataObj)), elapsedMillis(millis) { +RemoteCommandResponse::RemoteCommandResponse(BSONObj dataObj, Milliseconds millis) + : data(std::move(dataObj)), elapsedMillis(millis) { // The buffer backing the default empty BSONObj has static duration so it is effectively // owned. invariant(data.isOwned() || data.objdata() == BSONObj().objdata()); - invariant(metadata.isOwned() || metadata.objdata() == BSONObj().objdata()); }; RemoteCommandResponse::RemoteCommandResponse(Message messageArg, BSONObj dataObj, - BSONObj metadataObj, Milliseconds millis) : message(std::make_shared<const Message>(std::move(messageArg))), data(std::move(dataObj)), - metadata(std::move(metadataObj)), elapsedMillis(millis) { if (!data.isOwned()) { data.shareOwnershipWith(message->sharedBuffer()); } - if (!metadata.isOwned()) { - metadata.shareOwnershipWith(message->sharedBuffer()); - } } // TODO(amidvidy): we currently discard output docs when we use this constructor. We should // have RCR hold those too, but we need more machinery before that is possible. RemoteCommandResponse::RemoteCommandResponse(const rpc::ReplyInterface& rpcReply, Milliseconds millis) - : RemoteCommandResponse(rpcReply.getCommandReply(), rpcReply.getMetadata(), std::move(millis)) { -} + : RemoteCommandResponse(rpcReply.getCommandReply(), std::move(millis)) {} bool RemoteCommandResponse::isOK() const { return status.isOK(); @@ -101,8 +92,7 @@ bool RemoteCommandResponse::operator==(const RemoteCommandResponse& rhs) const { return true; } SimpleBSONObjComparator bsonComparator; - return bsonComparator.evaluate(data == rhs.data) && - bsonComparator.evaluate(metadata == rhs.metadata) && elapsedMillis == rhs.elapsedMillis; + return bsonComparator.evaluate(data == rhs.data) && elapsedMillis == rhs.elapsedMillis; } bool RemoteCommandResponse::operator!=(const RemoteCommandResponse& rhs) const { diff --git a/src/mongo/executor/remote_command_response.h b/src/mongo/executor/remote_command_response.h index 0dd51f68ba8..894ca915e7d 100644 --- a/src/mongo/executor/remote_command_response.h +++ b/src/mongo/executor/remote_command_response.h @@ -61,12 +61,9 @@ struct RemoteCommandResponse { RemoteCommandResponse(Status s, Milliseconds millis); - RemoteCommandResponse(BSONObj dataObj, BSONObj metadataObj, Milliseconds millis); + RemoteCommandResponse(BSONObj dataObj, Milliseconds millis); - RemoteCommandResponse(Message messageArg, - BSONObj dataObj, - BSONObj metadataObj, - Milliseconds millis); + RemoteCommandResponse(Message messageArg, BSONObj dataObj, Milliseconds millis); RemoteCommandResponse(const rpc::ReplyInterface& rpcReply, Milliseconds millis); @@ -79,7 +76,6 @@ struct RemoteCommandResponse { std::shared_ptr<const Message> message; // May be null. BSONObj data; // Always owned. May point into message. - BSONObj metadata; // Always owned. May point into message. boost::optional<Milliseconds> elapsedMillis; Status status = Status::OK(); }; diff --git a/src/mongo/rpc/legacy_reply.cpp b/src/mongo/rpc/legacy_reply.cpp index d21b880e60c..c6ba4bfbb02 100644 --- a/src/mongo/rpc/legacy_reply.cpp +++ b/src/mongo/rpc/legacy_reply.cpp @@ -102,10 +102,6 @@ LegacyReply::LegacyReply(const Message* message) { return; } -const BSONObj& LegacyReply::getMetadata() const { - return _commandReply; -} - const BSONObj& LegacyReply::getCommandReply() const { return _commandReply; } diff --git a/src/mongo/rpc/legacy_reply.h b/src/mongo/rpc/legacy_reply.h index f283bb954ec..b428319d5a7 100644 --- a/src/mongo/rpc/legacy_reply.h +++ b/src/mongo/rpc/legacy_reply.h @@ -41,7 +41,7 @@ namespace rpc { /** * Immutable view of an OP_REPLY legacy-style command reply. */ -class LegacyReply : public ReplyInterface { +class LegacyReply final : public ReplyInterface { public: /** * Construct a Reply from a Message. @@ -50,12 +50,6 @@ public: explicit LegacyReply(const Message* message); /** - * Accessor for the metadata object. Metadata is generally used for information - * that is independent of any specific command, e.g. auditing information. - */ - const BSONObj& getMetadata() const final; - - /** * The result of executing the command. */ const BSONObj& getCommandReply() const final; diff --git a/src/mongo/rpc/legacy_reply_builder.cpp b/src/mongo/rpc/legacy_reply_builder.cpp index 78663e9f75f..1d88eb96aa5 100644 --- a/src/mongo/rpc/legacy_reply_builder.cpp +++ b/src/mongo/rpc/legacy_reply_builder.cpp @@ -54,7 +54,7 @@ LegacyReplyBuilder::~LegacyReplyBuilder() {} LegacyReplyBuilder& LegacyReplyBuilder::setCommandReply(Status nonOKStatus, BSONObj extraErrorInfo) { - invariant(_state == State::kCommandReply); + invariant(!_haveCommandReply); if (nonOKStatus == ErrorCodes::StaleConfig) { _staleConfigError = true; @@ -77,32 +77,25 @@ LegacyReplyBuilder& LegacyReplyBuilder::setCommandReply(Status nonOKStatus, } LegacyReplyBuilder& LegacyReplyBuilder::setRawCommandReply(const BSONObj& commandReply) { - invariant(_state == State::kCommandReply); + invariant(!_haveCommandReply); + _bodyOffset = _builder.len(); commandReply.appendSelfToBufBuilder(_builder); - _state = State::kMetadata; + _haveCommandReply = true; return *this; } BSONObjBuilder LegacyReplyBuilder::getBodyBuilder() { - if (_state == State::kCommandReply) { + if (!_haveCommandReply) { auto bob = BSONObjBuilder(_builder); _bodyOffset = bob.offset(); - _state = State::kMetadata; + _haveCommandReply = true; return bob; } - invariant(_state == State::kMetadata); invariant(_bodyOffset); return BSONObjBuilder(BSONObjBuilder::ResumeBuildingTag{}, _builder, _bodyOffset); } -LegacyReplyBuilder& LegacyReplyBuilder::setMetadata(const BSONObj& metadata) { - invariant(_state == State::kMetadata); - BSONObjBuilder(BSONObjBuilder::ResumeBuildingTag(), _builder, sizeof(QueryResult::Value)) - .appendElements(metadata); - _state = State::kOutputDocs; - return *this; -} Protocol LegacyReplyBuilder::getProtocol() const { return rpc::Protocol::kOpQuery; @@ -116,20 +109,20 @@ void LegacyReplyBuilder::reserveBytes(const std::size_t bytes) { 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::kCommandReply) { + if (!_haveCommandReply) { return; } _builder.reset(); _builder.skip(sizeof(QueryResult::Value)); _message.reset(); - _state = State::kCommandReply; + _haveCommandReply = false; _staleConfigError = false; _bodyOffset = 0; } Message LegacyReplyBuilder::done() { - invariant(_state == State::kOutputDocs); + invariant(_haveCommandReply); QueryResult::View qr = _builder.buf(); @@ -148,7 +141,6 @@ Message LegacyReplyBuilder::done() { _message.setData(_builder.release()); - _state = State::kDone; return std::move(_message); } diff --git a/src/mongo/rpc/legacy_reply_builder.h b/src/mongo/rpc/legacy_reply_builder.h index 4f3128bb2be..3c90dc7db4f 100644 --- a/src/mongo/rpc/legacy_reply_builder.h +++ b/src/mongo/rpc/legacy_reply_builder.h @@ -54,8 +54,6 @@ public: BSONObjBuilder getBodyBuilder() final; - LegacyReplyBuilder& setMetadata(const BSONObj& metadata) final; - void reset() final; Message done() final; @@ -65,14 +63,12 @@ public: void reserveBytes(const std::size_t bytes) final; private: - enum class State { kMetadata, kCommandReply, kOutputDocs, kDone }; - - BufBuilder _builder{}; + BufBuilder _builder; std::size_t _bodyOffset = 0; Message _message; - State _state{State::kCommandReply}; + bool _haveCommandReply = false; // For stale config errors we need to set the correct ResultFlag. - bool _staleConfigError{false}; + bool _staleConfigError = false; }; } // namespace rpc diff --git a/src/mongo/rpc/op_msg_rpc_impls.h b/src/mongo/rpc/op_msg_rpc_impls.h index b16a9a7c0cd..ab09ffc3a2d 100644 --- a/src/mongo/rpc/op_msg_rpc_impls.h +++ b/src/mongo/rpc/op_msg_rpc_impls.h @@ -39,9 +39,6 @@ class OpMsgReply final : public rpc::ReplyInterface { public: explicit OpMsgReply(const Message* message) : _msg(OpMsg::parseOwned(*message)) {} explicit OpMsgReply(OpMsg msg) : _msg(std::move(msg)) {} - const BSONObj& getMetadata() const override { - return _msg.body; - } const BSONObj& getCommandReply() const override { return _msg.body; } @@ -68,10 +65,6 @@ public: OpMsgBuilder::DocSequenceBuilder getDocSequenceBuilder(StringData name) override { return _builder.beginDocSequence(name); } - ReplyBuilderInterface& setMetadata(const BSONObj& metadata) override { - _builder.resumeBody().appendElements(metadata); - return *this; - } rpc::Protocol getProtocol() const override { return rpc::Protocol::kOpMsg; } diff --git a/src/mongo/rpc/reply_builder_interface.h b/src/mongo/rpc/reply_builder_interface.h index 64a8cab8891..e8ba5383090 100644 --- a/src/mongo/rpc/reply_builder_interface.h +++ b/src/mongo/rpc/reply_builder_interface.h @@ -75,8 +75,6 @@ public: uasserted(50875, "Only OpMsg may use document sequences"); } - 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 diff --git a/src/mongo/rpc/reply_builder_test.cpp b/src/mongo/rpc/reply_builder_test.cpp index dd0d36a5739..2a15f519bdf 100644 --- a/src/mongo/rpc/reply_builder_test.cpp +++ b/src/mongo/rpc/reply_builder_test.cpp @@ -125,7 +125,7 @@ TEST(LegacyReplyBuilder, CommandError) { const BSONObj extraObj = extra.obj(); rpc::LegacyReplyBuilder replyBuilder; replyBuilder.setCommandReply(status, extraObj); - replyBuilder.setMetadata(metadata); + replyBuilder.getBodyBuilder().appendElements(metadata); auto msg = replyBuilder.done(); rpc::LegacyReply parsed(&msg); @@ -136,7 +136,6 @@ TEST(LegacyReplyBuilder, CommandError) { return unifiedBuilder.obj(); }()); - ASSERT_BSONOBJ_EQ(parsed.getMetadata(), body); ASSERT_BSONOBJ_EQ(parsed.getCommandReply(), body); } @@ -149,7 +148,7 @@ TEST(OpMsgReplyBuilder, CommandError) { const BSONObj extraObj = extra.obj(); rpc::OpMsgReplyBuilder replyBuilder; replyBuilder.setCommandReply(status, extraObj); - replyBuilder.setMetadata(metadata); + replyBuilder.getBodyBuilder().appendElements(metadata); auto msg = replyBuilder.done(); rpc::OpMsgReply parsed(&msg); @@ -160,7 +159,6 @@ TEST(OpMsgReplyBuilder, CommandError) { return unifiedBuilder.obj(); }()); - ASSERT_BSONOBJ_EQ(parsed.getMetadata(), body); ASSERT_BSONOBJ_EQ(parsed.getCommandReply(), body); } @@ -170,7 +168,7 @@ void testRoundTrip(rpc::ReplyBuilderInterface& replyBuilder, bool unifiedBodyAnd auto commandReply = buildEmptyCommand(); replyBuilder.setCommandReply(commandReply); - replyBuilder.setMetadata(metadata); + replyBuilder.getBodyBuilder().appendElements(metadata); auto msg = replyBuilder.done(); @@ -184,10 +182,8 @@ void testRoundTrip(rpc::ReplyBuilderInterface& replyBuilder, bool unifiedBodyAnd }()); ASSERT_BSONOBJ_EQ(parsed.getCommandReply(), body); - ASSERT_BSONOBJ_EQ(parsed.getMetadata(), body); } else { ASSERT_BSONOBJ_EQ(parsed.getCommandReply(), commandReply); - ASSERT_BSONOBJ_EQ(parsed.getMetadata(), metadata); } } @@ -198,7 +194,7 @@ void testErrors(rpc::ReplyBuilderInterface& replyBuilder) { const auto status = Status(ErrorExtraInfoExample(123), "Why does this keep failing!"); replyBuilder.setCommandReply(status); - replyBuilder.setMetadata(buildMetadata()); + replyBuilder.getBodyBuilder().appendElements(buildMetadata()); const auto msg = replyBuilder.done(); diff --git a/src/mongo/rpc/reply_interface.h b/src/mongo/rpc/reply_interface.h index a5e72255d6e..c51fb686459 100644 --- a/src/mongo/rpc/reply_interface.h +++ b/src/mongo/rpc/reply_interface.h @@ -47,12 +47,6 @@ public: virtual ~ReplyInterface() = default; /** - * Accessor for the metadata object. Metadata is generally used for information - * that is independent of any specific command, e.g. auditing information. - */ - virtual const BSONObj& getMetadata() const = 0; - - /** * The result of executing the command. */ virtual const BSONObj& getCommandReply() const = 0; diff --git a/src/mongo/s/client/rs_local_client.cpp b/src/mongo/s/client/rs_local_client.cpp index a4cf1810b3d..21888c8fc1c 100644 --- a/src/mongo/s/client/rs_local_client.cpp +++ b/src/mongo/s/client/rs_local_client.cpp @@ -87,7 +87,6 @@ StatusWith<Shard::CommandResponse> RSLocalClient::runCommandOnce(OperationContex auto result = commandResponse->getCommandReply().getOwned(); return Shard::CommandResponse(boost::none, result, - commandResponse->getMetadata().getOwned(), getStatusFromCommandResult(result), getWriteConcernStatusFromCommandResult(result)); } catch (const DBException& ex) { diff --git a/src/mongo/s/client/shard.h b/src/mongo/s/client/shard.h index ead66c59fa5..a90d1160c0f 100644 --- a/src/mongo/s/client/shard.h +++ b/src/mongo/s/client/shard.h @@ -56,12 +56,10 @@ public: struct CommandResponse { CommandResponse(boost::optional<HostAndPort> hostAndPort, BSONObj response, - BSONObj metadata, Status commandStatus, Status writeConcernStatus) : hostAndPort(std::move(hostAndPort)), response(std::move(response)), - metadata(std::move(metadata)), commandStatus(std::move(commandStatus)), writeConcernStatus(std::move(writeConcernStatus)) {} @@ -79,7 +77,6 @@ public: boost::optional<HostAndPort> hostAndPort; BSONObj response; - BSONObj metadata; Status commandStatus; Status writeConcernStatus; }; diff --git a/src/mongo/s/client/shard_remote.cpp b/src/mongo/s/client/shard_remote.cpp index e6a67f8b83c..e8ea73f9225 100644 --- a/src/mongo/s/client/shard_remote.cpp +++ b/src/mongo/s/client/shard_remote.cpp @@ -244,7 +244,6 @@ StatusWith<Shard::CommandResponse> ShardRemote::_runCommand(OperationContext* op return Shard::CommandResponse(std::move(host), std::move(result), - response.metadata.getOwned(), std::move(commandStatus), std::move(writeConcernStatus)); } diff --git a/src/mongo/s/client/shard_remote_test.cpp b/src/mongo/s/client/shard_remote_test.cpp index fe53c03dcc4..9503ecdc5d9 100644 --- a/src/mongo/s/client/shard_remote_test.cpp +++ b/src/mongo/s/client/shard_remote_test.cpp @@ -184,10 +184,11 @@ TEST_F(ShardRemoteTest, ScatterGatherRepliesWithLastCommittedOpTime) { onCommandWithMetadata([&](const executor::RemoteCommandRequest& request) { std::vector<BSONObj> batch = {BSON("_id" << 1)}; CursorResponse cursorResponse(nss, CursorId(123), batch); - auto result = cursorResponse.toBSON(CursorResponse::ResponseType::InitialResponse); + auto result = BSONObjBuilder( + cursorResponse.toBSON(CursorResponse::ResponseType::InitialResponse)); + result.appendElements(makeLastCommittedOpTimeMetadata(expectedTime)); - return executor::RemoteCommandResponse( - result, makeLastCommittedOpTimeMetadata(expectedTime), Milliseconds(1)); + return executor::RemoteCommandResponse(result.obj(), Milliseconds(1)); }); } diff --git a/src/mongo/s/cluster_last_error_info_test.cpp b/src/mongo/s/cluster_last_error_info_test.cpp index b9903192581..1de77def4b9 100644 --- a/src/mongo/s/cluster_last_error_info_test.cpp +++ b/src/mongo/s/cluster_last_error_info_test.cpp @@ -84,11 +84,10 @@ TEST_F(ClusterGetLastErrorTest, // Make the reply contain ShardingMetadata. repl::OpTime opTime{Timestamp{10, 10}, 10}; onCommandWithMetadata([&](const RemoteCommandRequest& request) { - BSONObjBuilder metadataBob; - rpc::ShardingMetadata(opTime, OID() /* ignored OID field */) - .writeToMetadata(&metadataBob) - .transitional_ignore(); - return RemoteCommandResponse(BSON("ok" << 1), metadataBob.obj(), Milliseconds(1)); + auto bob = BSONObjBuilder(BSON("ok" << 1)); + uassertStatusOK( + rpc::ShardingMetadata(opTime, OID() /* ignored OID field */).writeToMetadata(&bob)); + return RemoteCommandResponse(bob.obj(), Milliseconds(1)); }); future.timed_get(kFutureTimeout); @@ -129,11 +128,10 @@ TEST_F(ClusterGetLastErrorTest, ClusterLastErrorInfoNotUpdatedIfNotInitialized) // Make the reply contain ShardingMetadata. repl::OpTime opTime{Timestamp{10, 10}, 10}; onCommandWithMetadata([&](const RemoteCommandRequest& request) { - BSONObjBuilder metadataBob; - rpc::ShardingMetadata(opTime, OID() /* ignored OID field */) - .writeToMetadata(&metadataBob) - .transitional_ignore(); - return RemoteCommandResponse(BSON("ok" << 1), metadataBob.obj(), Milliseconds(1)); + auto bob = BSONObjBuilder(BSON("ok" << 1)); + uassertStatusOK( + rpc::ShardingMetadata(opTime, OID() /* ignored OID field */).writeToMetadata(&bob)); + return RemoteCommandResponse(bob.obj(), Milliseconds(1)); }); future.timed_get(kFutureTimeout); @@ -174,7 +172,7 @@ TEST_F(ClusterGetLastErrorTest, ClusterLastErrorInfoNotUpdatedIfReplyDoesntHaveS // Do not return ShardingMetadata in the reply. repl::OpTime opTime{Timestamp{10, 10}, 10}; onCommandWithMetadata([&](const RemoteCommandRequest& request) { - return RemoteCommandResponse(BSON("ok" << 1), BSONObj(), Milliseconds(1)); + return RemoteCommandResponse(BSON("ok" << 1), Milliseconds(1)); }); future.timed_get(kFutureTimeout); diff --git a/src/mongo/s/commands/cluster_multicast.cpp b/src/mongo/s/commands/cluster_multicast.cpp index d1fcd3cc729..4585a3f8e01 100644 --- a/src/mongo/s/commands/cluster_multicast.cpp +++ b/src/mongo/s/commands/cluster_multicast.cpp @@ -134,7 +134,6 @@ public: if (CommandHelpers::appendCommandStatusNoThrow(subbob, response.status)) { subbob.append("data", response.data); - subbob.append("metadata", response.metadata); if (response.elapsedMillis) { subbob.append("elapsedMillis", response.elapsedMillis->count()); } diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 83e8ca526ae..850a9a61537 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -580,7 +580,6 @@ DbResponse Strategy::clientCommand(OperationContext* opCtx, const Message& m) { return {}; // Don't reply. } - reply->setMetadata(BSONObj()); // mongos doesn't use metadata but the API requires this call. return DbResponse{reply->done()}; } diff --git a/src/mongo/s/query/async_results_merger_test.cpp b/src/mongo/s/query/async_results_merger_test.cpp index 64428771bb0..7960d22f018 100644 --- a/src/mongo/s/query/async_results_merger_test.cpp +++ b/src/mongo/s/query/async_results_merger_test.cpp @@ -186,7 +186,7 @@ protected: for (const auto& obj : objs) { ASSERT_TRUE(net->hasReadyRequests()); Milliseconds millis(0); - RemoteCommandResponse response(obj, BSONObj(), millis); + RemoteCommandResponse response(obj, millis); executor::TaskExecutor::ResponseStatus responseStatus(response); net->scheduleResponse(net->getNextReadyRequest(), net->now(), responseStatus); } diff --git a/src/mongo/s/sharding_task_executor.cpp b/src/mongo/s/sharding_task_executor.cpp index 5b007cc44ec..2e84102ecc8 100644 --- a/src/mongo/s/sharding_task_executor.cpp +++ b/src/mongo/s/sharding_task_executor.cpp @@ -208,8 +208,7 @@ StatusWith<TaskExecutor::CallbackHandle> ShardingTaskExecutor::scheduleRemoteCom // Update getLastError info for the client if we're tracking it. if (clusterGLE) { - auto swShardingMetadata = - rpc::ShardingMetadata::readFromMetadata(args.response.metadata); + auto swShardingMetadata = rpc::ShardingMetadata::readFromMetadata(args.response.data); if (swShardingMetadata.isOK()) { auto shardingMetadata = std::move(swShardingMetadata.getValue()); @@ -224,7 +223,7 @@ StatusWith<TaskExecutor::CallbackHandle> ShardingTaskExecutor::scheduleRemoteCom } else if (swShardingMetadata.getStatus() != ErrorCodes::NoSuchKey) { warning() << "Got invalid sharding metadata " << redact(swShardingMetadata.getStatus()) << " metadata object was '" - << redact(args.response.metadata) << "'"; + << redact(args.response.data) << "'"; } } }; diff --git a/src/mongo/scripting/mozjs/mongo.cpp b/src/mongo/scripting/mozjs/mongo.cpp index 22a6a728461..0b88d21fb60 100644 --- a/src/mongo/scripting/mozjs/mongo.cpp +++ b/src/mongo/scripting/mozjs/mongo.cpp @@ -248,7 +248,6 @@ void MongoBase::Functions::runCommandWithMetadata::call(JSContext* cx, JS::CallA BSONObjBuilder mergedResultBob; mergedResultBob.append("commandReply", res->getCommandReply()); - mergedResultBob.append("metadata", res->getMetadata()); ValueReader(cx, args.rval()).fromBSON(mergedResultBob.obj(), nullptr, false); setHiddenMongo(cx, std::get<1>(resTuple), conn.get(), args); diff --git a/src/mongo/tools/bridge.cpp b/src/mongo/tools/bridge.cpp index 6938882953d..5793af94803 100644 --- a/src/mongo/tools/bridge.cpp +++ b/src/mongo/tools/bridge.cpp @@ -303,14 +303,12 @@ DbResponse ServiceEntryPointBridge::handleRequest(OperationContext* opCtx, const invariant(!isFireAndForgetCommand); auto replyBuilder = rpc::makeReplyBuilder(rpc::protocolForMessage(request)); - BSONObj metadata; BSONObj reply; StatusWith<BSONObj> commandReply(reply); if (!status->isOK()) { commandReply = StatusWith<BSONObj>(*status); } - return { - replyBuilder->setCommandReply(std::move(commandReply)).setMetadata(metadata).done()}; + return {replyBuilder->setCommandReply(std::move(commandReply)).done()}; } |