SERVER-46966 Handle implicit collection creation errors during insert

5 files changed, 94 insertions, 22 deletions

diff --git a/jstests/core/txns/create_collection.js b/jstests/core/txns/create_collection.js
index b2a1f99df2e..7490fdf1606 100644
--- a/jstests/core/txns/create_collection.js
+++ b/jstests/core/txns/create_collection.js
@@ -10,6 +10,7 @@
 "use strict";
 
 load("jstests/libs/create_collection_txn_helpers.js");
+load("jstests/libs/fixture_helpers.js");  // for isMongos
 
 function runCollectionCreateTest(explicitCreate, upsert) {
     const session = db.getMongo().startSession();
@@ -79,6 +80,24 @@ function runCollectionCreateTest(explicitCreate, upsert) {
     assert.eq(sessionColl.find({}).itcount(), 0);
     assert.eq(secondSessionColl.find({}).itcount(), 0);
 
+    sessionColl.drop({writeConcern: {w: "majority"}});
+
+    // mongos does not support throwWCEDuringTxnCollCreate
+    if (!FixtureHelpers.isMongos(db)) {
+        jsTest.log(
+            "Testing createCollection with writeConflict errors in a transaction (SHOULD ABORT");
+        assert.commandWorked(
+            db.adminCommand({configureFailPoint: "throwWCEDuringTxnCollCreate", mode: "alwaysOn"}));
+        session.startTransaction({writeConcern: {w: "majority"}});
+        assertCollCreateFailedWithCode(
+            sessionDB, collName, explicitCreate, upsert, ErrorCodes.WriteConflict);
+        assert.commandFailedWithCode(session.abortTransaction_forTesting(),
+                                     ErrorCodes.NoSuchTransaction);
+        assert.eq(sessionColl.find({}).itcount(), 0);
+        assert.commandWorked(
+            db.adminCommand({configureFailPoint: "throwWCEDuringTxnCollCreate", mode: "off"}));
+    }
+
     session.endSession();
 }
 
diff --git a/jstests/libs/create_collection_txn_helpers.js b/jstests/libs/create_collection_txn_helpers.js
index 08d7093a5c7..3b113ebd5f8 100644
--- a/jstests/libs/create_collection_txn_helpers.js
+++ b/jstests/libs/create_collection_txn_helpers.js
@@ -25,3 +25,24 @@ const createCollAndCRUDInTxn = function(sessionDB, collName, explicitCreate, ups
     assert.commandWorked(sessionColl.deleteOne({_id: 2}));
     assert.eq(sessionColl.find({}).itcount(), 1);
 };
+
+const assertCollCreateFailedWithCode = function(sessionDB, collName, explicitCreate, upsert, code) {
+    if (undefined === explicitCreate) {
+        doassert('assertWriteConflictForCollCreate called with undefined explicitCreate');
+    }
+    if (undefined === upsert) {
+        doassert('assertWriteConflictForCollCreate called with undefined upsert');
+    }
+    if (undefined === code) {
+        doassert('assertWriteConflictForCollCreate called with undefined code');
+    }
+    let sessionColl = sessionDB[collName];
+    if (explicitCreate) {
+        assert.commandFailedWithCode(sessionDB.createCollection(collName), code);
+    } else if (upsert) {
+        assert.commandFailedWithCode(sessionColl.update({_id: 1}, {$inc: {a: 1}}, {upsert: true}),
+                                     code);
+    } else {
+        assert.commandFailedWithCode(sessionColl.insert({a: 1}), code);
+    }
+};
diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index 5ea7746c8ce..3339be94676 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -82,6 +82,7 @@
 namespace mongo {
 
 namespace {
+MONGO_FAIL_POINT_DEFINE(throwWCEDuringTxnCollCreate);
 MONGO_FAIL_POINT_DEFINE(hangBeforeLoggingCreateCollection);
 MONGO_FAIL_POINT_DEFINE(hangAndFailAfterCreateCollectionReservesOpTime);
 MONGO_FAIL_POINT_DEFINE(openCreateCollectionWindowFp);
@@ -573,6 +574,14 @@ void DatabaseImpl::_checkCanCreateCollection(OperationContext* opCtx,
         }
     }
 
+    if (MONGO_unlikely(throwWCEDuringTxnCollCreate.shouldFail()) &&
+        opCtx->inMultiDocumentTransaction()) {
+        LOGV2(4696600,
+              "Throwing WriteConflictException due to failpoint 'throwWCEDuringTxnCollCreate'");
+        throw WriteConflictException();
+    }
+
+
     uassert(17320,
             str::stream() << "cannot do createCollection on namespace with a $ in it: " << nss,
             nss.ns().find('$') == std::string::npos);
diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp
index cf08d7497d5..cd499617d1f 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -432,31 +432,51 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx,
             &hangWithLockDuringBatchInsert, opCtx, "hangWithLockDuringBatchInsert");
     };
 
+    auto txnParticipant = TransactionParticipant::get(opCtx);
+    auto inTxn = txnParticipant && opCtx->inMultiDocumentTransaction();
+    bool shouldProceedWithBatchInsert = true;
+
     try {
         acquireCollection();
-        auto txnParticipant = TransactionParticipant::get(opCtx);
-        auto inTxn = txnParticipant && opCtx->inMultiDocumentTransaction();
-        if (!collection->getCollection()->isCapped() && !inTxn && batch.size() > 1) {
-            // First try doing it all together. If all goes well, this is all we need to do.
-            // See Collection::_insertDocuments for why we do all capped inserts one-at-a-time.
-            lastOpFixer->startingOp();
-            insertDocuments(
-                opCtx, collection->getCollection(), batch.begin(), batch.end(), fromMigrate);
-            lastOpFixer->finishedOpSuccessfully();
-            globalOpCounters.gotInserts(batch.size());
-            ServerWriteConcernMetrics::get(opCtx)->recordWriteConcernForInserts(
-                opCtx->getWriteConcern(), batch.size());
-            SingleWriteResult result;
-            result.setN(1);
-
-            std::fill_n(std::back_inserter(out->results), batch.size(), std::move(result));
-            curOp.debug().additiveMetrics.incrementNinserted(batch.size());
-            return true;
-        }
-    } catch (const DBException&) {
-        // Ignore this failure and behave as if we never tried to do the combined batch
-        // insert. The loop below will handle reporting any non-transient errors.
+    } catch (const DBException& ex) {
         collection.reset();
+        if (inTxn) {
+            // It is not safe to ignore errors from collection creation while inside a
+            // multi-document transaction.
+            auto canContinue =
+                handleError(opCtx, ex, wholeOp.getNamespace(), wholeOp.getWriteCommandBase(), out);
+            invariant(!canContinue);
+            return false;
+        }
+        // Otherwise, proceed as though the batch insert block failed, since the batch insert block
+        // assumes `acquireCollection` is successful.
+        shouldProceedWithBatchInsert = false;
+    }
+
+    if (shouldProceedWithBatchInsert) {
+        try {
+            if (!collection->getCollection()->isCapped() && !inTxn && batch.size() > 1) {
+                // First try doing it all together. If all goes well, this is all we need to do.
+                // See Collection::_insertDocuments for why we do all capped inserts one-at-a-time.
+                lastOpFixer->startingOp();
+                insertDocuments(
+                    opCtx, collection->getCollection(), batch.begin(), batch.end(), fromMigrate);
+                lastOpFixer->finishedOpSuccessfully();
+                globalOpCounters.gotInserts(batch.size());
+                ServerWriteConcernMetrics::get(opCtx)->recordWriteConcernForInserts(
+                    opCtx->getWriteConcern(), batch.size());
+                SingleWriteResult result;
+                result.setN(1);
+
+                std::fill_n(std::back_inserter(out->results), batch.size(), std::move(result));
+                curOp.debug().additiveMetrics.incrementNinserted(batch.size());
+                return true;
+            }
+        } catch (const DBException&) {
+            // Ignore this failure and behave as if we never tried to do the combined batch
+            // insert. The loop below will handle reporting any non-transient errors.
+            collection.reset();
+        }
     }
 
     // Try to insert the batch one-at-a-time. This path is executed for singular batches,
diff --git a/src/mongo/db/storage/write_unit_of_work.cpp b/src/mongo/db/storage/write_unit_of_work.cpp
index aaa7c9fcfb3..bdbda47471a 100644
--- a/src/mongo/db/storage/write_unit_of_work.cpp
+++ b/src/mongo/db/storage/write_unit_of_work.cpp
@@ -53,6 +53,9 @@ WriteUnitOfWork::WriteUnitOfWork(OperationContext* opCtx)
         _opCtx->recoveryUnit()->beginUnitOfWork(_opCtx);
         _opCtx->_ruState = RecoveryUnitState::kActiveUnitOfWork;
     }
+    // Make sure we don't silently proceed after a previous WriteUnitOfWork under the same parent
+    // WriteUnitOfWork fails.
+    invariant(_opCtx->_ruState != RecoveryUnitState::kFailedUnitOfWork);
 }
 
 WriteUnitOfWork::~WriteUnitOfWork() {