SERVER-20928 Remove HeartbeatResponseAction::ScheduleElection action

Author: visualzhou

src/mongo/db/repl/heartbeat_response_action.cpp

@@ -43,12 +43,6 @@ HeartbeatResponseAction HeartbeatResponseAction::makeReconfigAction() {
     return result;
 }
 
-HeartbeatResponseAction HeartbeatResponseAction::makeScheduleElectionAction() {
-    HeartbeatResponseAction result;
-    result._action = ScheduleElection;
-    return result;
-}
-
 HeartbeatResponseAction HeartbeatResponseAction::makePriorityTakeoverAction() {
     HeartbeatResponseAction result;
     result._action = PriorityTakeover;

src/mongo/db/repl/heartbeat_response_action.h

@@ -47,7 +47,6 @@ class HeartbeatResponseAction {
     enum Action {
         NoAction,
         Reconfig,
-        ScheduleElection,
         StartElection,
         StepDownSelf,
         StepDownRemotePrimary,
@@ -64,13 +63,6 @@ class HeartbeatResponseAction {
      */
     static HeartbeatResponseAction makeReconfigAction();
 
-    /**
-     * Makes a new action telling the current node to schedule an election due to election timeout
-     * expiry. If an election timeout is already scheduled, the current node should not reschedule
-     * the timeout. Valid under protocol version 1 only.
-     */
-    static HeartbeatResponseAction makeScheduleElectionAction();
-
     /**
      * Makes a new action telling the current node to attempt to elect itself primary.
      */

src/mongo/db/repl/replication_coordinator_impl_elect_v1_test.cpp

@@ -85,26 +85,6 @@ void ReplCoordElectV1Test::simulateEnoughHeartbeatsForElectability() {
     net->exitNetwork();
 }
 
-TEST_F(ReplCoordElectV1Test, StartElectionDoesNotStartAnElectionWhenNodeHasNoOplogEntries) {
-    logger::globalLogDomain()->setMinimumLoggedSeverity(logger::LogSeverity::Debug(3));
-    // Election never starts because we haven't set a lastOpTimeApplied value yet, via a
-    // heartbeat.
-    startCapturingLogMessages();
-    assertStartSuccess(BSON("_id"
-                            << "mySet"
-                            << "version" << 1 << "members"
-                            << BSON_ARRAY(BSON("_id" << 1 << "host"
-                                                     << "node1:12345")
-                                          << BSON("_id" << 2 << "host"
-                                                        << "node2:12345")) << "protocolVersion"
-                            << 1),
-                       HostAndPort("node1", 12345));
-    ASSERT(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY));
-    simulateEnoughHeartbeatsForElectability();
-    stopCapturingLogMessages();
-    ASSERT_EQUALS(1, countLogLinesContaining("node has no applied oplog entries"));
-}
-
 TEST_F(ReplCoordElectV1Test, ElectionSucceedsWhenNodeIsTheOnlyElectableNode) {
     OperationContextReplMock txn;
     assertStartSuccess(
@@ -643,6 +623,7 @@ TEST_F(ReplCoordElectV1Test, ElectionFailsWhenTermChangesDuringDryRun) {
                                            << BSON("_id" << 3 << "host"
                                                          << "node3:12345")) << "protocolVersion"
                              << 1);
+
     assertStartSuccess(configObj, HostAndPort("node1", 12345));
     ReplicaSetConfig config = assertMakeRSConfig(configObj);
 
@@ -715,42 +696,6 @@ TEST_F(ReplCoordElectV1Test, ElectionFailsWhenTermChangesDuringActualElection) {
                   countLogLinesContaining("not becoming primary, we have been superceded already"));
 }
 
-TEST_F(ReplCoordElectV1Test, ElectionWillNotStartWhenNodeHasRecentlyLearnedAboutANewTerm) {
-    startCapturingLogMessages();
-    BSONObj configObj = BSON("_id"
-                             << "mySet"
-                             << "version" << 1 << "members"
-                             << BSON_ARRAY(BSON("_id" << 1 << "host"
-                                                      << "node1:12345")
-                                           << BSON("_id" << 2 << "host"
-                                                         << "node2:12345")
-                                           << BSON("_id" << 3 << "host"
-                                                         << "node3:12345")) << "protocolVersion"
-                             << 1);
-    assertStartSuccess(configObj, HostAndPort("node1", 12345));
-    ReplicaSetConfig config = assertMakeRSConfig(configObj);
-
-    OperationContextNoop txn;
-    OpTime time1(Timestamp(100, 1), 0);
-    getReplCoord()->setMyLastOptime(time1);
-    ASSERT(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY));
-
-    logger::globalLogDomain()->setMinimumLoggedSeverity(logger::LogSeverity::Debug(2));
-    // Learned about a new term. The following HB won't trigger election during a timeout interval.
-    getReplCoord()->updateTerm(10);
-    simulateEnoughHeartbeatsForElectability();
-    stopCapturingLogMessages();
-    ASSERT(getReplCoord()->getMemberState().secondary())
-        << getReplCoord()->getMemberState().toString();
-    ASSERT_EQ(
-        2, countLogLinesContaining("because I stood up or learned about a new term too recently"));
-    logger::globalLogDomain()->setMinimumLoggedSeverity(logger::LogSeverity::Log());
-
-    simulateSuccessfulV1Election();
-    ASSERT(getReplCoord()->getMemberState().primary())
-        << getReplCoord()->getMemberState().toString();
-}
-
 TEST_F(ReplCoordElectV1Test, SchedulesPriorityTakeoverIfNodeHasHigherPriorityThanCurrentPrimary) {
     startCapturingLogMessages();
     BSONObj configObj = BSON("_id"

src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp

@@ -215,13 +215,6 @@ void ReplicationCoordinatorImpl::_handleHeartbeatResponseAction(
             invariant(responseStatus.isOK());
             _scheduleHeartbeatReconfig(responseStatus.getValue().getConfig());
             break;
-        case HeartbeatResponseAction::ScheduleElection:
-            DEV {
-                // Election timer should already be periodically scheduled at this point.
-                stdx::unique_lock<stdx::mutex> lk(_mutex);
-                fassert(28813, _handleElectionTimeoutCbh.isValid());
-            }
-            break;
         case HeartbeatResponseAction::StartElection:
             _startElectSelf();
             break;
@@ -250,9 +243,6 @@ void ReplicationCoordinatorImpl::_handleHeartbeatResponseAction(
             }
             break;
         }
-        default:
-            severe() << "Illegal heartbeat response action code " << int(action.getAction());
-            invariant(false);
     }
 }
 

src/mongo/db/repl/topology_coordinator_impl.cpp

@@ -1043,107 +1043,56 @@ HeartbeatResponseAction TopologyCoordinatorImpl::_updatePrimaryFromHBDataV1(
     const MemberState& originalState,
     Date_t now,
     const OpTime& lastOpApplied) {
-    // This method has two interrelated responsibilities, performed in two phases.
     //
-    // First, it updates the local notion of which remote node, if any is primary.
+    // Updates the local notion of which remote node, if any is primary.
+    // Start the priority takeover process if we are eligible.
     //
-    // Second, if there is no remote primary, and the local node is not primary, it considers
-    // whether or not to stand for election.
+
     invariant(updatedConfigIndex != _selfIndex);
 
-    // We are missing from the config, so do not participate in primary maintenance or election.
+    // If we are missing from the config, do not participate in primary maintenance or election.
     if (_selfIndex == -1) {
         return HeartbeatResponseAction::makeNoAction();
     }
-
-    ////////////////////
-    // Phase 1
-    ////////////////////
-
-    // If we believe the node whose data was just updated is primary, confirm that
-    // the updated data supports that notion.  If not, erase our notion of who is primary.
-    if (updatedConfigIndex == _currentPrimaryIndex) {
-        const MemberHeartbeatData& updatedHBData = _hbdata[updatedConfigIndex];
-        if (!updatedHBData.up() || !updatedHBData.getState().primary()) {
-            _currentPrimaryIndex = -1;
-        }
+    // If we are the primary, there must be no other primary, otherwise its higher term would
+    // have already made us step down.
+    if (_currentPrimaryIndex == _selfIndex) {
+        return HeartbeatResponseAction::makeNoAction();
     }
 
     // Scan the member list's heartbeat data for who is primary, and update _currentPrimaryIndex.
-    if (_currentPrimaryIndex != _selfIndex) {
-        int remotePrimaryIndex = -1;
-        for (std::vector<MemberHeartbeatData>::const_iterator it = _hbdata.begin();
-             it != _hbdata.end();
-             ++it) {
-            const int itIndex = indexOfIterator(_hbdata, it);
-            if (itIndex == _selfIndex) {
-                continue;
-            }
-
-            if (it->getState().primary() && it->up()) {
-                if (remotePrimaryIndex == -1 ||
-                    _hbdata[remotePrimaryIndex].getTerm() < it->getTerm()) {
-                    remotePrimaryIndex = itIndex;
-                }
+    int primaryIndex = -1;
+    for (size_t i = 0; i < _hbdata.size(); i++) {
+        const MemberHeartbeatData& member = _hbdata[i];
+        if (member.getState().primary() && member.up()) {
+            if (primaryIndex == -1 || _hbdata[primaryIndex].getTerm() < member.getTerm()) {
+                primaryIndex = i;
             }
         }
-
-        if (remotePrimaryIndex != -1) {
-            // Clear last heartbeat message on ourselves.
-            setMyHeartbeatMessage(now, "");
-
-            _currentPrimaryIndex = remotePrimaryIndex;
-
-            // Priority takeover when the replset is stable.
-            //
-            // Take over the primary only if the remote primary is in the latest term I know.
-            // Otherwise, there must be an outstanding election, which may succeed or not, but
-            // the remote primary will become aware of that election eventually and step down.
-            if (_hbdata[remotePrimaryIndex].getTerm() == _term &&
-                _rsConfig.getMemberAt(remotePrimaryIndex).getPriority() <
-                    _rsConfig.getMemberAt(_selfIndex).getPriority()) {
-                LOG(4) << "I can take over the primary due to higher priority."
-                       << " Current primary index: " << remotePrimaryIndex << " in term "
-                       << _hbdata[remotePrimaryIndex].getTerm();
-
-                return HeartbeatResponseAction::makePriorityTakeoverAction();
-            }
-            return HeartbeatResponseAction::makeNoAction();
-        }
     }
-
-    ////////////////////
-    // Phase 2
-    ////////////////////
-
-    // We do not believe any remote to be primary.
-
-    // Return if we are primary. The stepdown decision is based on liveness rather than
-    // heartbeats in pv 1.
-    if (_iAmPrimary()) {
+    _currentPrimaryIndex = primaryIndex;
+    if (_currentPrimaryIndex == -1) {
         return HeartbeatResponseAction::makeNoAction();
     }
 
-    fassert(28798, _currentPrimaryIndex == -1);
+    // Clear last heartbeat message on ourselves.
+    setMyHeartbeatMessage(now, "");
 
-    const MemberState currentState = getMemberState();
-    if (originalState.recovering() && currentState.secondary()) {
-        // We just transitioned from RECOVERING to SECONDARY, this can only happen if we
-        // received a heartbeat with an auth error when previously all the heartbeats we'd
-        // received had auth errors.  In this case, don't return makeElectAction() because
-        // that could cause the election to start before the ReplicationCoordinator has updated
-        // its notion of the member state to SECONDARY.  Instead return noAction so that the
-        // ReplicationCoordinator knows to update its tracking of the member state off of the
-        // TopologyCoordinator, and leave starting the election until the next heartbeat comes
-        // back.
-        return HeartbeatResponseAction::makeNoAction();
-    }
-
-    // At this point, there is no primary anywhere.  Check to see if we should become a candidate.
-    if (!checkShouldStandForElection(now, lastOpApplied)) {
-        return HeartbeatResponseAction::makeNoAction();
+    // Priority takeover when the replset is stable.
+    //
+    // Take over the primary only if the remote primary is in the latest term I know.
+    // Otherwise, there must be an outstanding election, which may succeed or not, but
+    // the remote primary will become aware of that election eventually and step down.
+    if (_hbdata[primaryIndex].getTerm() == _term &&
+        _rsConfig.getMemberAt(primaryIndex).getPriority() <
+            _rsConfig.getMemberAt(_selfIndex).getPriority()) {
+        LOG(4) << "I can take over the primary due to higher priority."
+               << " Current primary index: " << primaryIndex << " in term "
+               << _hbdata[primaryIndex].getTerm();
+
+        return HeartbeatResponseAction::makePriorityTakeoverAction();
     }
-    return HeartbeatResponseAction::makeScheduleElectionAction();
+    return HeartbeatResponseAction::makeNoAction();
 }
 
 HeartbeatResponseAction TopologyCoordinatorImpl::_updatePrimaryFromHBData(