SERVER-34496 Await all operations committed in ShardingTest::checkUUIDsConsistent hook

6 files changed, 63 insertions, 20 deletions

diff --git a/jstests/libs/override_methods/check_uuids_consistent_across_cluster.js b/jstests/libs/override_methods/check_uuids_consistent_across_cluster.js
index 4fbc9add333..0a05e19b25c 100644
--- a/jstests/libs/override_methods/check_uuids_consistent_across_cluster.js
+++ b/jstests/libs/override_methods/check_uuids_consistent_across_cluster.js
@@ -38,6 +38,31 @@ ShardingTest.prototype.checkUUIDsConsistentAcrossCluster = function() {
             "Checking consistency of the sharding catalog with shards' storage catalogs and catalog caches");
     }
 
+    this.awaitReplicationOnShards = function() {
+        var timeout = 1 * 60 * 1000;
+        for (var i = 0; i < this._rs.length; i++) {
+            // If this shard is standalone, the replica set object will be null. In that case, we
+            // will just skip.
+            if (!this._rs[i]) {
+                continue;
+            }
+            var rs = this._rs[i].test;
+            // The noop writer needs to be enabled in case a sync source isn't set, so that
+            // awaitLastOpCommitted() is guaranteed to finish.
+            // SERVER-33248 for reference.
+            rs.getPrimary().adminCommand({setParameter: 1, periodicNoopIntervalSecs: 1});
+            rs.getPrimary().adminCommand({setParameter: 1, writePeriodicNoops: true});
+            var keyFile = this._otherParams.keyFile;
+            if (keyFile) {
+                authutil.asCluster(rs.nodes, keyFile, function() {
+                    rs.awaitLastOpCommitted(timeout);
+                });
+            } else {
+                rs.awaitLastOpCommitted(timeout);
+            }
+        }
+    };
+
     function parseNs(dbDotColl) {
         assert.gt(dbDotColl.indexOf('.'),
                   0,
@@ -89,6 +114,11 @@ ShardingTest.prototype.checkUUIDsConsistentAcrossCluster = function() {
             shardConnStringToConn[conn.host] = conn;
         });
 
+        if (!jsTest.options().skipAwaitingReplicationOnShardsBeforeCheckingUUIDs) {
+            // Finish replication on all shards (if they are replica sets).
+            this.awaitReplicationOnShards();
+        }
+
         for (let authoritativeCollMetadata of authoritativeCollMetadataArr) {
             const ns = authoritativeCollMetadata._id;
             const[dbName, collName] = parseNs(ns);
diff --git a/jstests/sharding/aggregation_currentop.js b/jstests/sharding/aggregation_currentop.js
index dce4cf5482f..fb793441cec 100644
--- a/jstests/sharding/aggregation_currentop.js
+++ b/jstests/sharding/aggregation_currentop.js
@@ -15,6 +15,10 @@
  * This test requires replica set configuration and user credentials to persist across a restart.
  * @tags: [requires_persistence]
  */
+
+// Restarts cause issues with authentication for awaiting replication.
+TestData.skipAwaitingReplicationOnShardsBeforeCheckingUUIDs = true;
+
 (function() {
     "use strict";
 
diff --git a/jstests/sharding/linearizable_read_concern.js b/jstests/sharding/linearizable_read_concern.js
index 7f08a07b568..637566e1b2d 100644
--- a/jstests/sharding/linearizable_read_concern.js
+++ b/jstests/sharding/linearizable_read_concern.js
@@ -27,8 +27,10 @@ load("jstests/libs/write_concern_util.js");
 (function() {
     "use strict";
 
-    // Skip db hash check since this test leaves a replica set shard partitioned.
+    // Skip db hash check and shard replication since this test leaves a replica set shard
+    // partitioned.
     TestData.skipCheckDBHashes = true;
+    TestData.skipAwaitingReplicationOnShardsBeforeCheckingUUIDs = true;
 
     var testName = "linearizable_read_concern";
 
diff --git a/jstests/sharding/repl_monitor_refresh.js b/jstests/sharding/repl_monitor_refresh.js
index 3d799dfbbcc..b3d91d04065 100644
--- a/jstests/sharding/repl_monitor_refresh.js
+++ b/jstests/sharding/repl_monitor_refresh.js
@@ -7,8 +7,10 @@ load("jstests/replsets/rslib.js");
 (function() {
     "use strict";
 
-    // Skip db hash check since the removed node has wrong config and is still alive.
+    // Skip db hash check and shard replication since the removed node has wrong config and is still
+    // alive.
     TestData.skipCheckDBHashes = true;
+    TestData.skipAwaitingReplicationOnShardsBeforeCheckingUUIDs = true;
 
     var NODE_COUNT = 3;
     var st = new ShardingTest({shards: {rs0: {nodes: NODE_COUNT, oplogSize: 10}}});
diff --git a/src/mongo/shell/replsettest.js b/src/mongo/shell/replsettest.js
index 7c200283189..499715ab18e 100644
--- a/src/mongo/shell/replsettest.js
+++ b/src/mongo/shell/replsettest.js
@@ -1094,7 +1094,7 @@ var ReplSetTest = function(opts) {
      * of the oplog on *all* secondaries.
      * Returns last oplog entry.
      */
-    this.awaitLastOpCommitted = function() {
+    this.awaitLastOpCommitted = function(timeout) {
         var rst = this;
         var master = rst.getPrimary();
         var masterOpTime = _getLastOpTime(master);
@@ -1102,26 +1102,29 @@ var ReplSetTest = function(opts) {
         print("Waiting for op with OpTime " + tojson(masterOpTime) +
               " to be committed on all secondaries");
 
-        assert.soonNoExcept(function() {
-            for (var i = 0; i < rst.nodes.length; i++) {
-                var node = rst.nodes[i];
+        assert.soonNoExcept(
+            function() {
+                for (var i = 0; i < rst.nodes.length; i++) {
+                    var node = rst.nodes[i];
 
-                // Continue if we're connected to an arbiter
-                var res = assert.commandWorked(node.adminCommand({replSetGetStatus: 1}));
-                if (res.myState == ReplSetTest.State.ARBITER) {
-                    continue;
-                }
-                var rcmOpTime = _getReadConcernMajorityOpTime(node);
-                if (friendlyEqual(rcmOpTime, {ts: Timestamp(0, 0), t: NumberLong(0)})) {
-                    return false;
-                }
-                if (rs.compareOpTimes(rcmOpTime, masterOpTime) < 0) {
-                    return false;
+                    // Continue if we're connected to an arbiter
+                    var res = assert.commandWorked(node.adminCommand({replSetGetStatus: 1}));
+                    if (res.myState == ReplSetTest.State.ARBITER) {
+                        continue;
+                    }
+                    var rcmOpTime = _getReadConcernMajorityOpTime(node);
+                    if (friendlyEqual(rcmOpTime, {ts: Timestamp(0, 0), t: NumberLong(0)})) {
+                        return false;
+                    }
+                    if (rs.compareOpTimes(rcmOpTime, masterOpTime) < 0) {
+                        return false;
+                    }
                 }
-            }
 
-            return true;
-        }, "Op with OpTime " + tojson(masterOpTime) + " failed to be committed on all secondaries");
+                return true;
+            },
+            "Op with OpTime " + tojson(masterOpTime) + " failed to be committed on all secondaries",
+            timeout);
 
         return masterOpTime;
     };
diff --git a/src/mongo/shell/utils.js b/src/mongo/shell/utils.js
index 354162529c5..07cc666a15a 100644
--- a/src/mongo/shell/utils.js
+++ b/src/mongo/shell/utils.js
@@ -294,6 +294,8 @@ jsTestOptions = function() {
                 TestData.skipCheckingUUIDsConsistentAcrossCluster || false,
             skipCheckingCatalogCacheConsistencyWithShardingCatalog:
                 TestData.skipCheckingCatalogCacheConsistencyWithShardingCatalog || false,
+            skipAwaitingReplicationOnShardsBeforeCheckingUUIDs:
+                TestData.skipAwaitingReplicationOnShardsBeforeCheckingUUIDs || false,
             jsonSchemaTestFile: TestData.jsonSchemaTestFile,
             excludedDBsFromDBHash: TestData.excludedDBsFromDBHash,
             alwaysInjectTransactionNumber: TestData.alwaysInjectTransactionNumber,