SERVER-27157 replSetRequestVotes should only store votes with higher terms

(cherry picked from commit 2369c1863931e5c7f671c33421416c9580f80626)

3 files changed, 146 insertions, 1 deletions

diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
index d8518b8d6a1..404ffaeafbb 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
@@ -313,7 +313,24 @@ Status ReplicationCoordinatorExternalStateImpl::storeLocalLastVoteDocument(
         MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN {
             ScopedTransaction transaction(txn, MODE_IX);
             Lock::DBLock dbWriteLock(txn->lockState(), lastVoteDatabaseName, MODE_X);
-            Helpers::putSingleton(txn, lastVoteCollectionName, lastVoteObj);
+
+            // If there is no last vote document, we want to store one. Otherwise, we only want to
+            // replace it if the new last vote document would have a higher term. We both check
+            // the term of the current last vote document and insert the new document under the
+            // DBLock to synchronize the two operations.
+            BSONObj result;
+            bool exists = Helpers::getSingleton(txn, lastVoteCollectionName, result);
+            if (!exists) {
+                Helpers::putSingleton(txn, lastVoteCollectionName, lastVoteObj);
+            } else {
+                StatusWith<LastVote> oldLastVoteDoc = LastVote::readFromLastVote(result);
+                if (!oldLastVoteDoc.isOK()) {
+                    return oldLastVoteDoc.getStatus();
+                }
+                if (lastVote.getTerm() > oldLastVoteDoc.getValue().getTerm()) {
+                    Helpers::putSingleton(txn, lastVoteCollectionName, lastVoteObj);
+                }
+            }
         }
         MONGO_WRITE_CONFLICT_RETRY_LOOP_END(
             txn, "save replica set lastVote", lastVoteCollectionName);
diff --git a/src/mongo/dbtests/SConscript b/src/mongo/dbtests/SConscript
index f56098bfa1f..6f4d062df7c 100644
--- a/src/mongo/dbtests/SConscript
+++ b/src/mongo/dbtests/SConscript
@@ -109,6 +109,7 @@ dbtest = env.Program(
         'query_stage_update.cpp',
         'querytests.cpp',
         'replica_set_monitor_test.cpp',
+        'replica_set_tests.cpp',
         'repltests.cpp',
         'rollbacktests.cpp',
         'socktests.cpp',
diff --git a/src/mongo/dbtests/replica_set_tests.cpp b/src/mongo/dbtests/replica_set_tests.cpp
new file mode 100644
index 00000000000..0d9a7cc3a3b
--- /dev/null
+++ b/src/mongo/dbtests/replica_set_tests.cpp
@@ -0,0 +1,127 @@
+/**
+ *    Copyright (C) 2017 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or  modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects
+ *    for all of the code used other than as permitted herein. If you modify
+ *    file(s) with this exception, you may extend this exception to your
+ *    version of the file(s), but you are not obligated to do so. If you do not
+ *    wish to do so, delete this exception statement from your version. If you
+ *    delete this exception statement from all source files in the program,
+ *    then also delete it in the license file.
+ */
+
+#include "mongo/platform/basic.h"
+
+#include "mongo/db/client.h"
+#include "mongo/db/dbdirectclient.h"
+#include "mongo/db/repl/last_vote.h"
+#include "mongo/db/repl/replication_coordinator_external_state_impl.h"
+#include "mongo/db/service_context.h"
+#include "mongo/stdx/memory.h"
+#include "mongo/unittest/unittest.h"
+
+namespace mongo {
+namespace {
+
+ServiceContext::UniqueOperationContext makeOpCtx() {
+    return cc().makeOperationContext();
+}
+
+class ReplicaSetTest : public mongo::unittest::Test {
+protected:
+    void setUp() {
+        auto txn = makeOpCtx();
+        _replCoordExternalState.reset(new repl::ReplicationCoordinatorExternalStateImpl());
+    }
+
+    void tearDown() {
+        auto txn = makeOpCtx();
+        DBDirectClient client(txn.get());
+        client.dropCollection("local.replset.election");
+
+        _replCoordExternalState.reset();
+    }
+
+    repl::ReplicationCoordinatorExternalStateImpl* getReplCoordExternalState() {
+        return _replCoordExternalState.get();
+    }
+
+private:
+    std::unique_ptr<repl::ReplicationCoordinatorExternalStateImpl> _replCoordExternalState;
+};
+
+TEST_F(ReplicaSetTest, ReplCoordExternalStateStoresLastVoteWithNewTerm) {
+    auto txn = makeOpCtx();
+    auto replCoordExternalState = getReplCoordExternalState();
+
+    replCoordExternalState->storeLocalLastVoteDocument(txn.get(), repl::LastVote{2, 1});
+
+    auto lastVote = replCoordExternalState->loadLocalLastVoteDocument(txn.get());
+    ASSERT_OK(lastVote.getStatus());
+    ASSERT_EQ(lastVote.getValue().getTerm(), 2);
+    ASSERT_EQ(lastVote.getValue().getCandidateIndex(), 1);
+
+    replCoordExternalState->storeLocalLastVoteDocument(txn.get(), repl::LastVote{3, 1});
+
+    lastVote = replCoordExternalState->loadLocalLastVoteDocument(txn.get());
+    ASSERT_OK(lastVote.getStatus());
+    ASSERT_EQ(lastVote.getValue().getTerm(), 3);
+    ASSERT_EQ(lastVote.getValue().getCandidateIndex(), 1);
+}
+
+TEST_F(ReplicaSetTest, ReplCoordExternalStateDoesNotStoreLastVoteWithOldTerm) {
+    auto txn = makeOpCtx();
+    auto replCoordExternalState = getReplCoordExternalState();
+
+    replCoordExternalState->storeLocalLastVoteDocument(txn.get(), repl::LastVote{2, 1});
+
+    auto lastVote = replCoordExternalState->loadLocalLastVoteDocument(txn.get());
+    ASSERT_OK(lastVote.getStatus());
+    ASSERT_EQ(lastVote.getValue().getTerm(), 2);
+    ASSERT_EQ(lastVote.getValue().getCandidateIndex(), 1);
+
+    replCoordExternalState->storeLocalLastVoteDocument(txn.get(), repl::LastVote{1, 1});
+
+    lastVote = replCoordExternalState->loadLocalLastVoteDocument(txn.get());
+    ASSERT_OK(lastVote.getStatus());
+    ASSERT_EQ(lastVote.getValue().getTerm(), 2);
+    ASSERT_EQ(lastVote.getValue().getCandidateIndex(), 1);
+}
+
+TEST_F(ReplicaSetTest, ReplCoordExternalStateDoesNotStoreLastVoteWithEqualTerm) {
+    auto txn = makeOpCtx();
+    auto replCoordExternalState = getReplCoordExternalState();
+
+    replCoordExternalState->storeLocalLastVoteDocument(txn.get(), repl::LastVote{2, 1});
+
+    auto lastVote = replCoordExternalState->loadLocalLastVoteDocument(txn.get());
+    ASSERT_OK(lastVote.getStatus());
+    ASSERT_EQ(lastVote.getValue().getTerm(), 2);
+    ASSERT_EQ(lastVote.getValue().getCandidateIndex(), 1);
+
+    replCoordExternalState->storeLocalLastVoteDocument(txn.get(), repl::LastVote{2, 2});
+
+    lastVote = replCoordExternalState->loadLocalLastVoteDocument(txn.get());
+    ASSERT_OK(lastVote.getStatus());
+    ASSERT_EQ(lastVote.getValue().getTerm(), 2);
+    ASSERT_EQ(lastVote.getValue().getCandidateIndex(), 1);
+}
+
+}  // namespace
+}  // namespace mongo