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

(cherry picked from commit eb284b042c71edf0eac445d3ceb79f7fdeabc5d1)

4 files changed, 139 insertions, 13 deletions

diff --git a/jstests/sharding/retryable_writes.js b/jstests/sharding/retryable_writes.js
index cfcef0d0eed..8133a8c9011 100644
--- a/jstests/sharding/retryable_writes.js
+++ b/jstests/sharding/retryable_writes.js
@@ -508,6 +508,30 @@
         assert.commandFailed(localDB.runCommand(cmd));
     }
 
+    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);
+    }
+
     // Tests for replica set
     var replTest = new ReplSetTest({nodes: 2});
     replTest.startSet({verbose: 5});
@@ -519,6 +543,7 @@
     runFailpointTests(priConn, priConn);
     runMultiTests(priConn);
     runInvalidTests(priConn);
+    runRetryableWriteErrorTest(priConn);
 
     replTest.stopSet();
 
diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp
index c94ea1bfba0..edcc5be1294 100644
--- a/src/mongo/db/session.cpp
+++ b/src/mongo/db/session.cpp
@@ -504,7 +504,7 @@ void Session::_beginOrContinueTxn(WithLock wl,
 
     // Check if the given transaction number is valid for this session. The transaction number must
     // be >= the active transaction number.
-    _checkTxnValid(wl, txnNumber);
+    _checkTxnValid(wl, txnNumber, autocommit);
 
     //
     // Continue an active transaction.
@@ -610,14 +610,33 @@ void Session::_beginOrContinueTxn(WithLock wl,
     invariant(_transactionOperations.empty());
 }
 
-void Session::_checkTxnValid(WithLock, TxnNumber txnNumber) const {
-    uassert(ErrorCodes::TransactionTooOld,
-            str::stream() << "Cannot start transaction " << txnNumber << " on session "
-                          << getSessionId()
-                          << " because a newer transaction "
-                          << _activeTxnNumber
-                          << " has already started.",
-            txnNumber >= _activeTxnNumber);
+void Session::_checkTxnValid(WithLock,
+                             TxnNumber txnNumber,
+                             boost::optional<bool> autocommit) const {
+    if (txnNumber < _activeTxnNumber) {
+        const std::string currOperation =
+            _txnState == MultiDocumentTransactionState::kNone ? "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 "
+                                    << _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 "
+                                    << _activeTxnNumber
+                                    << " has already started on this session.");
+        }
+    }
 }
 
 Session::TxnResources::TxnResources(OperationContext* opCtx, bool keepTicket) {
@@ -994,7 +1013,10 @@ void Session::_abortTransaction(WithLock wl) {
 
 void Session::_beginOrContinueTxnOnMigration(WithLock wl, TxnNumber txnNumber) {
     _checkValid(wl);
-    _checkTxnValid(wl, txnNumber);
+    // The value for 'autocommit' is only used to
+    // generate the uassert error message. In this case, the exception will never be
+    // propagated to the user.
+    _checkTxnValid(wl, txnNumber, boost::optional<bool>());
 
     // Check for continuing an existing transaction
     if (txnNumber == _activeTxnNumber)
diff --git a/src/mongo/db/session.h b/src/mongo/db/session.h
index 01d94721537..df9bceeae2e 100644
--- a/src/mongo/db/session.h
+++ b/src/mongo/db/session.h
@@ -421,9 +421,10 @@ private:
     // Checks if there is a conflicting operation on the current Session
     void _checkValid(WithLock) const;
 
-    // Checks that a new txnNumber is higher than the activeTxnNumber so
-    // we don't start a txn that is too old.
-    void _checkTxnValid(WithLock, TxnNumber txnNumber) const;
+    // Checks that a new txnNumber is higher than the activeTxnNumber so we don't start a
+    // transaction or retryable write that is older
+    // than the current one.
+    void _checkTxnValid(WithLock, TxnNumber txnNumber, boost::optional<bool> autocommit) const;
 
     void _setActiveTxn(WithLock, TxnNumber txnNumber);
 
diff --git a/src/mongo/db/session_test.cpp b/src/mongo/db/session_test.cpp
index e16f0a74663..fcd5ad7b9dc 100644
--- a/src/mongo/db/session_test.cpp
+++ b/src/mongo/db/session_test.cpp
@@ -921,6 +921,84 @@ TEST_F(SessionTest, CannotSpecifyStartTransactionOnInProgressTxn) {
                        ErrorCodes::ConflictingOperationInProgress);
 }
 
+TEST_F(SessionTest, OlderTransactionFailsOnSessionWithNewerTransaction) {
+    // Will start the transaction.
+    const auto sessionId = makeLogicalSessionIdForTest();
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    const bool autocommit = false;
+    const bool startTransaction = true;
+    const TxnNumber txnNum = 20;
+    session.beginOrContinueTxn(opCtx(), txnNum, autocommit, startTransaction, "testDB", "insert");
+
+    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(session.beginOrContinueTxn(
+                           opCtx(), txnNum - 1, autocommit, startTransaction, "testDB", "insert"),
+                       AssertionException,
+                       sb.str());
+    ASSERT(session.getLastWriteOpTime(txnNum).isNull());
+}
+
+TEST_F(SessionTest, OldRetryableWriteFailsOnSessionWithNewerTransaction) {
+    // Will start the transaction.
+    const auto sessionId = makeLogicalSessionIdForTest();
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    const bool autocommit = false;
+    const bool startTransaction = true;
+    const TxnNumber txnNum = 20;
+    session.beginOrContinueTxn(opCtx(), txnNum, autocommit, startTransaction, "testDB", "insert");
+
+    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(session.beginOrContinueTxn(
+                           opCtx(), txnNum - 1, boost::none, boost::none, "testDB", "insert"),
+                       AssertionException,
+                       sb.str());
+    ASSERT(session.getLastWriteOpTime(txnNum).isNull());
+}
+
+TEST_F(SessionTest, OlderRetryableWriteFailsOnSessionWithNewerRetryableWrite) {
+    const auto sessionId = makeLogicalSessionIdForTest();
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+    const TxnNumber txnNum = 22;
+
+    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.";
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none, "testDB", "insert");
+    ASSERT_THROWS_WHAT(session.beginOrContinueTxn(
+                           opCtx(), txnNum - 1, boost::none, boost::none, "testDB", "insert"),
+                       AssertionException,
+                       sb.str());
+    ASSERT(session.getLastWriteOpTime(txnNum).isNull());
+}
+
+TEST_F(SessionTest, OldTransactionFailsOnSessionWithNewerRetryableWrite) {
+    const auto sessionId = makeLogicalSessionIdForTest();
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    const TxnNumber txnNum = 22;
+    const auto autocommit = false;
+
+    StringBuilder sb;
+    sb << "Cannot start transaction 21 on session " << sessionId
+       << " because a newer retryable write with txnNumber 22 has already started on this session.";
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none, "testDB", "insert");
+    ASSERT_THROWS_WHAT(session.beginOrContinueTxn(
+                           opCtx(), txnNum - 1, autocommit, boost::none, "testDB", "insert"),
+                       AssertionException,
+                       sb.str());
+    ASSERT(session.getLastWriteOpTime(txnNum).isNull());
+}
+
 TEST_F(SessionTest, AutocommitRequiredOnEveryTxnOp) {
     const auto sessionId = makeLogicalSessionIdForTest();
     Session session(sessionId);