SERVER-46253 Roll back collection registrations during txn abort

4 files changed, 57 insertions, 2 deletions

diff --git a/jstests/core/txns/create_collection_parallel.js b/jstests/core/txns/create_collection_parallel.js
index d0a06df9d9b..3c8afddf811 100644
--- a/jstests/core/txns/create_collection_parallel.js
+++ b/jstests/core/txns/create_collection_parallel.js
@@ -61,6 +61,29 @@ function runParallelCollectionCreateTest(explicitCreate) {
     sessionColl.drop({writeConcern: {w: "majority"}});
     distinctSessionColl.drop({writeConcern: {w: "majority"}});
 
+    /* TODO(SERVER-46285) Re-enable below test.
+    jsTest.log(
+        "Testing createCollection conflict during commit, where the conflict rolls back a previously
+    committed collection.");
+
+    secondSession.startTransaction({writeConcern: {w: "majority"}});  // txn 2
+    createCollAndCRUDInTxn(secondSession.getDatabase("test"), collName, explicitCreate);
+
+    session.startTransaction({writeConcern: {w: "majority"}});            // txn 1
+    createCollAndCRUDInTxn(sessionDB, distinctCollName, explicitCreate);  // does not conflict
+    createCollAndCRUDInTxn(sessionDB, collName, explicitCreate);          // conflicts
+
+    jsTest.log("Committing transaction 2");
+    secondSession.commitTransaction();
+    jsTest.log("Committing transaction 1 (SHOULD FAIL)");
+    assert.commandFailedWithCode(session.commitTransaction_forTesting(), ErrorCodes.WriteConflict);
+    assert.eq(sessionColl.find({}).itcount(), 1);
+    assert.eq(distinctSessionColl.find({}).itcount(), 0);
+
+    sessionColl.drop({writeConcern: {w: "majority"}});
+    distinctSessionColl.drop({writeConcern: {w: "majority"}});
+    */
+
     jsTest.log("Testing distinct createCollections in parallel, both successfully commit.");
     session.startTransaction({writeConcern: {w: "majority"}});  // txn 1
     createCollAndCRUDInTxn(sessionDB, collName, explicitCreate);
diff --git a/src/mongo/db/catalog/collection_catalog.h b/src/mongo/db/catalog/collection_catalog.h
index 1f2c98de80d..85ff4a8daad 100644
--- a/src/mongo/db/catalog/collection_catalog.h
+++ b/src/mongo/db/catalog/collection_catalog.h
@@ -254,6 +254,7 @@ private:
 
     const std::vector<CollectionUUID>& _getOrdering_inlock(const StringData& db,
                                                            const stdx::lock_guard<Latch>&);
+
     mutable mongo::Mutex _catalogLock;
 
     /**
diff --git a/src/mongo/db/catalog/uncommitted_collections.cpp b/src/mongo/db/catalog/uncommitted_collections.cpp
index 853a5afecac..34b367c1912 100644
--- a/src/mongo/db/catalog/uncommitted_collections.cpp
+++ b/src/mongo/db/catalog/uncommitted_collections.cpp
@@ -63,6 +63,12 @@ void UncommittedCollections::addToTxn(OperationContext* opCtx,
 
     auto collListUnowned = getUncommittedCollections(opCtx).getResources();
 
+    opCtx->recoveryUnit()->onRollback([collListUnowned, uuid, nss]() {
+        UncommittedCollections::erase(uuid, nss, collListUnowned.lock().get());
+    });
+
+    auto svcCtx = opCtx->getServiceContext();
+
     opCtx->recoveryUnit()->registerPreCommitHook(
         [collListUnowned, uuid, createTime](OperationContext* opCtx) {
             UncommittedCollections::commit(opCtx, uuid, createTime, collListUnowned.lock().get());
@@ -74,8 +80,8 @@ void UncommittedCollections::addToTxn(OperationContext* opCtx,
             invariant(collPtr->getMinimumVisibleSnapshot() == createTime);
             UncommittedCollections::clear(collListUnowned.lock().get());
         });
-    opCtx->recoveryUnit()->onRollback([collListUnowned, uuid, nss]() {
-        UncommittedCollections::erase(uuid, nss, collListUnowned.lock().get());
+    opCtx->recoveryUnit()->onRollback([svcCtx, collListUnowned, uuid, nss]() {
+        UncommittedCollections::rollback(svcCtx, uuid, collListUnowned.lock().get());
     });
 }
 
@@ -112,6 +118,18 @@ void UncommittedCollections::erase(UUID uuid, NamespaceString nss, UncommittedCo
     map->erase(uuid, nss);
 }
 
+void UncommittedCollections::rollback(ServiceContext* svcCtx,
+                                      CollectionUUID uuid,
+                                      UncommittedCollectionsMap* map) {
+    auto it = std::find(map->_registeredUUIDs.begin(), map->_registeredUUIDs.end(), uuid);
+    if (it != map->_registeredUUIDs.end()) {
+        auto collPtr = CollectionCatalog::get(svcCtx).deregisterCollection(uuid);
+        auto nss = collPtr.get()->ns();
+        map->_collections[uuid] = std::move(collPtr);
+        map->_nssIndex.insert({nss, uuid});
+    }
+}
+
 void UncommittedCollections::commit(OperationContext* opCtx,
                                     UUID uuid,
                                     Timestamp createTs,
@@ -129,6 +147,7 @@ void UncommittedCollections::commit(OperationContext* opCtx,
     CollectionCatalog::get(opCtx).registerCollection(uuid, &(it->second));
     map->_collections.erase(it);
     map->_nssIndex.erase(nss);
+    map->_registeredUUIDs.push_back(uuid);
 }
 
 bool UncommittedCollections::isUncommittedCollection(OperationContext* opCtx,
diff --git a/src/mongo/db/catalog/uncommitted_collections.h b/src/mongo/db/catalog/uncommitted_collections.h
index 5fe97371648..fa2025354aa 100644
--- a/src/mongo/db/catalog/uncommitted_collections.h
+++ b/src/mongo/db/catalog/uncommitted_collections.h
@@ -55,8 +55,10 @@ public:
             _collections.erase(uuid);
             _nssIndex.erase(nss);
         }
+
         std::map<UUID, std::unique_ptr<Collection>> _collections;
         std::map<NamespaceString, UUID> _nssIndex;
+        std::vector<UUID> _registeredUUIDs;
     };
 
     UncommittedCollections() {
@@ -97,6 +99,15 @@ public:
                        UncommittedCollectionsMap* map);
 
     /**
+     * If the collection with uuid `uuid` was previously registered with the CollectionCatalog as
+     * part of the current multi-document transaction, this handler deregisters it.
+     */
+    static void rollback(ServiceContext* svcCtx,
+                         CollectionUUID uuid,
+                         UncommittedCollectionsMap* map);
+
+
+    /**
      * Erases the UUID/NamespaceString entries corresponding to `uuid` and `nss` from `map`.
      */
     static void erase(UUID uuid, NamespaceString nss, UncommittedCollectionsMap* map);
@@ -109,6 +120,7 @@ public:
     static void clear(UncommittedCollectionsMap* map) {
         map->_collections.clear();
         map->_nssIndex.clear();
+        map->_registeredUUIDs.clear();
     }
 
 private: