diff options
-rw-r--r-- | etc/backports_required_for_multiversion_tests.yml | 2 | ||||
-rw-r--r-- | jstests/sharding/retryable_writes.js | 25 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant.cpp | 24 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant_retryable_writes_test.cpp | 36 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant_test.cpp | 37 |
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()); |