SERVER-41157 Remove abortguard when committing unprepared transactions

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