SERVER-42300 Retry, not skip, renamed ns in forEachCollectionFromDb

2 files changed, 38 insertions, 14 deletions

diff --git a/jstests/concurrency/fsm_workloads/rename_collection_chain.js b/jstests/concurrency/fsm_workloads/rename_collection_chain.js
index cd6a541c170..cf2f6f49424 100644
--- a/jstests/concurrency/fsm_workloads/rename_collection_chain.js
+++ b/jstests/concurrency/fsm_workloads/rename_collection_chain.js
@@ -9,7 +9,6 @@
  */
 
 var $config = (function() {
-
     var data = {
         // Use the workload name as a prefix for the collection name,
         // since the workload name is assumed to be unique.
@@ -17,7 +16,6 @@ var $config = (function() {
     };
 
     var states = (function() {
-
         function uniqueCollectionName(prefix, tid, num) {
             return prefix + tid + '_' + num;
         }
@@ -35,11 +33,30 @@ var $config = (function() {
             this.fromCollName = toCollName;
         }
 
-        return {init: init, rename: rename};
+        function listCollections(db, collName) {
+            const collectionInfos = db.getCollectionInfos();
+            if (!this.allCollectionsInitialized) {
+                if (collectionInfos.length === this.threadCount) {
+                    this.allCollectionsInitialized = true;
+                    jsTestLog(`All collections visible to thread ${this.tid}: ${
+                        tojsononeline(collectionInfos)}`);
+                }
+            } else {
+                const numColls =
+                    collectionInfos.filter((collInfo) => collInfo.name.startsWith(this.prefix))
+                        .length;
+                assertAlways.eq(numColls, this.threadCount, () => tojson(collectionInfos));
+            }
+        }
 
+        return {init: init, rename: rename, listCollections: listCollections};
     })();
 
-    var transitions = {init: {rename: 1}, rename: {rename: 1}};
+    var transitions = {
+        init: {rename: 1},
+        rename: {rename: 0.9, listCollections: 0.1},
+        listCollections: {rename: 1},
+    };
 
     return {
         threadCount: 10,
@@ -48,5 +65,4 @@ var $config = (function() {
         states: states,
         transitions: transitions,
     };
-
 })();
diff --git a/src/mongo/db/catalog/collection_catalog_helper.cpp b/src/mongo/db/catalog/collection_catalog_helper.cpp
index 2cb11844d71..620a9a2a08a 100644
--- a/src/mongo/db/catalog/collection_catalog_helper.cpp
+++ b/src/mongo/db/catalog/collection_catalog_helper.cpp
@@ -51,18 +51,26 @@ void forEachCollectionFromDb(OperationContext* opCtx,
             continue;
         }
 
-        auto nss = catalog.lookupNSSByUUID(uuid);
+        boost::optional<Lock::CollectionLock> clk;
+        Collection* collection = nullptr;
 
-        // If the NamespaceString can't be resolved from the uuid, then the collection was dropped.
-        if (!nss) {
-            continue;
-        }
+        while (auto nss = catalog.lookupNSSByUUID(uuid)) {
+            // Get a fresh snapshot for each locked collection to see any catalog changes.
+            clk.emplace(opCtx, *nss, collLockMode);
+            opCtx->recoveryUnit()->abandonSnapshot();
 
-        Lock::CollectionLock clk(opCtx, *nss, collLockMode);
-        opCtx->recoveryUnit()->abandonSnapshot();
+            if (catalog.lookupNSSByUUID(uuid) == nss) {
+                // Success: locked the namespace and the UUID still maps to it.
+                collection = catalog.lookupCollectionByUUID(uuid);
+                invariant(collection);
+                break;
+            }
+            // Failed: collection got renamed before locking it, so unlock and try again.
+            clk.reset();
+        }
 
-        auto collection = catalog.lookupCollectionByUUID(uuid);
-        if (!collection || collection->ns() != *nss)
+        // The NamespaceString couldn't be resolved from the uuid, so the collection was dropped.
+        if (!collection)
             continue;
 
         if (!callback(collection))