diff options
author | Ali Mir <ali.mir@mongodb.com> | 2020-02-18 16:19:33 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-03-31 18:13:48 +0000 |
commit | 89a34f9f823a1efccef072cea0cc1c12fbfcfeea (patch) | |
tree | ad0fef789129fed9f4c33476f4aaf276ebae6201 | |
parent | e8ee1f2e3f4b6d0bddda76480397d5266a5b9671 (diff) | |
download | mongo-89a34f9f823a1efccef072cea0cc1c12fbfcfeea.tar.gz |
SERVER-43889 Distinguish between retryable write and transaction when failing a command
(cherry picked from commit eb284b042c71edf0eac445d3ceb79f7fdeabc5d1)
-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 6b24d743dbc..d978bf7991c 100644 --- a/etc/backports_required_for_multiversion_tests.yml +++ b/etc/backports_required_for_multiversion_tests.yml @@ -55,6 +55,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 d35172edf3a..f176da2d00d 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 ff474125e85..f7b91da6582 100644 --- a/src/mongo/db/transaction_participant.cpp +++ b/src/mongo/db/transaction_participant.cpp @@ -504,11 +504,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 27c2587eeb9..c982c6e6305 100644 --- a/src/mongo/db/transaction_participant_retryable_writes_test.cpp +++ b/src/mongo/db/transaction_participant_retryable_writes_test.cpp @@ -381,6 +381,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 1ac7afd93d3..507ecc17c34 100644 --- a/src/mongo/db/transaction_participant_test.cpp +++ b/src/mongo/db/transaction_participant_test.cpp @@ -1139,6 +1139,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()); |