diff options
author | William Schultz <william.schultz@mongodb.com> | 2020-02-10 15:15:07 -0500 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-02-10 20:52:39 +0000 |
commit | d80b24a633107f81fcdd2cb7cf9ce0dec89802a1 (patch) | |
tree | 75c4e594921c15474e5890b048819710dc833ca8 /src/mongo/db/repl | |
parent | e8887c0a8015c4f1dff5f8db040584c3eae81bfe (diff) | |
download | mongo-d80b24a633107f81fcdd2cb7cf9ce0dec89802a1.tar.gz |
SERVER-45082 Utilize ConfigVersionAndTerm structure for comparison and string formatting
Diffstat (limited to 'src/mongo/db/repl')
-rw-r--r-- | src/mongo/db/repl/repl_set_config.h | 3 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_config_checks.cpp | 21 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_config_test.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_request_votes_args.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_request_votes_args.h | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp | 25 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl_test.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/repl/topology_coordinator.cpp | 25 | ||||
-rw-r--r-- | src/mongo/db/repl/topology_coordinator_v1_test.cpp | 14 |
10 files changed, 49 insertions, 59 deletions
diff --git a/src/mongo/db/repl/repl_set_config.h b/src/mongo/db/repl/repl_set_config.h index 23d2614adef..36fdb2d0e8e 100644 --- a/src/mongo/db/repl/repl_set_config.h +++ b/src/mongo/db/repl/repl_set_config.h @@ -94,9 +94,8 @@ public: return !(*this < rhs); } - // TODO (SERVER-45082): Implement string conversion. std::string toString() const { - return ""; + return str::stream() << "{version: " << _version << ", term: " << _term << "}"; }; friend std::ostream& operator<<(std::ostream& out, const ConfigVersionAndTerm& cvt) { diff --git a/src/mongo/db/repl/repl_set_config_checks.cpp b/src/mongo/db/repl/repl_set_config_checks.cpp index 20eb82b5cec..19648e8319a 100644 --- a/src/mongo/db/repl/repl_set_config_checks.cpp +++ b/src/mongo/db/repl/repl_set_config_checks.cpp @@ -63,10 +63,10 @@ StatusWith<int> findSelfInConfig(ReplicationCoordinatorExternalState* externalSt } if (meConfigs.empty()) { return StatusWith<int>(ErrorCodes::NodeNotFound, - str::stream() << "No host described in new configuration with term " - << newConfig.getConfigTerm() << " and version " - << newConfig.getConfigVersion() << " for replica set " - << newConfig.getReplSetName() << " maps to this node"); + str::stream() << "No host described in new configuration with " + << newConfig.getConfigVersionAndTerm().toString() + << " for replica set " << newConfig.getReplSetName() + << " maps to this node"); } if (meConfigs.size() > 1) { str::stream message; @@ -75,9 +75,9 @@ StatusWith<int> findSelfInConfig(ReplicationCoordinatorExternalState* externalSt message << ", " << meConfigs[i]->getHostAndPort().toString(); } message << " and " << meConfigs.back()->getHostAndPort().toString() - << " all map to this node in new configuration with term " - << newConfig.getConfigTerm() << " and version " << newConfig.getConfigVersion() - << " for replica set " << newConfig.getReplSetName(); + << " all map to this node in new configuration with " + << newConfig.getConfigVersionAndTerm().toString() << " for replica set " + << newConfig.getReplSetName(); return StatusWith<int>(ErrorCodes::InvalidReplicaSetConfig, message); } @@ -96,10 +96,9 @@ Status checkElectable(const ReplSetConfig& newConfig, int configIndex) { return Status(ErrorCodes::NodeNotElectable, str::stream() << "This node, " << myConfig.getHostAndPort().toString() << ", with _id " << myConfig.getId() - << " is not electable under the new configuration with term " - << newConfig.getConfigTerm() << " and version " - << newConfig.getConfigVersion() << " for replica set " - << newConfig.getReplSetName()); + << " is not electable under the new configuration with " + << newConfig.getConfigVersionAndTerm().toString() + << " for replica set " << newConfig.getReplSetName()); } return Status::OK(); } diff --git a/src/mongo/db/repl/repl_set_config_test.cpp b/src/mongo/db/repl/repl_set_config_test.cpp index bdb4770bfa2..506842f66a3 100644 --- a/src/mongo/db/repl/repl_set_config_test.cpp +++ b/src/mongo/db/repl/repl_set_config_test.cpp @@ -1735,6 +1735,13 @@ TEST(ReplSetConfig, ConfigVersionAndTermComparison) { ASSERT_GT(ConfigVersionAndTerm(2, -1), ConfigVersionAndTerm(1, 1)); ASSERT_GT(ConfigVersionAndTerm(2, -1), ConfigVersionAndTerm(1, -1)); } +TEST(ReplSetConfig, ConfigVersionAndTermToString) { + ASSERT_EQ(ConfigVersionAndTerm(0, 1).toString(), "{version: 0, term: 1}"); + ASSERT_EQ(ConfigVersionAndTerm(0, 2).toString(), "{version: 0, term: 2}"); + ASSERT_EQ(ConfigVersionAndTerm(1, 1).toString(), "{version: 1, term: 1}"); + ASSERT_EQ(ConfigVersionAndTerm(1, 2).toString(), "{version: 1, term: 2}"); + ASSERT_EQ(ConfigVersionAndTerm(1, -1).toString(), "{version: 1, term: -1}"); +} } // namespace } // namespace repl } // namespace mongo diff --git a/src/mongo/db/repl/repl_set_request_votes_args.cpp b/src/mongo/db/repl/repl_set_request_votes_args.cpp index 413fbcf0b4d..e72af47af64 100644 --- a/src/mongo/db/repl/repl_set_request_votes_args.cpp +++ b/src/mongo/db/repl/repl_set_request_votes_args.cpp @@ -129,6 +129,10 @@ long long ReplSetRequestVotesArgs::getConfigTerm() const { return _cfgTerm; } +ConfigVersionAndTerm ReplSetRequestVotesArgs::getConfigVersionAndTerm() const { + return ConfigVersionAndTerm(_cfgVer, _cfgTerm); +} + OpTime ReplSetRequestVotesArgs::getLastDurableOpTime() const { return _lastDurableOpTime; } diff --git a/src/mongo/db/repl/repl_set_request_votes_args.h b/src/mongo/db/repl/repl_set_request_votes_args.h index f17f25a3bbd..c86429e79ed 100644 --- a/src/mongo/db/repl/repl_set_request_votes_args.h +++ b/src/mongo/db/repl/repl_set_request_votes_args.h @@ -32,6 +32,7 @@ #include <string> #include "mongo/db/repl/optime.h" +#include "mongo/db/repl/repl_set_config.h" namespace mongo { @@ -48,6 +49,7 @@ public: long long getCandidateIndex() const; long long getConfigVersion() const; long long getConfigTerm() const; + ConfigVersionAndTerm getConfigVersionAndTerm() const; OpTime getLastDurableOpTime() const; bool isADryRun() const; diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index 5931e3f4f7a..f1db12180a2 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -2825,8 +2825,8 @@ void ReplicationCoordinatorImpl::_finishReplSetReconfig(OperationContext* opCtx, // we have already set our ReplicationCoordinatorImpl::_rsConfigState state to // "kConfigReconfiguring" which prevents new elections from happening. if (electionFinishedEvent) { - LOG(2) << "Waiting for election to complete before finishing reconfig to config with term " - << newConfig.getConfigTerm() << ", version " << newConfig.getConfigVersion(); + LOG(2) << "Waiting for election to complete before finishing reconfig to config with " + << newConfig.getConfigVersionAndTerm(); // Wait for the election to complete and the node's Role to be set to follower. _replExecutor->waitForEvent(electionFinishedEvent); } diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp index a20e7d5b975..db9d264aa16 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp @@ -445,16 +445,14 @@ void ReplicationCoordinatorImpl::_scheduleHeartbeatReconfig_inlock(const ReplSet switch (_rsConfigState) { case kConfigUninitialized: case kConfigSteady: - LOG_FOR_HEARTBEATS(1) << "Received new config via heartbeat with term " - << newConfig.getConfigTerm() << ", version " - << newConfig.getConfigVersion(); + LOG_FOR_HEARTBEATS(1) << "Received new config via heartbeat with " + << newConfig.getConfigVersionAndTerm(); break; case kConfigInitiating: case kConfigReconfiguring: case kConfigHBReconfiguring: - LOG_FOR_HEARTBEATS(1) << "Ignoring new configuration with term " - << newConfig.getConfigTerm() << ", version " - << newConfig.getConfigVersion() + LOG_FOR_HEARTBEATS(1) << "Ignoring new configuration with " + << newConfig.getConfigVersionAndTerm() << " because already in the midst of a configuration process."; return; case kConfigPreStart: @@ -468,9 +466,8 @@ void ReplicationCoordinatorImpl::_scheduleHeartbeatReconfig_inlock(const ReplSet invariant(!_rsConfig.isInitialized() || _rsConfig.getConfigVersionAndTerm() < newConfig.getConfigVersionAndTerm()); if (auto electionFinishedEvent = _cancelElectionIfNeeded_inlock()) { - LOG_FOR_HEARTBEATS(2) << "Rescheduling heartbeat reconfig to config with term " - << newConfig.getConfigTerm() << ", version " - << newConfig.getConfigVersion() + LOG_FOR_HEARTBEATS(2) << "Rescheduling heartbeat reconfig to config with " + << newConfig.getConfigVersionAndTerm() << " to be processed after election is cancelled."; _replExecutor @@ -520,8 +517,7 @@ void ReplicationCoordinatorImpl::_heartbeatReconfigStore( "it is invalid: " << myIndex.getStatus(); } else { - LOG_FOR_HEARTBEATS(2) << "Config with term " << newConfig.getConfigTerm() << ", version " - << newConfig.getConfigVersion() + LOG_FOR_HEARTBEATS(2) << "Config with " << newConfig.getConfigVersionAndTerm() << " validated for reconfig; persisting to disk."; auto opCtx = cc().makeOperationContext(); @@ -558,8 +554,7 @@ void ReplicationCoordinatorImpl::_heartbeatReconfigStore( shouldStartDataReplication = true; } - LOG_FOR_HEARTBEATS(2) << "New configuration with term " << newConfig.getConfigTerm() - << ", version " << newConfig.getConfigVersion() + LOG_FOR_HEARTBEATS(2) << "New configuration with " << newConfig.getConfigVersionAndTerm() << " persisted to local storage; installing new config in memory"; } @@ -602,8 +597,8 @@ void ReplicationCoordinatorImpl::_heartbeatReconfigFinish( stdx::lock_guard<Latch> lk(_mutex); if (auto electionFinishedEvent = _cancelElectionIfNeeded_inlock()) { LOG_FOR_HEARTBEATS(0) - << "Waiting for election to complete before finishing reconfig to config with term " - << newConfig.getConfigTerm() << ", version " << newConfig.getConfigVersion(); + << "Waiting for election to complete before finishing reconfig to config with " + << newConfig.getConfigVersionAndTerm(); // Wait for the election to complete and the node's Role to be set to follower. _replExecutor ->onEvent(electionFinishedEvent, diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp index df8663533ab..aefefa5177e 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp @@ -289,8 +289,7 @@ TEST_F(ReplCoordTest, << "node2:12345"))), &result1); ASSERT_EQUALS(ErrorCodes::InvalidReplicaSetConfig, status); - ASSERT_STRING_CONTAINS(status.reason(), - "is not electable under the new configuration with term"); + ASSERT_STRING_CONTAINS(status.reason(), "is not electable under the new configuration with"); ASSERT_FALSE(getExternalState()->threadsStarted()); } diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp index 2248dc2e450..832373164e9 100644 --- a/src/mongo/db/repl/topology_coordinator.cpp +++ b/src/mongo/db/repl/topology_coordinator.cpp @@ -2773,33 +2773,16 @@ void TopologyCoordinator::processReplSetRequestVotes(const ReplSetRequestVotesAr return; } - // If either config term is -1, ignore the config term entirely and compare config versions. - bool compareConfigTerms = args.getConfigTerm() != -1 && _rsConfig.getConfigTerm() != -1; - if (args.getTerm() < _term) { response->setVoteGranted(false); response->setReason(str::stream() << "candidate's term ({}) is lower than mine ({})"_format( args.getTerm(), _term)); - } else if (compareConfigTerms && args.getConfigTerm() < _rsConfig.getConfigTerm()) { - response->setVoteGranted(false); - response->setReason(str::stream() - << "candidate's term in config(term, version): ({}, {}) is lower " - "than mine ({}, {})"_format(args.getConfigTerm(), - args.getConfigVersion(), - _rsConfig.getConfigTerm(), - _rsConfig.getConfigVersion())); - } else if ((!compareConfigTerms || args.getConfigTerm() == _rsConfig.getConfigTerm()) && - args.getConfigVersion() < _rsConfig.getConfigVersion()) { - // If the terms should not be compared or if the terms are equal, fall back to version - // comparison. + } else if (args.getConfigVersionAndTerm() < _rsConfig.getConfigVersionAndTerm()) { response->setVoteGranted(false); response->setReason(str::stream() - << "ignoring term of -1 for comparison, candidate's version in " - "config(term, version): ({}, {}) is lower than mine ({}, {})"_format( - args.getConfigTerm(), - args.getConfigVersion(), - _rsConfig.getConfigTerm(), - _rsConfig.getConfigVersion())); + << "candidate's config with {} is older " + "than mine with {}"_format(args.getConfigVersionAndTerm(), + _rsConfig.getConfigVersionAndTerm())); } else if (args.getSetName() != _rsConfig.getReplSetName()) { response->setVoteGranted(false); response->setReason(str::stream() diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp index 6756e50e246..9fc313a6042 100644 --- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp +++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp @@ -2798,8 +2798,8 @@ TEST_F(TopoCoordTest, NodeDoesNotGrantVoteWhenConfigVersionIsLower) { getTopoCoord().processReplSetRequestVotes(args, &response); ASSERT_EQUALS( - "ignoring term of -1 for comparison, candidate's version in config(term, version): (1, 0) " - "is lower than mine (1, 1)", + "candidate's config with {version: 0, term: 1} is older than mine with {version: 1, term: " + "1}", response.getReason()); ASSERT_FALSE(response.getVoteGranted()); } @@ -2829,8 +2829,10 @@ TEST_F(TopoCoordTest, NodeDoesNotGrantVoteWhenConfigTermIsLower) { ReplSetRequestVotesResponse response; getTopoCoord().processReplSetRequestVotes(args, &response); - ASSERT_EQUALS("candidate's term in config(term, version): (1, 1) is lower than mine (2, 1)", - response.getReason()); + ASSERT_EQUALS( + "candidate's config with {version: 1, term: 1} is older than mine with {version: 1, term: " + "2}", + response.getReason()); ASSERT_FALSE(response.getVoteGranted()); } @@ -2997,8 +2999,8 @@ TEST_F(TopoCoordTest, NodeDoesNotGrantDryRunVoteWhenConfigVersionIsLower) { getTopoCoord().processReplSetRequestVotes(args, &response); ASSERT_EQUALS( - "ignoring term of -1 for comparison, candidate's version in config(term, version): (1, 0) " - "is lower than mine (1, 1)", + "candidate's config with {version: 0, term: 1} is older than mine with {version: 1, term: " + "1}", response.getReason()); ASSERT_EQUALS(1, response.getTerm()); ASSERT_FALSE(response.getVoteGranted()); |