From df442da1c38bfb1bdb1e6358bbe5ab263b0f78c1 Mon Sep 17 00:00:00 2001 From: Siyuan Zhou Date: Wed, 25 Sep 2019 22:43:20 +0000 Subject: SERVER-39613 Remove kCommittingWithPrepare and kCommittingWithoutPrepare from TransactionState (cherry picked from commit dd2de577670e9461c31ca9e6fe5c9713b4401181) --- src/mongo/db/op_observer_impl_test.cpp | 5 --- src/mongo/db/transaction_participant.cpp | 64 ++++----------------------- src/mongo/db/transaction_participant.h | 23 ++-------- src/mongo/db/transaction_participant_test.cpp | 24 ---------- 4 files changed, 13 insertions(+), 103 deletions(-) diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp index 8184a4c9c03..ce06a980817 100644 --- a/src/mongo/db/op_observer_impl_test.cpp +++ b/src/mongo/db/op_observer_impl_test.cpp @@ -786,8 +786,6 @@ TEST_F(OpObserverTransactionTest, TransactionalPreparedCommitTest) { opCtx()->setWriteUnitOfWork(nullptr); opCtx()->lockState()->unsetMaxLockTimeout(); - txnParticipant.transitionToCommittingWithPrepareforTest(opCtx()); - { Lock::GlobalLock lk(opCtx(), MODE_IX); opObserver().onPreparedTransactionCommit( @@ -1080,7 +1078,6 @@ TEST_F(OpObserverTransactionTest, CommittingPreparedTransactionWritesToTransacti opCtx()->setWriteUnitOfWork(nullptr); opCtx()->lockState()->unsetMaxLockTimeout(); - txnParticipant.transitionToCommittingWithPrepareforTest(opCtx()); { Lock::GlobalLock lk(opCtx(), MODE_IX); opObserver().onPreparedTransactionCommit( @@ -1769,7 +1766,6 @@ TEST_F(OpObserverMultiEntryTransactionTest, CommitPreparedTest) { // commitTimestamp must be greater than the prepareTimestamp. auto commitTimestamp = Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); - txnParticipant.transitionToCommittingWithPrepareforTest(opCtx()); { Lock::GlobalLock lk(opCtx(), MODE_IX); opObserver().onPreparedTransactionCommit( @@ -2039,7 +2035,6 @@ TEST_F(OpObserverMultiEntryTransactionTest, CommitPreparedPackingTest) { // commitTimestamp must be greater than the prepareTimestamp. auto commitTimestamp = Timestamp(prepareTimestamp.getSecs(), prepareTimestamp.getInc() + 1); - txnParticipant.transitionToCommittingWithPrepareforTest(opCtx()); opObserver().onPreparedTransactionCommit( opCtx(), commitSlot, diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp index 85e09b8dfc9..f9a0a64cc65 100644 --- a/src/mongo/db/transaction_participant.cpp +++ b/src/mongo/db/transaction_participant.cpp @@ -1226,11 +1226,9 @@ std::vector& TransactionParticipant::Participant::retrieveCompletedTransactionOperations( OperationContext* opCtx) { - // Ensure that we only ever retrieve a transaction's completed operations when in progress, - // committing with prepare, or prepared. - invariant(o().txnState.isInSet(TransactionState::kInProgress | - TransactionState::kCommittingWithPrepare | - TransactionState::kPrepared), + // Ensure that we only ever retrieve a transaction's completed operations when in progress + // or prepared. + invariant(o().txnState.isInSet(TransactionState::kInProgress | TransactionState::kPrepared), str::stream() << "Current state: " << o().txnState); return p().transactionOperations; @@ -1244,9 +1242,8 @@ TxnResponseMetadata TransactionParticipant::Participant::getResponseMetadata() { } void TransactionParticipant::Participant::clearOperationsInMemory(OperationContext* opCtx) { - // Ensure that we only ever end a transaction when committing with prepare or in progress. - invariant(o().txnState.isInSet(TransactionState::kCommittingWithPrepare | - TransactionState::kInProgress), + // Ensure that we only ever end a prepared or in-progress transaction. + invariant(o().txnState.isInSet(TransactionState::kPrepared | TransactionState::kInProgress), str::stream() << "Current state: " << o().txnState); invariant(p().autoCommit); p().transactionOperationBytes = 0; @@ -1275,22 +1272,11 @@ void TransactionParticipant::Participant::commitUnpreparedTransaction(OperationC auto needsNoopWrite = txnOps.empty() && !opCtx->getWriteConcern().usedDefault; clearOperationsInMemory(opCtx); - { - stdx::lock_guard lk(*opCtx->getClient()); - // The oplog entry is written in the same WUOW with the data change for unprepared - // transactions. We can still consider the state is InProgress until now, since no - // externally visible changes have been made yet by the commit operation. If anything throws - // before this point in the function, entry point will abort the transaction. - o(lk).txnState.transitionTo(TransactionState::kCommittingWithoutPrepare); - } try { - // Once entering "committing without prepare" we cannot throw an exception. + // Once committing we cannot throw an exception. UninterruptibleLockGuard noInterrupt(opCtx->lockState()); _commitStorageTransaction(opCtx); - invariant(o().txnState.isCommittingWithoutPrepare(), - str::stream() << "Current state: " << o().txnState); - _finishCommitTransaction(opCtx); } catch (...) { // It is illegal for committing a transaction to fail for any reason, other than an @@ -1364,11 +1350,6 @@ void TransactionParticipant::Participant::commitPreparedTransaction( MONGO_FAIL_POINT(skipCommitTxnCheckPrepareMajorityCommitted)); } - { - stdx::lock_guard lk(*opCtx->getClient()); - o(lk).txnState.transitionTo(TransactionState::kCommittingWithPrepare); - } - try { // We can no longer uassert without terminating. unlockGuard.dismiss(); @@ -1550,7 +1531,6 @@ void TransactionParticipant::Participant::abortTransactionForStepUp(OperationCon void TransactionParticipant::Participant::_abortActiveTransaction( OperationContext* opCtx, TransactionState::StateSet expectedStates, bool writeOplog) { invariant(!o().txnResourceStash); - invariant(!o().txnState.isCommittingWithPrepare()); if (!o().txnState.isInRetryableWriteMode()) { stdx::lock_guard lk(*opCtx->getClient()); @@ -1593,8 +1573,6 @@ void TransactionParticipant::Participant::_abortActiveTransaction( // Cannot abort these states unless they are specified in expectedStates explicitly. const auto unabortableStates = TransactionState::kPrepared // - | TransactionState::kCommittingWithPrepare // - | TransactionState::kCommittingWithoutPrepare // | TransactionState::kCommitted; // invariant(!o().txnState.isInSet(unabortableStates), str::stream() << "Cannot abort transaction in " << o().txnState); @@ -1735,10 +1713,6 @@ std::string TransactionParticipant::TransactionState::toString(StateFlag state) return "TxnState::InProgress"; case TransactionParticipant::TransactionState::kPrepared: return "TxnState::Prepared"; - case TransactionParticipant::TransactionState::kCommittingWithoutPrepare: - return "TxnState::CommittingWithoutPrepare"; - case TransactionParticipant::TransactionState::kCommittingWithPrepare: - return "TxnState::CommittingWithPrepare"; case TransactionParticipant::TransactionState::kCommitted: return "TxnState::Committed"; case TransactionParticipant::TransactionState::kAbortedWithoutPrepare: @@ -1768,7 +1742,7 @@ bool TransactionParticipant::TransactionState::_isLegalTransition(StateFlag oldS switch (newState) { case kNone: case kPrepared: - case kCommittingWithoutPrepare: + case kCommitted: case kAbortedWithoutPrepare: return true; default: @@ -1777,26 +1751,8 @@ bool TransactionParticipant::TransactionState::_isLegalTransition(StateFlag oldS MONGO_UNREACHABLE; case kPrepared: switch (newState) { - case kCommittingWithPrepare: case kAbortedWithPrepare: - return true; - default: - return false; - } - MONGO_UNREACHABLE; - case kCommittingWithPrepare: - switch (newState) { - case kCommitted: - return true; - default: - return false; - } - MONGO_UNREACHABLE; - case kCommittingWithoutPrepare: - switch (newState) { - case kNone: case kCommitted: - case kAbortedWithoutPrepare: return true; default: return false; @@ -1967,8 +1923,7 @@ void TransactionParticipant::Participant::_setNewTxnNumber(OperationContext* opC const TxnNumber& txnNumber) { uassert(ErrorCodes::PreparedTransactionInProgress, "Cannot change transaction number while the session has a prepared transaction", - !o().txnState.isInSet(TransactionState::kPrepared | - TransactionState::kCommittingWithPrepare)); + !o().txnState.isInSet(TransactionState::kPrepared)); LOG_FOR_TRANSACTION(4) << "New transaction started with txnNumber: " << txnNumber << " on session with lsid " << _sessionId().getId(); @@ -2124,8 +2079,7 @@ void TransactionParticipant::Participant::invalidate(OperationContext* opCtx) { uassert(ErrorCodes::PreparedTransactionInProgress, "Cannot invalidate prepared transaction", - !o().txnState.isInSet(TransactionState::kPrepared | - TransactionState::kCommittingWithPrepare)); + !o().txnState.isInSet(TransactionState::kPrepared)); // Invalidate the session and clear both the retryable writes and transactional states on // this participant. diff --git a/src/mongo/db/transaction_participant.h b/src/mongo/db/transaction_participant.h index 1b5b41d0d75..a569241e3cf 100644 --- a/src/mongo/db/transaction_participant.h +++ b/src/mongo/db/transaction_participant.h @@ -92,12 +92,10 @@ class TransactionParticipant { kNone = 1 << 0, kInProgress = 1 << 1, kPrepared = 1 << 2, - kCommittingWithoutPrepare = 1 << 3, - kCommittingWithPrepare = 1 << 4, - kCommitted = 1 << 5, - kAbortedWithoutPrepare = 1 << 6, - kAbortedWithPrepare = 1 << 7, - kExecutedRetryableWrite = 1 << 8, + kCommitted = 1 << 3, + kAbortedWithoutPrepare = 1 << 4, + kAbortedWithPrepare = 1 << 5, + kExecutedRetryableWrite = 1 << 6, }; using StateSet = int; @@ -130,14 +128,6 @@ class TransactionParticipant { return _state == kPrepared; } - bool isCommittingWithoutPrepare() const { - return _state == kCommittingWithoutPrepare; - } - - bool isCommittingWithPrepare() const { - return _state == kCommittingWithPrepare; - } - bool isCommitted() const { return _state == kCommitted; } @@ -665,11 +655,6 @@ public: o(lk).txnState.transitionTo(TransactionState::kPrepared); } - void transitionToCommittingWithPrepareforTest(OperationContext* opCtx) { - stdx::lock_guard lk(*opCtx->getClient()); - o(lk).txnState.transitionTo(TransactionState::kCommittingWithPrepare); - } - void transitionToAbortedWithoutPrepareforTest(OperationContext* opCtx) { stdx::lock_guard lk(*opCtx->getClient()); o(lk).txnState.transitionTo(TransactionState::kAbortedWithoutPrepare); diff --git a/src/mongo/db/transaction_participant_test.cpp b/src/mongo/db/transaction_participant_test.cpp index fb109b38bbc..a186012cc2b 100644 --- a/src/mongo/db/transaction_participant_test.cpp +++ b/src/mongo/db/transaction_participant_test.cpp @@ -1294,30 +1294,6 @@ TEST_F(TxnParticipantTest, ImplicitAbortDoesNotAbortPreparedTransaction) { ASSERT_TRUE(_opObserver->transactionPrepared); } -DEATH_TEST_F(TxnParticipantTest, - AbortIsIllegalDuringCommittingPreparedTransaction, - "isCommittingWithPrepare") { - auto outerScopedSession = checkOutSession(); - auto txnParticipant = TransactionParticipant::get(opCtx()); - - txnParticipant.unstashTransactionResources(opCtx(), "insert"); - auto operation = repl::OplogEntry::makeInsertOperation(kNss, _uuid, BSON("TestValue" << 0)); - - txnParticipant.addTransactionOperation(opCtx(), operation); - auto prepareTimestamp = txnParticipant.prepareTransaction(opCtx(), {}); - - _opObserver->onPreparedTransactionCommitFn = - [&](OplogSlot commitOplogEntryOpTime, - Timestamp commitTimestamp, - const std::vector& statements) { - // Hit an invariant. This should never happen. - txnParticipant.abortActiveTransaction(opCtx()); - ASSERT_FALSE(txnParticipant.transactionIsAborted()); - }; - - txnParticipant.commitPreparedTransaction(opCtx(), prepareTimestamp, {}); -} - TEST_F(TxnParticipantTest, CannotContinueNonExistentTransaction) { MongoDOperationContextSession opCtxSession(opCtx()); auto txnParticipant = TransactionParticipant::get(opCtx()); -- cgit v1.2.1