summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorWilliam Schultz <william.schultz@mongodb.com>2020-02-10 15:15:07 -0500
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2020-02-10 20:52:39 +0000
commitd80b24a633107f81fcdd2cb7cf9ce0dec89802a1 (patch)
tree75c4e594921c15474e5890b048819710dc833ca8 /src
parente8887c0a8015c4f1dff5f8db040584c3eae81bfe (diff)
downloadmongo-d80b24a633107f81fcdd2cb7cf9ce0dec89802a1.tar.gz
SERVER-45082 Utilize ConfigVersionAndTerm structure for comparison and string formatting
Diffstat (limited to 'src')
-rw-r--r--src/mongo/db/repl/repl_set_config.h3
-rw-r--r--src/mongo/db/repl/repl_set_config_checks.cpp21
-rw-r--r--src/mongo/db/repl/repl_set_config_test.cpp7
-rw-r--r--src/mongo/db/repl/repl_set_request_votes_args.cpp4
-rw-r--r--src/mongo/db/repl/repl_set_request_votes_args.h2
-rw-r--r--src/mongo/db/repl/replication_coordinator_impl.cpp4
-rw-r--r--src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp25
-rw-r--r--src/mongo/db/repl/replication_coordinator_impl_test.cpp3
-rw-r--r--src/mongo/db/repl/topology_coordinator.cpp25
-rw-r--r--src/mongo/db/repl/topology_coordinator_v1_test.cpp14
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());