SERVER-20707 Add --enableMajorityReadConcern option

15 files changed, 91 insertions, 74 deletions

diff --git a/src/mongo/base/error_codes.err b/src/mongo/base/error_codes.err
index f0e0386de39..6f2c71429d7 100644
--- a/src/mongo/base/error_codes.err
+++ b/src/mongo/base/error_codes.err
@@ -147,6 +147,7 @@ error_code("IncompatibleCatalogManager", 144)
 error_code("PooledConnectionsDropped", 145)
 error_code("ExceededMemoryLimit", 146)
 error_code("ZLibError", 147)
+error_code("ReadConcernMajorityNotEnabled", 148)
 
 # Non-sequential error codes (for compatibility only)
 error_code("RecvStaleConfig", 9996)
diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp
index 0ec2df52760..28e6220ee81 100644
--- a/src/mongo/db/db.cpp
+++ b/src/mongo/db/db.cpp
@@ -464,6 +464,20 @@ static void _initAndListen(int listenPort) {
 
     const repl::ReplSettings& replSettings = repl::getGlobalReplicationCoordinator()->getSettings();
 
+    if (!getGlobalServiceContext()->getGlobalStorageEngine()->getSnapshotManager()) {
+        if (moe::startupOptionsParsed.count("replication.enableMajorityReadConcern")) {
+            // Note: we are intentionally only erroring if the user explicitly requested that we
+            // enable majority read concern. We do not error if the they are implicitly enabled for
+            // CSRS because a required step in the upgrade procedure can involve an mmapv1 node in
+            // the CSRS in the REMOVED state. This is handled by the TopologyCoordinator.
+            invariant(replSettings.majorityReadConcernEnabled);
+            severe() << "Majority read concern requires a storage engine that supports"
+                     << "snapshots, such as wiredTiger. " << storageGlobalParams.engine
+                     << " does not support snapshots.";
+            exitCleanly(EXIT_BADOPTIONS);
+        }
+    }
+
     {
         ProcessId pid = ProcessId::getCurrent();
         LogstreamBuilder l = log(LogComponent::kControl);
diff --git a/src/mongo/db/mongod_options.cpp b/src/mongo/db/mongod_options.cpp
index 4127a610fb0..d21a9df3e14 100644
--- a/src/mongo/db/mongod_options.cpp
+++ b/src/mongo/db/mongod_options.cpp
@@ -418,6 +418,11 @@ Status addMongodOptions(moe::OptionSection* options) {
                    "specify index prefetching behavior (if secondary) [none|_id_only|all]")
         .format("(:?none)|(:?_id_only)|(:?all)", "(none/_id_only/all)");
 
+    rs_options.addOptionChaining("replication.enableMajorityReadConcern",
+                                 "enableMajorityReadConcern",
+                                 moe::Switch,
+                                 "enables majority readConcern");
+
     // Sharding Options
 
     sharding_options.addOptionChaining(
@@ -1137,6 +1142,10 @@ Status storeMongodOptions(const moe::Environment& params, const std::vector<std:
             params["replication.secondaryIndexPrefetch"].as<std::string>();
     }
 
+    if (params.count("replication.enableMajorityReadConcern")) {
+        replSettings.majorityReadConcernEnabled = true;
+    }
+
     if (params.count("storage.indexBuildRetry")) {
         serverGlobalParams.indexBuildRetry = params["storage.indexBuildRetry"].as<bool>();
     }
@@ -1233,6 +1242,10 @@ Status storeMongodOptions(const moe::Environment& params, const std::vector<std:
         serverGlobalParams.configsvrMode = CatalogManager::ConfigServerMode::SCCC;
     }
 
+    if (serverGlobalParams.configsvrMode == CatalogManager::ConfigServerMode::CSRS) {
+        replSettings.majorityReadConcernEnabled = true;
+    }
+
     if (params.count("sharding.archiveMovedChunks")) {
         serverGlobalParams.moveParanoia = params["sharding.archiveMovedChunks"].as<bool>();
     }
diff --git a/src/mongo/db/repl/repl_settings.h b/src/mongo/db/repl/repl_settings.h
index 1ce5fa6b4dc..cd5a67d52e4 100644
--- a/src/mongo/db/repl/repl_settings.h
+++ b/src/mongo/db/repl/repl_settings.h
@@ -49,82 +49,48 @@ typedef enum { NotSlave = 0, SimpleSlave } SlaveTypes;
 
 class ReplSettings {
 public:
-    SlaveTypes slave;
+    std::string ourSetName() const {
+        std::string setname;
+        size_t sl = replSet.find('/');
+        if (sl == std::string::npos)
+            return replSet;
+        return replSet.substr(0, sl);
+    }
+    bool usingReplSets() const {
+        return !replSet.empty();
+    }
+
+    SlaveTypes slave = NotSlave;
 
     /**
      * true means we are master and doing replication.  if we are not writing to oplog, this won't
      * be true.
      */
-    bool master;
+    bool master = false;
+
+    bool fastsync = false;
 
-    bool fastsync;
+    bool autoresync = false;
 
-    bool autoresync;
+    int slavedelay = 0;
 
-    int slavedelay;
+    long long oplogSize = 0;  // --oplogSize
 
-    long long oplogSize;  // --oplogSize
+    /**
+     * True means that the majorityReadConcern feature is enabled, either explicitly by the user or
+     * implicitly by a requiring feature such as CSRS. It does not mean that the storage engine
+     * supports snapshots or that the snapshot thread is running. Those are tracked separately.
+     */
+    bool majorityReadConcernEnabled = false;
 
     // for master/slave replication
     std::string source;  // --source
     std::string only;    // --only
-    int pretouch;        // --pretouch for replication application (experimental)
+    int pretouch = 0;    // --pretouch for replication application (experimental)
 
     std::string replSet;  // --replSet[/<seedlist>]
-    std::string ourSetName() const {
-        std::string setname;
-        size_t sl = replSet.find('/');
-        if (sl == std::string::npos)
-            return replSet;
-        return replSet.substr(0, sl);
-    }
-    bool usingReplSets() const {
-        return !replSet.empty();
-    }
 
     std::string rsIndexPrefetch;  // --indexPrefetch
-
-    ReplSettings()
-        : slave(NotSlave),
-          master(false),
-          fastsync(),
-          autoresync(false),
-          slavedelay(),
-          oplogSize(0),
-          pretouch(0) {}
-
-    // TODO(spencer): Remove explicit copy constructor after we no longer have mutable state
-    // in ReplSettings.
-    ReplSettings(const ReplSettings& other)
-        : slave(other.slave),
-          master(other.master),
-          fastsync(other.fastsync),
-          autoresync(other.autoresync),
-          slavedelay(other.slavedelay),
-          oplogSize(other.oplogSize),
-          source(other.source),
-          only(other.only),
-          pretouch(other.pretouch),
-          replSet(other.replSet),
-          rsIndexPrefetch(other.rsIndexPrefetch) {}
-
-    ReplSettings& operator=(const ReplSettings& other) {
-        if (this == &other)
-            return *this;
-
-        slave = other.slave;
-        master = other.master;
-        fastsync = other.fastsync;
-        autoresync = other.autoresync;
-        slavedelay = other.slavedelay;
-        oplogSize = other.oplogSize;
-        source = other.source;
-        only = other.only;
-        pretouch = other.pretouch;
-        replSet = other.replSet;
-        rsIndexPrefetch = other.rsIndexPrefetch;
-        return *this;
-    }
 };
 
 }  // namespace repl
diff --git a/src/mongo/db/repl/replication_coordinator_external_state.h b/src/mongo/db/repl/replication_coordinator_external_state.h
index 0692975b668..ebb17a839ab 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state.h
+++ b/src/mongo/db/repl/replication_coordinator_external_state.h
@@ -49,6 +49,7 @@ class StatusWith;
 namespace repl {
 
 class LastVote;
+class ReplSettings;
 
 /**
  * This class represents the interface the ReplicationCoordinator uses to interact with the
@@ -68,7 +69,7 @@ public:
      *
      * NOTE: Only starts threads if they are not already started,
      */
-    virtual void startThreads() = 0;
+    virtual void startThreads(const ReplSettings& settings) = 0;
 
     /**
      * Starts the Master/Slave threads and sets up logOp
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 6e97d9c4987..dbc79f45bef 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
@@ -51,6 +51,7 @@
 #include "mongo/db/repl/last_vote.h"
 #include "mongo/db/repl/master_slave.h"
 #include "mongo/db/repl/oplog.h"
+#include "mongo/db/repl/repl_settings.h"
 #include "mongo/db/repl/rs_sync.h"
 #include "mongo/db/repl/snapshot_thread.h"
 #include "mongo/db/server_parameters.h"
@@ -89,7 +90,7 @@ ReplicationCoordinatorExternalStateImpl::ReplicationCoordinatorExternalStateImpl
     : _startedThreads(false), _nextThreadId(0) {}
 ReplicationCoordinatorExternalStateImpl::~ReplicationCoordinatorExternalStateImpl() {}
 
-void ReplicationCoordinatorExternalStateImpl::startThreads() {
+void ReplicationCoordinatorExternalStateImpl::startThreads(const ReplSettings& settings) {
     stdx::lock_guard<stdx::mutex> lk(_threadMutex);
     if (_startedThreads) {
         return;
@@ -100,7 +101,7 @@ void ReplicationCoordinatorExternalStateImpl::startThreads() {
     _producerThread.reset(new stdx::thread(stdx::bind(&BackgroundSync::producerThread, bgsync)));
     _syncSourceFeedbackThread.reset(
         new stdx::thread(stdx::bind(&SyncSourceFeedback::run, &_syncSourceFeedback)));
-    if (enableReplSnapshotThread) {
+    if (settings.majorityReadConcernEnabled || enableReplSnapshotThread) {
         _snapshotThread = SnapshotThread::start(getGlobalServiceContext());
     }
     _startedThreads = true;
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.h b/src/mongo/db/repl/replication_coordinator_external_state_impl.h
index 83d7a3c7d34..584e6e89712 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_impl.h
+++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.h
@@ -49,7 +49,7 @@ class ReplicationCoordinatorExternalStateImpl : public ReplicationCoordinatorExt
 public:
     ReplicationCoordinatorExternalStateImpl();
     virtual ~ReplicationCoordinatorExternalStateImpl();
-    virtual void startThreads() override;
+    virtual void startThreads(const ReplSettings& settings) override;
     virtual void startMasterSlave(OperationContext* txn);
     virtual void shutdown();
     virtual void initiateOplog(OperationContext* txn, bool updateReplOpTime);
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp b/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp
index 86a37f54f18..ee4236a5718 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp
+++ b/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp
@@ -58,7 +58,7 @@ ReplicationCoordinatorExternalStateMock::ReplicationCoordinatorExternalStateMock
 
 ReplicationCoordinatorExternalStateMock::~ReplicationCoordinatorExternalStateMock() {}
 
-void ReplicationCoordinatorExternalStateMock::startThreads() {
+void ReplicationCoordinatorExternalStateMock::startThreads(const ReplSettings& settings) {
     _threadsStarted = true;
 }
 
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_mock.h b/src/mongo/db/repl/replication_coordinator_external_state_mock.h
index e6a259ca0ea..73bcb7067a5 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_mock.h
+++ b/src/mongo/db/repl/replication_coordinator_external_state_mock.h
@@ -52,7 +52,7 @@ public:
 
     ReplicationCoordinatorExternalStateMock();
     virtual ~ReplicationCoordinatorExternalStateMock();
-    virtual void startThreads() override;
+    virtual void startThreads(const ReplSettings& settings) override;
     virtual void startMasterSlave(OperationContext*);
     virtual void shutdown();
     virtual void initiateOplog(OperationContext* txn, bool updateReplOpTime);
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 8e071e9a496..76ebd28cddc 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -398,7 +398,7 @@ void ReplicationCoordinatorImpl::_finishLoadLocalConfig(
     }
     _performPostMemberStateUpdateAction(action);
     if (!isArbiter) {
-        _externalState->startThreads();
+        _externalState->startThreads(_settings);
     }
 }
 
@@ -831,6 +831,17 @@ OpTime ReplicationCoordinatorImpl::getMyLastOptime() const {
 
 ReadConcernResponse ReplicationCoordinatorImpl::waitUntilOpTime(OperationContext* txn,
                                                                 const ReadConcernArgs& settings) {
+    const bool isMajorityReadConcern =
+        settings.getLevel() == ReadConcernLevel::kMajorityReadConcern;
+
+    if (isMajorityReadConcern && !getSettings().majorityReadConcernEnabled) {
+        // This is an opt-in feature. Fail if the user didn't opt-in.
+        return ReadConcernResponse(
+            Status(ErrorCodes::ReadConcernMajorityNotEnabled,
+                   "Majority read concern requested, but server was not started with "
+                   "--enableMajorityReadConcern."));
+    }
+
     const auto ts = settings.getOpTime();
     // Note that if 'settings' has no explicit after-optime, 'ts' will be the earliest
     // possible optime, which means the comparisons with 'ts' below are always false.  This is
@@ -852,7 +863,6 @@ ReadConcernResponse ReplicationCoordinatorImpl::waitUntilOpTime(OperationContext
 
     Timer timer;
     stdx::unique_lock<stdx::mutex> lock(_mutex);
-    bool isMajorityReadConcern = settings.getLevel() == ReadConcernLevel::kMajorityReadConcern;
     if (isMajorityReadConcern && !_externalState->snapshotsEnabled()) {
         return ReadConcernResponse(
             Status(ErrorCodes::CommandNotSupported,
@@ -2121,7 +2131,7 @@ Status ReplicationCoordinatorImpl::processReplSetInitiate(OperationContext* txn,
         // A configuration passed to replSetInitiate() with the current node as an arbiter
         // will fail validation with a "replSet initiate got ... while validating" reason.
         invariant(!newConfig.getMemberAt(myIndex.getValue()).isArbiter());
-        _externalState->startThreads();
+        _externalState->startThreads(_settings);
     }
 
     return Status::OK();
diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
index d69003b4b73..f744cce5fa1 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
@@ -427,7 +427,7 @@ void ReplicationCoordinatorImpl::_heartbeatReconfigStore(
         bool isArbiter = myIndex.isOK() && myIndex.getValue() != -1 &&
             newConfig.getMemberAt(myIndex.getValue()).isArbiter();
         if (!isArbiter) {
-            _externalState->startThreads();
+            _externalState->startThreads(_settings);
         }
     }
 
diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp
index 7034ab1c4b6..db7193bc958 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp
@@ -2357,7 +2357,10 @@ TEST_F(ReplCoordTest, ReadAfterEqualOpTime) {
 }
 
 TEST_F(ReplCoordTest, CantUseReadAfterCommittedIfNotReplSet) {
-    init(ReplSettings());
+    auto settings = ReplSettings();
+    settings.majorityReadConcernEnabled = true;
+    init(settings);
+
     OperationContextNoop txn;
     auto result = getReplCoord()->waitUntilOpTime(
         &txn, ReadConcernArgs(OpTime(Timestamp(50, 0), 0), ReadConcernLevel::kMajorityReadConcern));
@@ -2366,6 +2369,16 @@ TEST_F(ReplCoordTest, CantUseReadAfterCommittedIfNotReplSet) {
     ASSERT_EQUALS(ErrorCodes::NotAReplicaSet, result.getStatus());
 }
 
+TEST_F(ReplCoordTest, CantUseReadAfterCommittedIfNotEnabled) {
+    init(ReplSettings());
+    OperationContextNoop txn;
+    auto result = getReplCoord()->waitUntilOpTime(
+        &txn, ReadConcernArgs(OpTime(Timestamp(50, 0), 0), ReadConcernLevel::kMajorityReadConcern));
+
+    ASSERT_FALSE(result.didWait());
+    ASSERT_EQUALS(ErrorCodes::ReadConcernMajorityNotEnabled, result.getStatus());
+}
+
 TEST_F(ReplCoordTest, ReadAfterCommittedWhileShutdown) {
     OperationContextNoop txn;
     assertStartSuccess(BSON("_id"
diff --git a/src/mongo/db/repl/replication_coordinator_test_fixture.cpp b/src/mongo/db/repl/replication_coordinator_test_fixture.cpp
index 738626eabdc..c0bc9096c14 100644
--- a/src/mongo/db/repl/replication_coordinator_test_fixture.cpp
+++ b/src/mongo/db/repl/replication_coordinator_test_fixture.cpp
@@ -75,6 +75,7 @@ BSONObj ReplCoordTest::addProtocolVersion(const BSONObj& configDoc, int protocol
 
 void ReplCoordTest::setUp() {
     _settings.replSet = "mySet/node1:12345,node2:54321";
+    _settings.majorityReadConcernEnabled = true;
 }
 
 void ReplCoordTest::tearDown() {
diff --git a/src/mongo/shell/replsettest.js b/src/mongo/shell/replsettest.js
index bf2d6888c00..5390d497ab8 100644
--- a/src/mongo/shell/replsettest.js
+++ b/src/mongo/shell/replsettest.js
@@ -767,7 +767,7 @@ ReplSetTest.prototype.start = function( n , options , restart , wait ) {
     
     var rval = this.nodes[n] = MongoRunner.runMongod( options )
 
-    if(!rval) return rval;
+    if(!rval) throw Error("Failed to start node " + n);
     
     // Add replica set specific attributes
     this.nodes[n].nodeId = n
diff --git a/src/mongo/shell/servers.js b/src/mongo/shell/servers.js
index 0cd0e11bf5f..0b48c97e7ae 100755
--- a/src/mongo/shell/servers.js
+++ b/src/mongo/shell/servers.js
@@ -793,9 +793,6 @@ function appendSetParameterArgs(argArray) {
                     });
                 }
             }
-            if (argArray.indexOf('--configsvr') > 0 && argArray.indexOf('--replSet') > 0) {
-                argArray.push.apply(argArray, ['--setParameter', "enableReplSnapshotThread=1"]);
-            }
         }
     }
     return argArray;