SERVER-43889 Distinguish between retryable write and transaction when failing a command

5 files changed, 119 insertions, 5 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index c38b750f49e..8bc4ab38061 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -59,6 +59,8 @@ replica_sets_multiversion:
 sharding_multiversion:
 - ticket: SERVER-38691
   test_file: jstests/sharding/explain_cmd.js
+- ticket: SERVER-43889
+  test_file: jstests/sharding/retryable_writes.js
 
 sharding_jscore_multiversion_passthrough:
 
diff --git a/jstests/sharding/retryable_writes.js b/jstests/sharding/retryable_writes.js
index 9da8f40effa..c361052fce2 100644
--- a/jstests/sharding/retryable_writes.js
+++ b/jstests/sharding/retryable_writes.js
@@ -400,6 +400,30 @@ function runFailpointTests(mainConn, priConn) {
     assert.eq(1, collContents[1].y);
 }
 
+function runRetryableWriteErrorTest(mainConn) {
+    // Test TransactionTooOld error message on retryable writes
+    const lsid = UUID();
+    const testDb = mainConn.getDB('TestDB');
+
+    assert.commandWorked(testDb.runCommand({
+        insert: 'user',
+        documents: [{x: 1}],
+        ordered: true,
+        lsid: {id: lsid},
+        txnNumber: NumberLong(2)
+    }));
+    const writeResult = testDb.runCommand({
+        update: 'user',
+        updates: [{q: {x: 1}, u: {$inc: {x: 1}}}],
+        ordered: true,
+        lsid: {id: lsid},
+        txnNumber: NumberLong(1)
+    });
+    assert.commandFailedWithCode(writeResult, ErrorCodes.TransactionTooOld);
+    assert(writeResult.errmsg.includes("Retryable write with txnNumber 1 is prohibited"),
+           writeResult);
+}
+
 function runMultiTests(mainConn) {
     // Test the behavior of retryable writes with multi=true / limit=0
     var lsid = {id: UUID()};
@@ -514,6 +538,7 @@ var priConn = replTest.getPrimary();
 
 runTests(priConn, priConn);
 runFailpointTests(priConn, priConn);
+runRetryableWriteErrorTest(priConn);
 runMultiTests(priConn);
 runInvalidTests(priConn);
 
diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp
index c8180467c20..adf348e6eed 100644
--- a/src/mongo/db/transaction_participant.cpp
+++ b/src/mongo/db/transaction_participant.cpp
@@ -495,11 +495,25 @@ void TransactionParticipant::Participant::beginOrContinue(OperationContext* opCt
                     getTestCommandsEnabled());
     }
 
-    uassert(ErrorCodes::TransactionTooOld,
-            str::stream() << "Cannot start transaction " << txnNumber << " on session "
-                          << _sessionId() << " because a newer transaction " << o().activeTxnNumber
-                          << " has already started.",
-            txnNumber >= o().activeTxnNumber);
+    if (txnNumber < o().activeTxnNumber) {
+        const std::string currOperation =
+            o().txnState.isInRetryableWriteMode() ? "retryable write" : "transaction";
+        if (!autocommit) {
+            uasserted(ErrorCodes::TransactionTooOld,
+                      str::stream()
+                          << "Retryable write with txnNumber " << txnNumber
+                          << " is prohibited on session " << _sessionId() << " because a newer "
+                          << currOperation << " with txnNumber " << o().activeTxnNumber
+                          << " has already started on this session.");
+        } else {
+            uasserted(ErrorCodes::TransactionTooOld,
+                      str::stream() << "Cannot start transaction " << txnNumber << " on session "
+                                    << _sessionId() << " because a newer " << currOperation
+                                    << " with txnNumber " << o().activeTxnNumber
+                                    << " has already started on this session.");
+        }
+    }
+
 
     // Requests without an autocommit field are interpreted as retryable writes. They cannot specify
     // startTransaction, which is verified earlier when parsing the request.
diff --git a/src/mongo/db/transaction_participant_retryable_writes_test.cpp b/src/mongo/db/transaction_participant_retryable_writes_test.cpp
index 6e4d4b096e7..36b0a98ab83 100644
--- a/src/mongo/db/transaction_participant_retryable_writes_test.cpp
+++ b/src/mongo/db/transaction_participant_retryable_writes_test.cpp
@@ -379,6 +379,42 @@ TEST_F(TransactionParticipantRetryableWritesTest, StartingOldTxnShouldAssert) {
     ASSERT(txnParticipant.getLastWriteOpTime().isNull());
 }
 
+TEST_F(TransactionParticipantRetryableWritesTest,
+       OlderRetryableWriteFailsOnSessionWithNewerRetryableWrite) {
+    auto txnParticipant = TransactionParticipant::get(opCtx());
+    txnParticipant.refreshFromStorageIfNeeded(opCtx());
+    const TxnNumber txnNum = 22;
+    const auto& sessionId = *opCtx()->getLogicalSessionId();
+
+    StringBuilder sb;
+    sb << "Retryable write with txnNumber 21 is prohibited on session " << sessionId
+       << " because a newer retryable write with txnNumber 22 has already started on this session.";
+    txnParticipant.beginOrContinue(opCtx(), txnNum, boost::none, boost::none);
+    ASSERT_THROWS_WHAT(
+        txnParticipant.beginOrContinue(opCtx(), txnNum - 1, boost::none, boost::none),
+        AssertionException,
+        sb.str());
+    ASSERT(txnParticipant.getLastWriteOpTime().isNull());
+}
+
+TEST_F(TransactionParticipantRetryableWritesTest,
+       OldTransactionFailsOnSessionWithNewerRetryableWrite) {
+    auto txnParticipant = TransactionParticipant::get(opCtx());
+    txnParticipant.refreshFromStorageIfNeeded(opCtx());
+    const TxnNumber txnNum = 22;
+    auto autocommit = false;
+    const auto& sessionId = *opCtx()->getLogicalSessionId();
+
+    StringBuilder sb;
+    sb << "Cannot start transaction 21 on session " << sessionId
+       << " because a newer retryable write with txnNumber 22 has already started on this session.";
+    txnParticipant.beginOrContinue(opCtx(), txnNum, boost::none, boost::none);
+    ASSERT_THROWS_WHAT(txnParticipant.beginOrContinue(opCtx(), txnNum - 1, autocommit, boost::none),
+                       AssertionException,
+                       sb.str());
+    ASSERT(txnParticipant.getLastWriteOpTime().isNull());
+}
+
 TEST_F(TransactionParticipantRetryableWritesTest, SessionTransactionsCollectionNotDefaultCreated) {
     auto txnParticipant = TransactionParticipant::get(opCtx());
     txnParticipant.refreshFromStorageIfNeeded(opCtx());
diff --git a/src/mongo/db/transaction_participant_test.cpp b/src/mongo/db/transaction_participant_test.cpp
index 932ef261e88..b02fd45ba4c 100644
--- a/src/mongo/db/transaction_participant_test.cpp
+++ b/src/mongo/db/transaction_participant_test.cpp
@@ -1145,6 +1145,43 @@ TEST_F(TxnParticipantTest, CannotContinueTransactionIfNotPrimary) {
         ErrorCodes::NotMaster);
 }
 
+TEST_F(TxnParticipantTest, OlderTransactionFailsOnSessionWithNewerTransaction) {
+    // Will start the transaction.
+    auto sessionCheckout = checkOutSession();
+    auto txnParticipant = TransactionParticipant::get(opCtx());
+    ASSERT_TRUE(txnParticipant.transactionIsOpen());
+    auto autocommit = false;
+    auto startTransaction = true;
+    const auto& sessionId = *opCtx()->getLogicalSessionId();
+
+    StringBuilder sb;
+    sb << "Cannot start transaction 19 on session " << sessionId
+       << " because a newer transaction with txnNumber 20 has already started on this session.";
+    ASSERT_THROWS_WHAT(txnParticipant.beginOrContinue(
+                           opCtx(), *opCtx()->getTxnNumber() - 1, autocommit, startTransaction),
+                       AssertionException,
+                       sb.str());
+    ASSERT(txnParticipant.getLastWriteOpTime().isNull());
+}
+
+
+TEST_F(TxnParticipantTest, OldRetryableWriteFailsOnSessionWithNewerTransaction) {
+    // Will start the transaction.
+    auto sessionCheckout = checkOutSession();
+    auto txnParticipant = TransactionParticipant::get(opCtx());
+    ASSERT_TRUE(txnParticipant.transactionIsOpen());
+    const auto& sessionId = *opCtx()->getLogicalSessionId();
+
+    StringBuilder sb;
+    sb << "Retryable write with txnNumber 19 is prohibited on session " << sessionId
+       << " because a newer transaction with txnNumber 20 has already started on this session.";
+    ASSERT_THROWS_WHAT(txnParticipant.beginOrContinue(
+                           opCtx(), *opCtx()->getTxnNumber() - 1, boost::none, boost::none),
+                       AssertionException,
+                       sb.str());
+    ASSERT(txnParticipant.getLastWriteOpTime().isNull());
+}
+
 TEST_F(TxnParticipantTest, CannotStartNewTransactionWhilePreparedTransactionInProgress) {
     auto sessionCheckout = checkOutSession();
     auto txnParticipant = TransactionParticipant::get(opCtx());