diff options
author | A. Jesse Jiryu Davis <jesse@mongodb.com> | 2020-04-16 11:25:57 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-04-23 15:13:43 +0000 |
commit | 33adeafe0448512e460b372baf621c8f94d5eb09 (patch) | |
tree | 47ed8bd8f6c76c47c5e8073dfa5c3088883a7fe5 | |
parent | 906e1a72f98d42106c02914bf9cc1dd3101483ac (diff) | |
download | mongo-33adeafe0448512e460b372baf621c8f94d5eb09.tar.gz |
SERVER-47125 Don't trust OplogQueryMetadata.primaryIndex
-rw-r--r-- | src/mongo/db/repl/README.md | 11 | ||||
-rw-r--r-- | src/mongo/db/repl/data_replicator_external_state_impl.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/oplog_fetcher.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/oplog_fetcher_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl_test.cpp | 2 | ||||
-rw-r--r-- | src/mongo/rpc/metadata/oplog_query_metadata.h | 10 | ||||
-rw-r--r-- | src/mongo/rpc/metadata/oplog_query_metadata_test.cpp | 11 |
7 files changed, 26 insertions, 16 deletions
diff --git a/src/mongo/db/repl/README.md b/src/mongo/db/repl/README.md index 0f486fbb08a..8e859471e90 100644 --- a/src/mongo/db/repl/README.md +++ b/src/mongo/db/repl/README.md @@ -346,12 +346,11 @@ It then creates a `ReplSetHeartbeatResponse` object. This includes: 2. The receiving node's election time 3. The receiving node's last applied OpTime 4. The receiving node's last durable OpTime -5. The node the receiving node thinks is primary -6. The term of the receiving node -7. The state of the receiving node -8. The receiving node's sync source -9. The receiving node's `ReplicaSetConfig` version -10. Whether the receiving node is primary +5. The term of the receiving node +6. The state of the receiving node +7. The receiving node's sync source +8. The receiving node's `ReplicaSetConfig` version +9. Whether the receiving node is primary When the sending node receives the response to the heartbeat, it first processes its `ReplSetMetadata` like before. diff --git a/src/mongo/db/repl/data_replicator_external_state_impl.cpp b/src/mongo/db/repl/data_replicator_external_state_impl.cpp index f260fe6cde4..69bd994f544 100644 --- a/src/mongo/db/repl/data_replicator_external_state_impl.cpp +++ b/src/mongo/db/repl/data_replicator_external_state_impl.cpp @@ -86,7 +86,7 @@ void DataReplicatorExternalStateImpl::processMetadata(const rpc::ReplSetMetadata _replicationCoordinator->processReplSetMetadata(replMetadata); - if (oqMetadata.getPrimaryIndex() != rpc::OplogQueryMetadata::kNoPrimary) { + if (oqMetadata.hasPrimaryIndex()) { _replicationCoordinator->cancelAndRescheduleElectionTimeout(); } } diff --git a/src/mongo/db/repl/oplog_fetcher.cpp b/src/mongo/db/repl/oplog_fetcher.cpp index 47c40a9912f..f4f9cc1b7f8 100644 --- a/src/mongo/db/repl/oplog_fetcher.cpp +++ b/src/mongo/db/repl/oplog_fetcher.cpp @@ -859,7 +859,7 @@ Status OplogFetcher::_onSuccessfulBatch(const Documents& documents) { errMsg << " (config version: " << replSetMetadata.getConfigVersion(); errMsg << "; last applied optime: " << oqMetadata.getLastOpApplied().toString(); errMsg << "; sync source index: " << oqMetadata.getSyncSourceIndex(); - errMsg << "; primary index: " << oqMetadata.getPrimaryIndex(); + errMsg << "; has primary index: " << oqMetadata.hasPrimaryIndex(); errMsg << ") is no longer valid"; errMsg << "last fetched optime: " << lastFetched.toString(); return Status(ErrorCodes::InvalidSyncSource, errMsg); diff --git a/src/mongo/db/repl/oplog_fetcher_test.cpp b/src/mongo/db/repl/oplog_fetcher_test.cpp index 37f270dc182..ff0d2b058b7 100644 --- a/src/mongo/db/repl/oplog_fetcher_test.cpp +++ b/src/mongo/db/repl/oplog_fetcher_test.cpp @@ -853,8 +853,8 @@ TEST_F(OplogFetcherTest, ValidMetadataWithInResponseShouldBeForwardedToProcessMe ASSERT_TRUE(dataReplicatorExternalState->metadataWasProcessed); ASSERT_EQUALS(replSetMetadata.getIsPrimary(), dataReplicatorExternalState->replMetadataProcessed.getIsPrimary()); - ASSERT_EQUALS(oqMetadata.getPrimaryIndex(), - dataReplicatorExternalState->oqMetadataProcessed.getPrimaryIndex()); + ASSERT_EQUALS(oqMetadata.hasPrimaryIndex(), + dataReplicatorExternalState->oqMetadataProcessed.hasPrimaryIndex()); } TEST_F(OplogFetcherTest, MetadataAndBatchAreNotProcessedWhenSyncSourceRollsBack) { diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp index c5a695f963b..f72f4b863d6 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp @@ -6388,7 +6388,7 @@ TEST_F(ReplCoordTest, PrepareOplogQueryMetadata) { ASSERT_EQ(oqMetadata.getValue().getLastOpApplied(), optime2); ASSERT_EQ(oqMetadata.getValue().getRBID(), 100); ASSERT_EQ(oqMetadata.getValue().getSyncSourceIndex(), -1); - ASSERT_EQ(oqMetadata.getValue().getPrimaryIndex(), -1); + ASSERT_EQ(oqMetadata.getValue().hasPrimaryIndex(), false); auto replMetadata = rpc::ReplSetMetadata::readFromMetadata(metadata); ASSERT_OK(replMetadata.getStatus()); diff --git a/src/mongo/rpc/metadata/oplog_query_metadata.h b/src/mongo/rpc/metadata/oplog_query_metadata.h index 92ce4e24827..205569a4f31 100644 --- a/src/mongo/rpc/metadata/oplog_query_metadata.h +++ b/src/mongo/rpc/metadata/oplog_query_metadata.h @@ -88,11 +88,13 @@ public: } /** - * Returns the index of the current primary from the perspective of the sender. - * Returns kNoPrimary if there is no primary. + * True if the sender thinks there is a primary. + * + * Note: the $oplogQueryData's primaryIndex isn't safe to use, we don't know which config + * version it's from. All we can safely say is whether the sender thinks there's a primary. */ - int getPrimaryIndex() const { - return _currentPrimaryIndex; + bool hasPrimaryIndex() const { + return _currentPrimaryIndex != kNoPrimary; } /** diff --git a/src/mongo/rpc/metadata/oplog_query_metadata_test.cpp b/src/mongo/rpc/metadata/oplog_query_metadata_test.cpp index bb267ccac59..6d7218e83fc 100644 --- a/src/mongo/rpc/metadata/oplog_query_metadata_test.cpp +++ b/src/mongo/rpc/metadata/oplog_query_metadata_test.cpp @@ -48,6 +48,7 @@ TEST(ReplResponseMetadataTest, OplogQueryMetadataRoundtrip) { ASSERT_EQ(opTime1, metadata.getLastOpCommitted().opTime); ASSERT_EQ(committedWall, metadata.getLastOpCommitted().wallTime); ASSERT_EQ(opTime2, metadata.getLastOpApplied()); + ASSERT_TRUE(metadata.hasPrimaryIndex()); BSONObjBuilder builder; metadata.writeToMetadata(&builder).transitional_ignore(); @@ -75,8 +76,8 @@ TEST(ReplResponseMetadataTest, OplogQueryMetadataRoundtrip) { ASSERT_EQ(opTime2, clonedMetadata.getLastOpApplied()); ASSERT_EQ(committedWall, clonedMetadata.getLastOpCommitted().wallTime); ASSERT_EQ(metadata.getRBID(), clonedMetadata.getRBID()); - ASSERT_EQ(metadata.getPrimaryIndex(), clonedMetadata.getPrimaryIndex()); ASSERT_EQ(metadata.getSyncSourceIndex(), clonedMetadata.getSyncSourceIndex()); + ASSERT_TRUE(clonedMetadata.hasPrimaryIndex()); BSONObjBuilder clonedBuilder; clonedMetadata.writeToMetadata(&clonedBuilder).transitional_ignore(); @@ -85,6 +86,14 @@ TEST(ReplResponseMetadataTest, OplogQueryMetadataRoundtrip) { ASSERT_BSONOBJ_EQ(expectedObj, clonedSerializedObj); } +TEST(ReplResponseMetadataTest, OplogQueryMetadataHasPrimaryIndex) { + for (auto [currentPrimaryIndex, hasPrimaryIndex] : + std::vector<std::pair<int, bool>>{{-1, false}, {0, true}, {1, true}}) { + OplogQueryMetadata oqm({OpTime(), Date_t()}, OpTime(), 1, currentPrimaryIndex, -1); + ASSERT_EQUALS(hasPrimaryIndex, oqm.hasPrimaryIndex()); + } +} + } // unnamed namespace } // namespace rpc } // namespace mongo |