SERVER-39807 Remove the server parameter gating large transactions

(cherry picked from commit 9b002077ae7c46b6edcac0ad9eb72e63312cfde3)

10 files changed, 49 insertions, 145 deletions

diff --git a/buildscripts/resmokeconfig/suites/core_txns_multi_oplog_entries.yml b/buildscripts/resmokeconfig/suites/core_txns_multi_oplog_entries.yml
index 84b7de8b0ef..48d55f69d00 100644
--- a/buildscripts/resmokeconfig/suites/core_txns_multi_oplog_entries.yml
+++ b/buildscripts/resmokeconfig/suites/core_txns_multi_oplog_entries.yml
@@ -36,6 +36,5 @@ executor:
     mongod_options:
       set_parameters:
         enableTestCommands: 1
-        useMultipleOplogEntryFormatForTransactions: true
     # Use a 1-node replica set.
     num_nodes: 1
diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns.yml
index 0f7f9eaad18..5f60b8b329c 100644
--- a/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns.yml
+++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns.yml
@@ -12,7 +12,3 @@ executor:
     shell_options:
       nodb: ''
       readMode: commands
-      global_vars:
-        TestData:
-          setParameters:
-            useMultipleOplogEntryFormatForTransactions: true
diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns_jscore_passthrough.yml
index ec7e706bc00..b3ff37c562b 100644
--- a/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_oplog_txns_jscore_passthrough.yml
@@ -41,5 +41,4 @@ executor:
     mongod_options:
       set_parameters:
         enableTestCommands: 1
-        useMultipleOplogEntryFormatForTransactions: true
     num_nodes: 2
diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp
index 12b71626fe9..597030d47c4 100644
--- a/src/mongo/db/op_observer_impl.cpp
+++ b/src/mongo/db/op_observer_impl.cpp
@@ -1083,8 +1083,7 @@ OpTimeBundle logApplyOpsForTransaction(OperationContext* opCtx,
 // Log a single applyOps for transactions without any attempt to pack operations. If the given
 // statements would exceed the maximum BSON size limit of a single oplog entry, this will throw a
 // TransactionTooLarge error.
-// TODO (SERVER-39809): Consider removing this function once old transaction format is no longer
-// needed.
+// TODO(SERVER-41470): Remove this function once old transaction format is no longer needed.
 OpTimeBundle logApplyOpsForTransaction(OperationContext* opCtx,
                                        const std::vector<repl::ReplOperation>& statements,
                                        const OplogSlot& oplogSlot,
@@ -1293,8 +1292,7 @@ void OpObserverImpl::onUnpreparedTransactionCommit(
     // a single call into the OpObserver. Therefore, we store the result here and pass it as an
     // argument.
     const auto fcv = serverGlobalParams.featureCompatibility.getVersion();
-    if (!gUseMultipleOplogEntryFormatForTransactions ||
-        fcv < ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) {
+    if (fcv < ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) {
         auto txnParticipant = TransactionParticipant::get(opCtx);
         const auto lastWriteOpTime = txnParticipant.getLastWriteOpTime();
         invariant(lastWriteOpTime.isNull());
@@ -1361,8 +1359,7 @@ void OpObserverImpl::onTransactionPrepare(OperationContext* opCtx,
     // a single call into the OpObserver. Therefore, we store the result here and pass it as an
     // argument.
     const auto fcv = serverGlobalParams.featureCompatibility.getVersion();
-    if (!gUseMultipleOplogEntryFormatForTransactions ||
-        fcv < ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) {
+    if (fcv < ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) {
         // We write the oplog entry in a side transaction so that we do not commit the now-prepared
         // transaction.
         // We write an empty 'applyOps' entry if there were no writes to choose a prepare timestamp
diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp
index 5bb742454cc..3fd8661a42c 100644
--- a/src/mongo/db/op_observer_impl_test.cpp
+++ b/src/mongo/db/op_observer_impl_test.cpp
@@ -676,48 +676,6 @@ private:
     TxnNumber _txnNum = 0;
 };
 
-/**
- * Test fixture with sessions and an extra-large oplog for testing large transactions.
- */
-class OpObserverLargeTransactionTest : public OpObserverTransactionTest {
-private:
-    repl::ReplSettings createReplSettings() override {
-        repl::ReplSettings settings;
-        // We need an oplog comfortably large enough to hold an oplog entry that exceeds the BSON
-        // size limit.  Otherwise we will get the wrong error code when trying to write one.
-        settings.setOplogSizeBytes(BSONObjMaxInternalSize + 2 * 1024 * 1024);
-        settings.setReplSetString("mySet/node1:12345");
-        return settings;
-    }
-};
-
-// Tests that a transaction aborts if it becomes too large only during the commit.
-TEST_F(OpObserverLargeTransactionTest, TransactionTooLargeWhileCommitting) {
-    const NamespaceString nss("testDB", "testColl");
-    auto uuid = CollectionUUID::gen();
-
-    auto txnParticipant = TransactionParticipant::get(opCtx());
-    txnParticipant.unstashTransactionResources(opCtx(), "insert");
-
-    // This size is crafted such that two operations of this size are not too big to fit in a single
-    // oplog entry, but two operations plus oplog overhead are too big to fit in a single oplog
-    // entry.
-    constexpr size_t kHalfTransactionSize = BSONObjMaxInternalSize / 2 - 175;
-    std::unique_ptr<uint8_t[]> halfTransactionData(new uint8_t[kHalfTransactionSize]());
-    auto operation = repl::OplogEntry::makeInsertOperation(
-        nss,
-        uuid,
-        BSON(
-            "_id" << 0 << "data"
-                  << BSONBinData(halfTransactionData.get(), kHalfTransactionSize, BinDataGeneral)));
-    txnParticipant.addTransactionOperation(opCtx(), operation);
-    txnParticipant.addTransactionOperation(opCtx(), operation);
-    ASSERT_THROWS_CODE(opObserver().onUnpreparedTransactionCommit(
-                           opCtx(), txnParticipant.retrieveCompletedTransactionOperations(opCtx())),
-                       AssertionException,
-                       ErrorCodes::TransactionTooLarge);
-}
-
 TEST_F(OpObserverTransactionTest, TransactionalPrepareTest) {
     const NamespaceString nss1("testDB", "testColl");
     const NamespaceString nss2("testDB2", "testColl2");
@@ -758,11 +716,13 @@ TEST_F(OpObserverTransactionTest, TransactionalPrepareTest) {
     {
         Lock::GlobalLock lk(opCtx(), MODE_IX);
         WriteUnitOfWork wuow(opCtx());
-        OplogSlot slot = repl::getNextOpTime(opCtx());
-        txnParticipant.transitionToPreparedforTest(opCtx(), slot);
-        opCtx()->recoveryUnit()->setPrepareTimestamp(slot.getTimestamp());
+        // One reserved slot for each statement, plus the prepare.
+        auto reservedSlots = repl::getNextOpTimes(opCtx(), 5);
+        auto prepareOpTime = reservedSlots.back();
+        txnParticipant.transitionToPreparedforTest(opCtx(), prepareOpTime);
+        opCtx()->recoveryUnit()->setPrepareTimestamp(prepareOpTime.getTimestamp());
         opObserver().onTransactionPrepare(
-            opCtx(), {slot}, txnParticipant.retrieveCompletedTransactionOperations(opCtx()));
+            opCtx(), reservedSlots, txnParticipant.retrieveCompletedTransactionOperations(opCtx()));
     }
 
     auto oplogEntryObj = getSingleOplogEntry(opCtx());
@@ -983,18 +943,21 @@ TEST_F(OpObserverTransactionTest, TransactionalUnpreparedAbortTest) {
     ASSERT_EQUALS(ErrorCodes::CollectionIsEmpty, oplogIter->next().getStatus());
 }
 
-TEST_F(OpObserverTransactionTest, PreparingEmptyTransactionLogsEmptyApplyOps) {
+TEST_F(OpObserverTransactionTest,
+       PreparingEmptyTransactionLogsEmptyApplyOpsAndWritesToTransactionTable) {
     auto txnParticipant = TransactionParticipant::get(opCtx());
     txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction");
-
+    repl::OpTime prepareOpTime;
     {
         Lock::GlobalLock lk(opCtx(), MODE_IX);
         WriteUnitOfWork wuow(opCtx());
-        OplogSlot slot = repl::getNextOpTime(opCtx());
-        txnParticipant.transitionToPreparedforTest(opCtx(), slot);
-        opCtx()->recoveryUnit()->setPrepareTimestamp(slot.getTimestamp());
+        prepareOpTime = repl::getNextOpTime(opCtx());
+        txnParticipant.transitionToPreparedforTest(opCtx(), prepareOpTime);
+        opCtx()->recoveryUnit()->setPrepareTimestamp(prepareOpTime.getTimestamp());
         opObserver().onTransactionPrepare(
-            opCtx(), {slot}, txnParticipant.retrieveCompletedTransactionOperations(opCtx()));
+            opCtx(),
+            {prepareOpTime},
+            txnParticipant.retrieveCompletedTransactionOperations(opCtx()));
     }
 
     auto oplogEntryObj = getSingleOplogEntry(opCtx());
@@ -1004,7 +967,14 @@ TEST_F(OpObserverTransactionTest, PreparingEmptyTransactionLogsEmptyApplyOps) {
     auto oExpected = BSON("applyOps" << BSONArray() << "prepare" << true);
     ASSERT_BSONOBJ_EQ(oExpected, o);
     ASSERT(oplogEntry.shouldPrepare());
-    ASSERT_EQ(oplogEntry.getTimestamp(), opCtx()->recoveryUnit()->getPrepareTimestamp());
+    const auto startOpTime = oplogEntry.getOpTime();
+    ASSERT_EQ(startOpTime.getTimestamp(), opCtx()->recoveryUnit()->getPrepareTimestamp());
+    ASSERT_EQ(prepareOpTime, txnParticipant.getLastWriteOpTime());
+
+    txnParticipant.stashTransactionResources(opCtx());
+    assertTxnRecord(txnNum(), prepareOpTime, DurableTxnStateEnum::kPrepared);
+    assertTxnRecordStartOpTime(startOpTime);
+    txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction");
 }
 
 TEST_F(OpObserverTransactionTest, PreparingTransactionWritesToTransactionTable) {
@@ -1098,7 +1068,7 @@ TEST_F(OpObserverTransactionTest, CommittingUnpreparedNonEmptyTransactionWritesT
 }
 
 TEST_F(OpObserverTransactionTest,
-       CommittingUnpreparedEmptyTransactionDoesNotWriteToTransactionTable) {
+       CommittingUnpreparedEmptyTransactionDoesNotWriteToTransactionTableOrOplog) {
     auto txnParticipant = TransactionParticipant::get(opCtx());
     txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction");
 
@@ -1107,6 +1077,8 @@ TEST_F(OpObserverTransactionTest,
 
     txnParticipant.stashTransactionResources(opCtx());
 
+    getNOplogEntries(opCtx(), 0);
+
     // Abort the storage-transaction without calling the OpObserver.
     txnParticipant.shutdown(opCtx());
 
@@ -1336,7 +1308,6 @@ TEST_F(OpObserverTransactionTest, TransactionalDeleteTest) {
 
 class OpObserverMultiEntryTransactionTest : public OpObserverTransactionTest {
     void setUp() override {
-        gUseMultipleOplogEntryFormatForTransactions = true;
         _prevPackingLimit = gMaxNumberOfTransactionOperationsInSingleOplogEntry;
         gMaxNumberOfTransactionOperationsInSingleOplogEntry = 1;
         OpObserverTransactionTest::setUp();
@@ -1344,7 +1315,6 @@ class OpObserverMultiEntryTransactionTest : public OpObserverTransactionTest {
 
     void tearDown() override {
         OpObserverTransactionTest::tearDown();
-        gUseMultipleOplogEntryFormatForTransactions = false;
         gMaxNumberOfTransactionOperationsInSingleOplogEntry = _prevPackingLimit;
     }
 
@@ -1352,24 +1322,6 @@ private:
     int _prevPackingLimit;
 };
 
-TEST_F(OpObserverMultiEntryTransactionTest,
-       CommittingUnpreparedEmptyTransactionDoesNotWriteToTransactionTableOrOplog) {
-    auto txnParticipant = TransactionParticipant::get(opCtx());
-    txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction");
-
-    opObserver().onUnpreparedTransactionCommit(
-        opCtx(), txnParticipant.retrieveCompletedTransactionOperations(opCtx()));
-
-    txnParticipant.stashTransactionResources(opCtx());
-
-    getNOplogEntries(opCtx(), 0);
-
-    // Abort the storage-transaction without calling the OpObserver.
-    txnParticipant.shutdown(opCtx());
-
-    assertNoTxnRecord();
-}
-
 TEST_F(OpObserverMultiEntryTransactionTest, TransactionSingleStatementTest) {
     const NamespaceString nss("testDB", "testColl");
     auto uuid = CollectionUUID::gen();
@@ -1631,35 +1583,6 @@ TEST_F(OpObserverMultiEntryTransactionTest, TransactionalDeleteTest) {
     ASSERT_BSONOBJ_EQ(oExpected, oplogEntries[1].getObject());
 }
 
-TEST_F(OpObserverMultiEntryTransactionTest,
-       PreparingEmptyTransactionOnlyWritesPrepareOplogEntryAndToTransactionTable) {
-    auto txnParticipant = TransactionParticipant::get(opCtx());
-    txnParticipant.unstashTransactionResources(opCtx(), "prepareTransaction");
-    repl::OpTime prepareOpTime;
-    auto reservedSlots = repl::getNextOpTimes(opCtx(), 1);
-    prepareOpTime = reservedSlots.back();
-    opCtx()->recoveryUnit()->setPrepareTimestamp(prepareOpTime.getTimestamp());
-    opObserver().onTransactionPrepare(
-        opCtx(), reservedSlots, txnParticipant.retrieveCompletedTransactionOperations(opCtx()));
-
-    auto oplogEntryObjs = getNOplogEntries(opCtx(), 1);
-    auto prepareEntryObj = oplogEntryObjs.back();
-    const auto prepareOplogEntry = assertGet(OplogEntry::parse(prepareEntryObj));
-    checkSessionAndTransactionFields(prepareEntryObj);
-    // The startOpTime should refer to the prepare oplog entry for an empty transaction.
-    const auto startOpTime = prepareOplogEntry.getOpTime();
-
-    ASSERT_EQ(prepareOpTime.getTimestamp(), opCtx()->recoveryUnit()->getPrepareTimestamp());
-    ASSERT_BSONOBJ_EQ(BSON("applyOps" << BSONArray() << "prepare" << true),
-                      prepareOplogEntry.getObject());
-    txnParticipant.stashTransactionResources(opCtx());
-    assertTxnRecord(txnNum(), prepareOpTime, DurableTxnStateEnum::kPrepared);
-    assertTxnRecordStartOpTime(startOpTime);
-    txnParticipant.unstashTransactionResources(opCtx(), "abortTransaction");
-
-    ASSERT_EQ(prepareOpTime, txnParticipant.getLastWriteOpTime());
-}
-
 TEST_F(OpObserverMultiEntryTransactionTest, TransactionalInsertPrepareTest) {
     const NamespaceString nss1("testDB", "testColl");
     const NamespaceString nss2("testDB2", "testColl2");
@@ -2322,22 +2245,25 @@ TEST_F(OpObserverMultiEntryTransactionTest, CommitPreparedPackingTest) {
     assertTxnRecordStartOpTime(boost::none);
 }
 
-class OpObserverLargeMultiEntryTransactionTest : public OpObserverLargeTransactionTest {
-    void setUp() override {
-        gUseMultipleOplogEntryFormatForTransactions = true;
-        OpObserverTransactionTest::setUp();
-    }
-
-    void tearDown() override {
-        OpObserverTransactionTest::tearDown();
-        gUseMultipleOplogEntryFormatForTransactions = false;
+/**
+ * Test fixture with sessions and an extra-large oplog for testing large transactions.
+ */
+class OpObserverLargeTransactionTest : public OpObserverTransactionTest {
+private:
+    repl::ReplSettings createReplSettings() override {
+        repl::ReplSettings settings;
+        // We need an oplog comfortably large enough to hold an oplog entry that exceeds the BSON
+        // size limit.  Otherwise we will get the wrong error code when trying to write one.
+        settings.setOplogSizeBytes(BSONObjMaxInternalSize + 2 * 1024 * 1024);
+        settings.setReplSetString("mySet/node1:12345");
+        return settings;
     }
 };
 
 // Tests that a large transaction may be committed.  This test creates a transaction with two
 // operations that together are just big enough to exceed the size limit, which should result in a
 // two oplog entry transaction.
-TEST_F(OpObserverLargeMultiEntryTransactionTest, LargeTransactionCreatesMultipleOplogEntries) {
+TEST_F(OpObserverLargeTransactionTest, LargeTransactionCreatesMultipleOplogEntries) {
     const NamespaceString nss("testDB", "testColl");
     auto uuid = CollectionUUID::gen();
 
diff --git a/src/mongo/db/repl/sync_tail_test.cpp b/src/mongo/db/repl/sync_tail_test.cpp
index 26672e7c0c5..403b3d4d268 100644
--- a/src/mongo/db/repl/sync_tail_test.cpp
+++ b/src/mongo/db/repl/sync_tail_test.cpp
@@ -552,7 +552,6 @@ protected:
     }
 
     void tearDown() override {
-        gUseMultipleOplogEntryFormatForTransactions = false;
         SyncTailTest::tearDown();
     }
 
diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp
index 70888e12126..dde5cb55a17 100644
--- a/src/mongo/db/transaction_participant.cpp
+++ b/src/mongo/db/transaction_participant.cpp
@@ -1035,9 +1035,9 @@ Timestamp TransactionParticipant::Participant::prepareTransaction(
         // being prepared. When the OplogSlotReserver goes out of scope and is destroyed, the
         // storage-transaction it uses to keep the hole open will abort and the slot (and
         // corresponding oplog hole) will vanish.
-        if (!gUseMultipleOplogEntryFormatForTransactions ||
-            serverGlobalParams.featureCompatibility.getVersion() <
-                ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) {
+        // TODO(SERVER-41470): Remove the if-clause here.
+        if (serverGlobalParams.featureCompatibility.getVersion() <
+            ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) {
             oplogSlotReserver.emplace(opCtx);
         } else {
             // Even if the prepared transaction contained no statements, we always reserve at least
@@ -1130,8 +1130,7 @@ void TransactionParticipant::Participant::addTransactionOperation(
                           << BSONObjMaxInternalSize
                           << " when using featureCompatibilityVersion < 4.2. Actual size is "
                           << p().transactionOperationBytes,
-            (gUseMultipleOplogEntryFormatForTransactions && isFCV42) ||
-                p().transactionOperationBytes <= BSONObjMaxInternalSize);
+            isFCV42 || p().transactionOperationBytes <= BSONObjMaxInternalSize);
 }
 
 std::vector<repl::ReplOperation>&
diff --git a/src/mongo/db/transaction_participant.idl b/src/mongo/db/transaction_participant.idl
index 7e0a0194613..95b68973ca6 100644
--- a/src/mongo/db/transaction_participant.idl
+++ b/src/mongo/db/transaction_participant.idl
@@ -57,14 +57,6 @@ server_parameters:
         on_update: std::ref(TransactionParticipant::observeTransactionLifetimeLimitSeconds)
         default: 60
 
-    useMultipleOplogEntryFormatForTransactions:
-        description: >-
-            Use multi oplog format for transactions to allow transactions > 16MB.
-        set_at: startup
-        cpp_vartype: bool
-        cpp_varname: gUseMultipleOplogEntryFormatForTransactions
-        default: true
-
     maxNumberOfTransactionOperationsInSingleOplogEntry:
         description: >-
             Maximum number of operations to pack into a single oplog entry, when multi-oplog
diff --git a/src/mongo/db/transaction_participant_test.cpp b/src/mongo/db/transaction_participant_test.cpp
index 3af6f687615..82f68661e2d 100644
--- a/src/mongo/db/transaction_participant_test.cpp
+++ b/src/mongo/db/transaction_participant_test.cpp
@@ -34,7 +34,6 @@
 #include "mongo/db/client.h"
 #include "mongo/db/db_raii.h"
 #include "mongo/db/dbdirectclient.h"
-#include "mongo/db/op_observer_impl.h"
 #include "mongo/db/op_observer_noop.h"
 #include "mongo/db/op_observer_registry.h"
 #include "mongo/db/operation_context.h"
@@ -127,7 +126,6 @@ public:
                             boost::optional<OplogSlot> abortOplogEntryOpTime) override;
     bool onTransactionAbortThrowsException = false;
     bool transactionAborted = false;
-    stdx::function<void()> onTransactionAbortFn = []() {};
 
     repl::OpTime onDropCollection(OperationContext* opCtx,
                                   const NamespaceString& collectionName,
@@ -190,7 +188,6 @@ void OpObserverMock::onTransactionAbort(OperationContext* opCtx,
             "onTransactionAbort() failed",
             !onTransactionAbortThrowsException);
     transactionAborted = true;
-    onTransactionAbortFn();
 }
 
 repl::OpTime OpObserverMock::onDropCollection(OperationContext* opCtx,
@@ -3803,6 +3800,5 @@ TEST_F(TxnParticipantTest, ExitPreparePromiseIsFulfilledOnAbortPreparedTransacti
     // ready.
     ASSERT_TRUE(txnParticipant.onExitPrepare().isReady());
 }
-
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp
index d7e10b856e6..6b1cfda9eb9 100644
--- a/src/mongo/dbtests/storage_timestamp_tests.cpp
+++ b/src/mongo/dbtests/storage_timestamp_tests.cpp
@@ -2823,15 +2823,16 @@ public:
     }
 };
 
+// Including this class in a test fixture forces transactions to use one oplog entry per operation
+// instead of packing them into as few oplog entries as fit.  This allows testing of the timestamps
+// of multi-oplog-entry transactions.
 class MultiOplogScopedSettings {
 public:
     MultiOplogScopedSettings()
         : _prevPackingLimit(gMaxNumberOfTransactionOperationsInSingleOplogEntry) {
-        gUseMultipleOplogEntryFormatForTransactions = true;
         gMaxNumberOfTransactionOperationsInSingleOplogEntry = 1;
     }
     ~MultiOplogScopedSettings() {
-        gUseMultipleOplogEntryFormatForTransactions = false;
         gMaxNumberOfTransactionOperationsInSingleOplogEntry = _prevPackingLimit;
     }