diff options
author | Jason Chan <jason.chan@10gen.com> | 2019-06-06 10:56:44 -0400 |
---|---|---|
committer | Jason Chan <jason.chan@10gen.com> | 2019-06-06 17:49:42 -0400 |
commit | caa94f234cbda79605b355b0bd48bfdd4d2b0a0e (patch) | |
tree | d83da31611d0dc5495d6110e29aab71e3c5882c0 | |
parent | 75cf3adf68c739482f8991a559f8e989be5cdc04 (diff) | |
download | mongo-caa94f234cbda79605b355b0bd48bfdd4d2b0a0e.tar.gz |
SERVER-41157 Remove abortguard when committing unprepared transactions
(cherry picked from commit b8da78a5e9072ccb293e7d848dbb1357889c794b)
-rw-r--r-- | src/mongo/db/SConscript | 3 | ||||
-rw-r--r-- | src/mongo/db/op_observer_impl.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant.cpp | 16 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant_test.cpp | 107 |
4 files changed, 8 insertions, 132 deletions
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 864d55d2248..c8874d3f659 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -2088,10 +2088,9 @@ env.CppUnitTest( 'auth/authmocks', 'dbdirectclient', 'logical_session_id', - 'op_observer_impl', + 'op_observer', 'query_exec', 'repl/mock_repl_coord_server_fixture', - 'repl/oplog_interface_local', 'repl/repl_coordinator_interface', 'service_liaison_mock', 'sessions_collection_mock', diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index 1e87d9681f7..12b71626fe9 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -1215,10 +1215,10 @@ int logOplogEntriesForTransaction(OperationContext* opCtx, return numEntriesWritten; } -void logCommitOrAbortForTransaction(OperationContext* opCtx, - const OplogSlot& oplogSlot, - const BSONObj& objectField, - DurableTxnStateEnum durableState) { +void logCommitOrAbortForPreparedTransaction(OperationContext* opCtx, + const OplogSlot& oplogSlot, + const BSONObj& objectField, + DurableTxnStateEnum durableState) { const NamespaceString cmdNss{"admin", "$cmd"}; OperationSessionInfo sessionInfo; @@ -1241,7 +1241,7 @@ void logCommitOrAbortForTransaction(OperationContext* opCtx, invariant(!opCtx->lockState()->hasMaxLockTimeout()); writeConflictRetry( - opCtx, "onTransactionCommitOrAbort", NamespaceString::kRsOplogNamespace.ns(), [&] { + opCtx, "onPreparedTransactionCommitOrAbort", NamespaceString::kRsOplogNamespace.ns(), [&] { // Writes to the oplog only require a Global intent lock. Guaranteed by // OplogSlotReserver. @@ -1340,7 +1340,7 @@ void OpObserverImpl::onPreparedTransactionCommit( CommitTransactionOplogObject cmdObj; cmdObj.setCommitTimestamp(commitTimestamp); - logCommitOrAbortForTransaction( + logCommitOrAbortForPreparedTransaction( opCtx, commitOplogEntryOpTime, cmdObj.toBSON(), DurableTxnStateEnum::kCommitted); } @@ -1459,7 +1459,7 @@ void OpObserverImpl::onTransactionAbort(OperationContext* opCtx, } AbortTransactionOplogObject cmdObj; - logCommitOrAbortForTransaction( + logCommitOrAbortForPreparedTransaction( opCtx, *abortOplogEntryOpTime, cmdObj.toBSON(), DurableTxnStateEnum::kAborted); } diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp index 96512354255..70888e12126 100644 --- a/src/mongo/db/transaction_participant.cpp +++ b/src/mongo/db/transaction_participant.cpp @@ -1174,20 +1174,6 @@ void TransactionParticipant::Participant::commitUnpreparedTransaction(OperationC auto opObserver = opCtx->getServiceContext()->getOpObserver(); invariant(opObserver); - auto abortGuard = makeGuard([&] { - if (gUseMultipleOplogEntryFormatForTransactions && - serverGlobalParams.featureCompatibility.getVersion() == - ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) { - // We should already be holding the RSTL if we have performed a read or write - // as part of this unprepared transaction. - invariant(opCtx->lockState()->isRSTLLocked()); - opCtx->runWithoutInterruptionExceptAtGlobalShutdown([&] { - _abortActiveTransaction( - opCtx, TransactionState::kInProgress, true /* writeOplog */); - }); - } - }); - opObserver->onUnpreparedTransactionCommit(opCtx, txnOps); // Read-only transactions with all read concerns must wait for any data they read to be majority @@ -1210,8 +1196,6 @@ void TransactionParticipant::Participant::commitUnpreparedTransaction(OperationC o(lk).txnState.transitionTo(TransactionState::kCommittingWithoutPrepare); } - abortGuard.dismiss(); - try { // Once entering "committing without prepare" we cannot throw an exception. UninterruptibleLockGuard noInterrupt(opCtx->lockState()); diff --git a/src/mongo/db/transaction_participant_test.cpp b/src/mongo/db/transaction_participant_test.cpp index f699bceb432..3af6f687615 100644 --- a/src/mongo/db/transaction_participant_test.cpp +++ b/src/mongo/db/transaction_participant_test.cpp @@ -41,9 +41,7 @@ #include "mongo/db/repl/mock_repl_coord_server_fixture.h" #include "mongo/db/repl/oplog.h" #include "mongo/db/repl/oplog_entry.h" -#include "mongo/db/repl/oplog_interface_local.h" #include "mongo/db/repl/optime.h" -#include "mongo/db/repl/replication_coordinator_mock.h" #include "mongo/db/server_transactions_metrics.h" #include "mongo/db/service_context.h" #include "mongo/db/session_catalog_mongod.h" @@ -64,9 +62,6 @@ namespace mongo { namespace { -using repl::OplogEntry; -using unittest::assertGet; - const NamespaceString kNss("TestDB", "TestColl"); /** @@ -1087,27 +1082,6 @@ TEST_F(TxnParticipantTest, CannotContinueNonExistentTransaction) { ErrorCodes::NoSuchTransaction); } -// Tests that a transaction aborts if it becomes too large before trying to commit it. -TEST_F(TxnParticipantTest, TransactionTooLargeWhileBuilding) { - auto sessionCheckout = checkOutSession(); - auto txnParticipant = TransactionParticipant::get(opCtx()); - - txnParticipant.unstashTransactionResources(opCtx(), "insert"); - - // Two 6MB operations should succeed; three 6MB operations should fail. - constexpr size_t kBigDataSize = 6 * 1024 * 1024; - std::unique_ptr<uint8_t[]> bigData(new uint8_t[kBigDataSize]()); - auto operation = repl::OplogEntry::makeInsertOperation( - kNss, - _uuid, - BSON("_id" << 0 << "data" << BSONBinData(bigData.get(), kBigDataSize, BinDataGeneral))); - txnParticipant.addTransactionOperation(opCtx(), operation); - txnParticipant.addTransactionOperation(opCtx(), operation); - ASSERT_THROWS_CODE(txnParticipant.addTransactionOperation(opCtx(), operation), - AssertionException, - ErrorCodes::TransactionTooLarge); -} - // Tests that a transaction aborts if it becomes too large based on the server parameter // 'transactionLimitBytes'. TEST_F(TxnParticipantTest, TransactionExceedsSizeParameter) { @@ -3830,86 +3804,5 @@ TEST_F(TxnParticipantTest, ExitPreparePromiseIsFulfilledOnAbortPreparedTransacti ASSERT_TRUE(txnParticipant.onExitPrepare().isReady()); } -class MultiEntryOplogTxnParticipantTest : public TxnParticipantTest { - void setUp() override { - gUseMultipleOplogEntryFormatForTransactions = true; - TxnParticipantTest::setUp(); - // Set up ReplicationCoordinator and create oplog. - auto service = opCtx()->getServiceContext(); - repl::ReplicationCoordinator::set( - service, - stdx::make_unique<repl::ReplicationCoordinatorMock>(service, createReplSettings())); - repl::setOplogCollectionName(service); - repl::createOplog(opCtx()); - - // Ensure that we are primary. - auto replCoord = repl::ReplicationCoordinator::get(opCtx()); - ASSERT_OK(replCoord->setFollowerMode(repl::MemberState::RS_PRIMARY)); - } - - void tearDown() override { - TxnParticipantTest::tearDown(); - gUseMultipleOplogEntryFormatForTransactions = false; - } - -protected: - // Assert that the oplog has the expected number of entries, and return them - std::vector<BSONObj> getNOplogEntries(OperationContext* opCtx, int n) { - std::vector<BSONObj> result(n); - repl::OplogInterfaceLocal oplogInterface(opCtx); - auto oplogIter = oplogInterface.makeIterator(); - for (int i = n - 1; i >= 0; i--) { - // The oplogIterator returns the entries in reverse order. - auto opEntry = unittest::assertGet(oplogIter->next()); - result[i] = opEntry.first; - } - ASSERT_EQUALS(ErrorCodes::CollectionIsEmpty, oplogIter->next().getStatus()); - return result; - } - - // Assert that oplog only has a single entry and return that oplog entry. - BSONObj getSingleOplogEntry(OperationContext* opCtx) { - return getNOplogEntries(opCtx, 1).back(); - } - -private: - // Creates a reasonable set of ReplSettings for most tests. - virtual repl::ReplSettings createReplSettings() { - repl::ReplSettings settings; - settings.setOplogSizeBytes(1 * 1024 * 1024); - settings.setReplSetString("mySet/node1:12345"); - return settings; - } -}; - -TEST_F(MultiEntryOplogTxnParticipantTest, ErrorOnUnpreparedCommitAbortsTransaction) { - OpObserverImpl opObserver; - auto sessionCheckout = checkOutSession(); - auto txnParticipant = TransactionParticipant::get(opCtx()); - txnParticipant.unstashTransactionResources(opCtx(), "commitTransaction"); - - // We expect to trigger 'onTransactionAbort' on failure of the unprepared commit. - _opObserver->onTransactionAbortFn = [&]() { - auto abortSlot = repl::getNextOpTime(opCtx()); - opObserver.onTransactionAbort(opCtx(), abortSlot); - }; - - _opObserver->onUnpreparedTransactionCommitThrowsException = true; - ASSERT_THROWS_CODE(txnParticipant.commitUnpreparedTransaction(opCtx()), - AssertionException, - ErrorCodes::OperationFailed); - - auto oplogEntry = getSingleOplogEntry(opCtx()); - - auto abortEntry = assertGet(OplogEntry::parse(oplogEntry)); - auto o = abortEntry.getObject(); - - const auto oExpected = BSON("abortTransaction" << 1); - ASSERT_BSONOBJ_EQ(oExpected, o); - - ASSERT_FALSE(_opObserver->unpreparedTransactionCommitted); - ASSERT_TRUE(txnParticipant.transactionIsAborted()); -} - } // namespace } // namespace mongo |