diff options
14 files changed, 76 insertions, 18 deletions
diff --git a/jstests/replsets/reconfig_waits_for_a_majority_to_replicate_config.js b/jstests/replsets/reconfig_waits_for_a_majority_to_replicate_config.js index 27997337f78..532c0e34403 100644 --- a/jstests/replsets/reconfig_waits_for_a_majority_to_replicate_config.js +++ b/jstests/replsets/reconfig_waits_for_a_majority_to_replicate_config.js @@ -12,7 +12,7 @@ load("jstests/replsets/rslib.js"); load("jstests/libs/fail_point_util.js"); var replTest = new ReplSetTest({nodes: 2, useBridge: true}); -var nodes = replTest.startSet(); +replTest.startSet(); // Initiating with a high election timeout prevents unnecessary elections and also prevents // the primary from stepping down if it cannot communicate with the secondary. replTest.initiateWithHighElectionTimeout(); @@ -32,6 +32,7 @@ config.version++; assert.commandFailedWithCode( primary.getDB("admin").runCommand({replSetReconfig: config, maxTimeMS: 5000}), ErrorCodes.MaxTimeMSExpired); +assert.eq(isConfigCommitted(primary), false); // Try to run another reconfig, which should also fail immediately because the previous config is // not committed. @@ -40,12 +41,18 @@ config.version++; assert.commandFailedWithCode( primary.getDB("admin").runCommand({replSetReconfig: config, maxTimeMS: 5000}), ErrorCodes.ConfigurationInProgress); +assert.eq(isConfigCommitted(primary), false); // Reconnect the secondary to the primary. secondary.reconnect(primary); -// TODO SERVER-44812: test commitment status of reconfig. +assert.soon(() => isConfigCommitted(primary)); + +// Reconfig should now succeed. +config.version++; +assert.commandWorked(primary.getDB("admin").runCommand({replSetReconfig: config})); +assert(isConfigCommitted(primary)); reconfigFailPoint.off(); replTest.stopSet(); -}());
\ No newline at end of file +}()); diff --git a/jstests/replsets/reconfig_waits_for_oplog_commitment_condition.js b/jstests/replsets/reconfig_waits_for_oplog_commitment_condition.js index 92006ee16b9..0d8743092ed 100644 --- a/jstests/replsets/reconfig_waits_for_oplog_commitment_condition.js +++ b/jstests/replsets/reconfig_waits_for_oplog_commitment_condition.js @@ -7,6 +7,7 @@ (function() { "use strict"; load("jstests/libs/write_concern_util.js"); +load("jstests/replsets/rslib.js"); const dbName = "test"; const collName = "coll"; @@ -36,6 +37,7 @@ let singleNodeConfig = Object.assign({}, origConfig); singleNodeConfig.members = singleNodeConfig.members.slice(0, 1); // Remove the second node. singleNodeConfig.version++; assert.commandWorked(primary.adminCommand({replSetReconfig: singleNodeConfig})); +assert(isConfigCommitted(primary)); // // Below we start out in config C1 = {n0}, try to reconfig to C2 = {n0,n1}, and then to C3 = @@ -66,10 +68,10 @@ jsTestLog("Reconfig to add the secondary back in."); // committed in C2. assert.commandFailedWithCode(primary.adminCommand({replSetReconfig: C2, maxTimeMS: 1000}), ErrorCodes.MaxTimeMSExpired); +assert.eq(isConfigCommitted(primary), false); // Wait until the config has propagated to the secondary and the primary has learned of it, so that // the config replication check is satisfied. -// TODO (SERVER-44812): Wait for this by checking commitment status. assert.soon(function() { const res = primary.adminCommand({replSetGetStatus: 1}); return res.members[1].configVersion === rst.getReplSetConfigFromNode().version; @@ -78,6 +80,7 @@ assert.soon(function() { // Reconfig should fail immediately since we have not committed the last committed op from C1 in C2. assert.commandFailedWithCode(primary.adminCommand({replSetReconfig: C3}), ErrorCodes.ConfigurationInProgress); +assert.eq(isConfigCommitted(primary), false); // Make sure we can connect to the secondary after it was REMOVED. reconnect(secondary); @@ -85,10 +88,11 @@ reconnect(secondary); // Let the last committed op from C1 become committed in the current config. restartServerReplication(secondary); rst.awaitReplication(); +assert.soon(() => isConfigCommitted(primary)); // Now that we can commit the op in the new config, reconfig should succeed. assert.commandWorked(primary.adminCommand({replSetReconfig: C3})); - +assert(isConfigCommitted(primary)); rst.awaitReplication(); // @@ -106,6 +110,7 @@ stopServerReplication(secondary); // config. config.version++; assert.commandWorked(primary.adminCommand({replSetReconfig: config})); +assert(isConfigCommitted(primary)); jsTestLog("Stepping down the primary."); // Step down the primary and then step it back up so that it writes a log entry in a newer term. @@ -118,6 +123,7 @@ rst.stepUpNoAwaitReplication(primary); assert.eq(primary, rst.getPrimary()); // Reconfig should now fail since the primary has not yet committed an op in its term. +assert.eq(isConfigCommitted(primary), false); config.version++; assert.commandFailedWithCode(primary.adminCommand({replSetReconfig: config}), ErrorCodes.ConfigurationInProgress); @@ -125,10 +131,11 @@ assert.commandFailedWithCode(primary.adminCommand({replSetReconfig: config}), // Restart server replication to let the primary commit an op. restartServerReplication(secondary); rst.awaitLastOpCommitted(); +assert.soon(() => isConfigCommitted(primary)); // Reconfig should now succeed. config.version++; assert.commandWorked(primary.adminCommand({replSetReconfig: config})); - +assert(isConfigCommitted(primary)); rst.stopSet(); }()); diff --git a/jstests/replsets/reconfig_waits_for_oplog_commitment_condition_when_leaving_force.js b/jstests/replsets/reconfig_waits_for_oplog_commitment_condition_when_leaving_force.js index 960a7fac667..8481679f903 100644 --- a/jstests/replsets/reconfig_waits_for_oplog_commitment_condition_when_leaving_force.js +++ b/jstests/replsets/reconfig_waits_for_oplog_commitment_condition_when_leaving_force.js @@ -8,6 +8,7 @@ (function() { "use strict"; load("jstests/libs/write_concern_util.js"); +load("jstests/replsets/rslib.js"); const dbName = "test"; const collName = "coll"; @@ -39,6 +40,7 @@ singleNodeConfig.version++; jsTestLog("Force reconfig down to a single node."); assert.commandWorked(primary.adminCommand({replSetReconfig: singleNodeConfig, force: true})); +assert(isConfigCommitted(primary)); jsTestLog("Do a write on primary and commit it in the current config."); assert.commandWorked(coll.insert({x: 1}, {writeConcern: {w: "majority"}})); @@ -53,7 +55,6 @@ assert.commandFailedWithCode( // Wait until the config has propagated to the secondary and the primary has learned of it, so that // the config replication check is satisfied. -// TODO (SERVER-44812): Wait for this by checking commitment status. assert.soon(function() { const res = primary.adminCommand({replSetGetStatus: 1}); return res.members[1].configVersion === rst.getReplSetConfigFromNode().version; @@ -61,6 +62,7 @@ assert.soon(function() { // Reconfig should fail immediately since we have not committed the last committed op in the current // config. +assert.eq(isConfigCommitted(primary), false); twoNodeConfig.version = rst.getReplSetConfigFromNode().version + 1; assert.commandFailedWithCode(primary.adminCommand({replSetReconfig: twoNodeConfig}), ErrorCodes.ConfigurationInProgress); @@ -71,10 +73,11 @@ reconnect(secondary); // Let the last committed op from the original 1 node config become committed in the current config. restartServerReplication(secondary); rst.awaitReplication(); +assert.soon(() => isConfigCommitted(primary)); // Now that we can commit the op in the new config, reconfig should succeed. assert.commandWorked(primary.adminCommand({replSetReconfig: twoNodeConfig})); - +assert(isConfigCommitted(primary)); rst.awaitReplication(); rst.stopSet(); diff --git a/jstests/replsets/rslib.js b/jstests/replsets/rslib.js index 0182c3e5574..ce5fad25496 100644 --- a/jstests/replsets/rslib.js +++ b/jstests/replsets/rslib.js @@ -17,6 +17,7 @@ var setLogVerbosity; var stopReplicationAndEnforceNewPrimaryToCatchUp; var setFailPoint; var clearFailPoint; +var isConfigCommitted; (function() { "use strict"; @@ -510,4 +511,13 @@ clearFailPoint = function(node, failpoint) { jsTestLog("Clearing fail point " + failpoint); assert.commandWorked(node.adminCommand({configureFailPoint: failpoint, mode: "off"})); }; + +/** + * Returns the replSetGetConfig field 'commitmentStatus', which is true or false. + */ +isConfigCommitted = function(node) { + let adminDB = node.getDB('admin'); + return assert.commandWorked(adminDB.runCommand({replSetGetConfig: 1, commitmentStatus: true})) + .commitmentStatus; +}; }()); diff --git a/src/mongo/db/repl/repl_set_commands.cpp b/src/mongo/db/repl/repl_set_commands.cpp index d9557fd599b..ff49418920a 100644 --- a/src/mongo/db/repl/repl_set_commands.cpp +++ b/src/mongo/db/repl/repl_set_commands.cpp @@ -196,7 +196,10 @@ public: Status status = ReplicationCoordinator::get(opCtx)->checkReplEnabledForCommand(&result); uassertStatusOK(status); - ReplicationCoordinator::get(opCtx)->processReplSetGetConfig(&result); + bool wantCommitmentStatus; + uassertStatusOK(bsonExtractBooleanFieldWithDefault( + cmdObj, "commitmentStatus", false, &wantCommitmentStatus)); + ReplicationCoordinator::get(opCtx)->processReplSetGetConfig(&result, wantCommitmentStatus); return true; } diff --git a/src/mongo/db/repl/replication_coordinator.h b/src/mongo/db/repl/replication_coordinator.h index d1b3b60644e..e10b9cf4d33 100644 --- a/src/mongo/db/repl/replication_coordinator.h +++ b/src/mongo/db/repl/replication_coordinator.h @@ -605,8 +605,11 @@ public: /** * Handles an incoming replSetGetConfig command. Adds BSON to 'result'. + * + * If commitmentStatus is true, adds a boolean 'commitmentStatus' field to 'result' indicating + * whether the current config is committed. */ - virtual void processReplSetGetConfig(BSONObjBuilder* result) = 0; + virtual void processReplSetGetConfig(BSONObjBuilder* result, bool commitmentStatus = false) = 0; /** * Processes the ReplSetMetadata returned from a command run against another diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index 2531d08a5df..1f460a3fdbc 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -2729,9 +2729,30 @@ ReplSetConfig ReplicationCoordinatorImpl::getConfig() const { return _rsConfig; } -void ReplicationCoordinatorImpl::processReplSetGetConfig(BSONObjBuilder* result) { +void ReplicationCoordinatorImpl::processReplSetGetConfig(BSONObjBuilder* result, + bool commitmentStatus) { stdx::lock_guard<Latch> lock(_mutex); result->append("config", _rsConfig.toBSON()); + + if (commitmentStatus) { + WriteConcernOptions configWriteConcern(ReplSetConfig::kConfigMajorityWriteConcernModeName, + WriteConcernOptions::SyncMode::NONE, + WriteConcernOptions::kNoTimeout); + configWriteConcern.checkCondition = WriteConcernOptions::CheckCondition::Config; + + auto configOplogCommitmentOpTime = _topCoord->getConfigOplogCommitmentOpTime(); + auto oplogWriteConcern = _populateUnsetWriteConcernOptionsSyncMode( + lock, + WriteConcernOptions(_rsConfig.getWriteMajority(), + WriteConcernOptions::SyncMode::NONE, + WriteConcernOptions::kNoTimeout)); + + // OpTime isn't used when checking for config replication. + OpTime ignored; + auto committed = _doneWaitingForReplication_inlock(ignored, configWriteConcern) && + _doneWaitingForReplication_inlock(configOplogCommitmentOpTime, oplogWriteConcern); + result->append("commitmentStatus", committed); + } } void ReplicationCoordinatorImpl::processReplSetMetadata(const rpc::ReplSetMetadata& replMetadata) { diff --git a/src/mongo/db/repl/replication_coordinator_impl.h b/src/mongo/db/repl/replication_coordinator_impl.h index 562417d99d9..a45bc225ff0 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.h +++ b/src/mongo/db/repl/replication_coordinator_impl.h @@ -218,7 +218,8 @@ public: virtual ReplSetConfig getConfig() const override; - virtual void processReplSetGetConfig(BSONObjBuilder* result) override; + virtual void processReplSetGetConfig(BSONObjBuilder* result, + bool commitmentStatus = false) override; virtual void processReplSetMetadata(const rpc::ReplSetMetadata& replMetadata) override; diff --git a/src/mongo/db/repl/replication_coordinator_mock.cpp b/src/mongo/db/repl/replication_coordinator_mock.cpp index 10189e95718..f6983bcfe68 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.cpp +++ b/src/mongo/db/repl/replication_coordinator_mock.cpp @@ -332,7 +332,8 @@ void ReplicationCoordinatorMock::setGetConfigReturnValue(ReplSetConfig returnVal _getConfigReturnValue = std::move(returnValue); } -void ReplicationCoordinatorMock::processReplSetGetConfig(BSONObjBuilder* result) { +void ReplicationCoordinatorMock::processReplSetGetConfig(BSONObjBuilder* result, + bool commitmentStatus) { // TODO } diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h index 57f60ece3cd..ddecc044a53 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.h +++ b/src/mongo/db/repl/replication_coordinator_mock.h @@ -186,7 +186,7 @@ public: virtual ReplSetConfig getConfig() const; - virtual void processReplSetGetConfig(BSONObjBuilder* result); + virtual void processReplSetGetConfig(BSONObjBuilder* result, bool commitmentStatus = false); virtual void processReplSetMetadata(const rpc::ReplSetMetadata& replMetadata) override; diff --git a/src/mongo/db/repl/replication_coordinator_noop.cpp b/src/mongo/db/repl/replication_coordinator_noop.cpp index 1605c42b131..0cdebd3464e 100644 --- a/src/mongo/db/repl/replication_coordinator_noop.cpp +++ b/src/mongo/db/repl/replication_coordinator_noop.cpp @@ -289,7 +289,8 @@ ReplSetConfig ReplicationCoordinatorNoOp::getConfig() const { MONGO_UNREACHABLE; } -void ReplicationCoordinatorNoOp::processReplSetGetConfig(BSONObjBuilder*) { +void ReplicationCoordinatorNoOp::processReplSetGetConfig(BSONObjBuilder* result, + bool commitmentStatus) { MONGO_UNREACHABLE; } diff --git a/src/mongo/db/repl/replication_coordinator_noop.h b/src/mongo/db/repl/replication_coordinator_noop.h index 3bc608f6b54..329d5b5fe14 100644 --- a/src/mongo/db/repl/replication_coordinator_noop.h +++ b/src/mongo/db/repl/replication_coordinator_noop.h @@ -164,7 +164,7 @@ public: ReplSetConfig getConfig() const final; - void processReplSetGetConfig(BSONObjBuilder*) final; + void processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus = false) final; void processReplSetMetadata(const rpc::ReplSetMetadata&) final; diff --git a/src/mongo/embedded/replication_coordinator_embedded.cpp b/src/mongo/embedded/replication_coordinator_embedded.cpp index 0238e7ea547..0d7ddec2605 100644 --- a/src/mongo/embedded/replication_coordinator_embedded.cpp +++ b/src/mongo/embedded/replication_coordinator_embedded.cpp @@ -314,7 +314,8 @@ ReplSetConfig ReplicationCoordinatorEmbedded::getConfig() const { UASSERT_NOT_IMPLEMENTED; } -void ReplicationCoordinatorEmbedded::processReplSetGetConfig(BSONObjBuilder*) { +void ReplicationCoordinatorEmbedded::processReplSetGetConfig(BSONObjBuilder*, + bool commitmentStatus) { UASSERT_NOT_IMPLEMENTED; } diff --git a/src/mongo/embedded/replication_coordinator_embedded.h b/src/mongo/embedded/replication_coordinator_embedded.h index 9bff367083c..313a5a46b0c 100644 --- a/src/mongo/embedded/replication_coordinator_embedded.h +++ b/src/mongo/embedded/replication_coordinator_embedded.h @@ -171,7 +171,7 @@ public: repl::ReplSetConfig getConfig() const override; - void processReplSetGetConfig(BSONObjBuilder*) override; + void processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus = false) override; void processReplSetMetadata(const rpc::ReplSetMetadata&) override; |