SERVER-38583 Fix transaction insert writeError format

2 files changed, 29 insertions, 33 deletions

diff --git a/src/mongo/db/handle_request_response.cpp b/src/mongo/db/handle_request_response.cpp
index 3a1e4919636..253a8526ea4 100644
--- a/src/mongo/db/handle_request_response.cpp
+++ b/src/mongo/db/handle_request_response.cpp
@@ -29,6 +29,7 @@
  */
 
 #include "mongo/db/handle_request_response.h"
+#include "mongo/base/transaction_error.h"
 
 namespace mongo {
 
@@ -44,24 +45,12 @@ BSONObj getErrorLabels(const OperationSessionInfoFromClient& sessionOptions,
     }
 
     // The errors that indicate the transaction fails without any persistent side-effect.
-    bool isTransientTransactionError = code == ErrorCodes::WriteConflict  //
-        || code == ErrorCodes::SnapshotUnavailable                        //
-        || code == ErrorCodes::LockTimeout                                //
-        || code == ErrorCodes::PreparedTransactionInProgress;
+    bool isTransient = isTransientTransactionError(
+        code,
+        hasWriteConcernError,
+        commandName == "commitTransaction" || commandName == "coordinateCommitTransaction");
 
-    if (commandName == "commitTransaction" || commandName == "coordinateCommitTransaction") {
-        // NoSuchTransaction is determined based on the data. It's safe to retry the whole
-        // transaction, only if the data cannot be rolled back.
-        isTransientTransactionError |=
-            code == ErrorCodes::NoSuchTransaction && !hasWriteConcernError;
-    } else {
-        bool isRetryable = ErrorCodes::isNotMasterError(code) || ErrorCodes::isShutdownError(code);
-        // For commands other than "commitTransaction", we know there's no side-effect for these
-        // errors, but it's not true for "commitTransaction" if a failover happens.
-        isTransientTransactionError |= isRetryable || code == ErrorCodes::NoSuchTransaction;
-    }
-
-    if (isTransientTransactionError) {
+    if (isTransient) {
         return BSON("errorLabels" << BSON_ARRAY("TransientTransactionError"));
     }
     return {};
diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp
index 1007bb72d3c..5b71430d49f 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -35,6 +35,7 @@
 #include <memory>
 
 #include "mongo/base/checked_cast.h"
+#include "mongo/base/transaction_error.h"
 #include "mongo/db/audit.h"
 #include "mongo/db/auth/authorization_session.h"
 #include "mongo/db/catalog/collection.h"
@@ -222,7 +223,7 @@ void makeCollection(OperationContext* opCtx, const NamespaceString& ns) {
 }
 
 /**
- * Returns true if the operation can continue.
+ * Returns true if the batch can continue, false to stop the batch, or throws to fail the command.
  */
 bool handleError(OperationContext* opCtx,
                  const DBException& ex,
@@ -239,8 +240,16 @@ bool handleError(OperationContext* opCtx,
 
     auto txnParticipant = TransactionParticipant::get(opCtx);
     if (txnParticipant && txnParticipant->inActiveOrKilledMultiDocumentTransaction()) {
+        if (isTransientTransactionError(
+                ex.code(), false /* hasWriteConcernError */, false /* isCommitTransaction */)) {
+            // Tell the client to try the whole txn again, by returning ok: 0 with errorLabels.
+            throw;
+        }
+
         // If we are in a transaction, we must fail the whole batch.
-        throw;
+        out->results.emplace_back(ex.toStatus());
+        txnParticipant->abortActiveTransaction(opCtx);
+        return false;
     }
 
     if (ex.extraInfo<StaleConfigInfo>()) {
@@ -397,7 +406,9 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx,
 
     try {
         acquireCollection();
-        if (!collection->getCollection()->isCapped() && batch.size() > 1) {
+        auto txnParticipant = TransactionParticipant::get(opCtx);
+        auto inTxn = txnParticipant && txnParticipant->inActiveOrKilledMultiDocumentTransaction();
+        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();
@@ -415,20 +426,13 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx,
             return true;
         }
     } catch (const DBException&) {
-
-        // If we cannot abandon the current snapshot, we give up and rethrow the exception.
-        // No WCE retrying is attempted.  This code path is intended for snapshot read concern.
-        if (opCtx->lockState()->inAWriteUnitOfWork()) {
-            throw;
-        }
-
-        // Otherwise, 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.
+        // 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 both for singular batches, and
-    // for batches that failed all-at-once inserting.
+    // Try to insert the batch one-at-a-time. This path is executed for singular batches,
+    // multi-statement transactions, capped collections, and if we failed all-at-once inserting.
     for (auto it = batch.begin(); it != batch.end(); ++it) {
         globalOpCounters.gotInsert();
         ServerWriteConcernMetrics::get(opCtx)->recordWriteConcernForInsert(
@@ -449,7 +453,7 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx,
                     // Release the lock following any error if we are not in multi-statement
                     // transaction. Among other things, this ensures that we don't sleep in the WCE
                     // retry loop with the lock held.
-                    // If we are in multi-statement transaction and under a under a WUOW, we will
+                    // If we are in multi-statement transaction and under a WUOW, we will
                     // not actually release the lock.
                     collection.reset();
                     throw;
@@ -458,8 +462,11 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx,
         } catch (const DBException& ex) {
             bool canContinue =
                 handleError(opCtx, ex, wholeOp.getNamespace(), wholeOp.getWriteCommandBase(), out);
-            if (!canContinue)
+
+            if (!canContinue) {
+                // Failed in ordered batch, or in a transaction, or from some unrecoverable error.
                 return false;
+            }
         }
     }