SERVER-41640 forEachCollectionFromDb() should call abandonSnapshot() before running the user provided callback

2 files changed, 54 insertions, 0 deletions

diff --git a/jstests/noPassthrough/abandon_snapshot_for_each_collection_from_db.js b/jstests/noPassthrough/abandon_snapshot_for_each_collection_from_db.js
new file mode 100644
index 00000000000..432ee93eaa7
--- /dev/null
+++ b/jstests/noPassthrough/abandon_snapshot_for_each_collection_from_db.js
@@ -0,0 +1,48 @@
+/**
+ * The 'forEachCollectionFromDb' CollectionCatalog helper should abandon its snapshot before it
+ * performs the user provided callback on each collection.
+ * Any newly added collections have the chance to be seen as 'forEachCollectionFromDb' iterates over
+ * the remaining collections. This could lead to inconsistencies, such as seeing indexes in the
+ * IndexCatalog of the new collection but not seeing them on-disk due to using an older snapshot
+ * (BF-13133).
+ *
+ * @tags: [requires_replication]
+ */
+(function() {
+    "use strict";
+
+    const dbName = "test";
+    const collName = "coll";
+
+    const rst = new ReplSetTest({nodes: 2});
+    rst.startSet();
+    rst.initiate();
+
+    const db = rst.getPrimary().getDB(dbName);
+    assert.commandWorked(db.createCollection(collName));
+
+    const failpoint = 'hangBeforeGettingNextCollection';
+
+    // Hang 'forEachCollectionFromDb' after iterating through the first collection.
+    assert.commandWorked(db.adminCommand({configureFailPoint: failpoint, mode: "alwaysOn"}));
+
+    TestData.failpoint = failpoint;
+    const awaitCreateCollections = startParallelShell(() => {
+        // The 'forEachCollectionFromDb' helper doesn't iterate in collection name order, so we need
+        // to insert multiple collections to have at least one next collection when the
+        // CollectionCatalog iterator is incremented.
+        for (let i = 0; i < 25; i++) {
+            const collName = "a".repeat(i + 1);
+            assert.commandWorked(db.createCollection(collName));
+        }
+
+        // Let 'forEachCollectionFromDb' iterate to the next collection.
+        assert.commandWorked(
+            db.adminCommand({configureFailPoint: TestData.failpoint, mode: "off"}));
+    }, rst.getPrimary().port);
+
+    assert.commandWorked(db.stats());
+    awaitCreateCollections();
+
+    rst.stopSet();
+}());
diff --git a/src/mongo/db/catalog/collection_catalog_helper.cpp b/src/mongo/db/catalog/collection_catalog_helper.cpp
index 7719ef531d1..2a10fef2066 100644
--- a/src/mongo/db/catalog/collection_catalog_helper.cpp
+++ b/src/mongo/db/catalog/collection_catalog_helper.cpp
@@ -34,6 +34,9 @@
 #include "mongo/db/concurrency/d_concurrency.h"
 
 namespace mongo {
+
+MONGO_FAIL_POINT_DEFINE(hangBeforeGettingNextCollection);
+
 namespace catalog {
 
 void forEachCollectionFromDb(OperationContext* opCtx,
@@ -57,6 +60,7 @@ void forEachCollectionFromDb(OperationContext* opCtx,
         }
 
         Lock::CollectionLock clk(opCtx, *nss, collLockMode);
+        opCtx->recoveryUnit()->abandonSnapshot();
 
         auto collection = catalog.lookupCollectionByUUID(uuid);
         auto catalogEntry = catalog.lookupCollectionCatalogEntryByUUID(uuid);
@@ -65,6 +69,8 @@ void forEachCollectionFromDb(OperationContext* opCtx,
 
         if (!callback(collection, catalogEntry))
             break;
+
+        MONGO_FAIL_POINT_PAUSE_WHILE_SET(hangBeforeGettingNextCollection);
     }
 }