SERVER-34051 Require autocommit=false on all transaction operations and add support

for 'startTransaction' argument

This patch requires all operations that are part of a multi-statement transaction to
specify an autocommit=false command argument. It also adds the 'startTransaction'
command argument, which when specified as 'true' indicates that a command is
the beginning of a multi-statement transaction.

14 files changed, 276 insertions, 99 deletions

diff --git a/src/mongo/db/command_generic_argument.cpp b/src/mongo/db/command_generic_argument.cpp
index ef9530d1cb0..ecd0213865b 100644
--- a/src/mongo/db/command_generic_argument.cpp
+++ b/src/mongo/db/command_generic_argument.cpp
@@ -50,7 +50,7 @@ struct SpecialArgRecord {
 // If that changes, it should be added. When you add to this list, consider whether you
 // should also change the filterCommandRequestForPassthrough() function.
 // clang-format off
-static constexpr std::array<SpecialArgRecord, 23> specials{{
+static constexpr std::array<SpecialArgRecord, 24> specials{{
     //                                       /-isGeneric
     //                                       |  /-stripFromRequest
     //                                       |  |  /-stripFromReply
@@ -73,6 +73,7 @@ static constexpr std::array<SpecialArgRecord, 23> specials{{
     {"lsid"_sd,                              1, 0, 0},
     {"txnNumber"_sd,                         1, 0, 0},
     {"autocommit"_sd,                        1, 1, 0},
+    {"startTransaction"_sd,                  1, 1, 0},
     {"stmtId"_sd,                            1, 0, 0},
     {"$gleStats"_sd,                         0, 0, 1},
     {"operationTime"_sd,                     0, 0, 1},
diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h
index cc69458f5ad..3fa2181b569 100644
--- a/src/mongo/db/commands.h
+++ b/src/mongo/db/commands.h
@@ -115,6 +115,38 @@ struct CommandHelpers {
     static BSONObj appendMajorityWriteConcern(const BSONObj& cmdObj);
 
     /**
+     * Returns true if the provided argument is one that is handled by the command processing layer
+     * and should generally be ignored by individual command implementations. In particular,
+     * commands that fail on unrecognized arguments must not fail for any of these.
+     */
+    static bool isGenericArgument(StringData arg) {
+        // Not including "help" since we don't pass help requests through to the command parser.
+        // If that changes, it should be added. When you add to this list, consider whether you
+        // should also change the filterCommandRequestForPassthrough() function.
+        return arg == "$audit" ||                        //
+            arg == "$client" ||                          //
+            arg == "$configServerState" ||               //
+            arg == "$db" ||                              //
+            arg == "allowImplicitCollectionCreation" ||  //
+            arg == "$oplogQueryData" ||                  //
+            arg == "$queryOptions" ||                    //
+            arg == "$readPreference" ||                  //
+            arg == "$replData" ||                        //
+            arg == "$clusterTime" ||                     //
+            arg == "maxTimeMS" ||                        //
+            arg == "readConcern" ||                      //
+            arg == "databaseVersion" ||                  //
+            arg == "shardVersion" ||                     //
+            arg == "tracking_info" ||                    //
+            arg == "writeConcern" ||                     //
+            arg == "lsid" ||                             //
+            arg == "txnNumber" ||                        //
+            arg == "autocommit" ||                       //
+            arg == "startTransaction" ||                 //
+            false;  // These comments tell clang-format to keep this line-oriented.
+    }
+
+    /**
      * Rewrites cmdObj into a format safe to blindly forward to shards.
      *
      * This performs 2 transformations:
diff --git a/src/mongo/db/logical_session_id.idl b/src/mongo/db/logical_session_id.idl
index d5639ddea69..6c83839e471 100644
--- a/src/mongo/db/logical_session_id.idl
+++ b/src/mongo/db/logical_session_id.idl
@@ -115,6 +115,11 @@ structs:
       autocommit:
         type: bool
         optional: true
+      startTransaction:
+        description: "Used to indicate that a command is the start of a multi-statement
+                      transaction."
+        type: bool
+        optional: true
 
   SessionsCollectionFetchResultIndividualResult:
     description: "Individual result"
diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp
index 625e12dfc91..31a6b9e2bc7 100644
--- a/src/mongo/db/op_observer_impl_test.cpp
+++ b/src/mongo/db/op_observer_impl_test.cpp
@@ -296,7 +296,7 @@ public:
                               NamespaceString nss,
                               TxnNumber txnNum,
                               StmtId stmtId) {
-        session->beginOrContinueTxn(opCtx, txnNum, boost::none);
+        session->beginOrContinueTxn(opCtx, txnNum, boost::none, boost::none);
 
         {
             AutoGetCollection autoColl(opCtx, nss, MODE_IX);
diff --git a/src/mongo/db/repl/do_txn_test.cpp b/src/mongo/db/repl/do_txn_test.cpp
index ce86cb12eda..67242427e4f 100644
--- a/src/mongo/db/repl/do_txn_test.cpp
+++ b/src/mongo/db/repl/do_txn_test.cpp
@@ -145,7 +145,10 @@ void DoTxnTest::setUp() {
     // Set up the transaction and session.
     _opCtx->setLogicalSessionId(makeLogicalSessionIdForTest());
     _opCtx->setTxnNumber(0);  // TxnNumber can always be 0 because we have a new session.
-    _ocs.emplace(_opCtx.get(), true /* checkOutSession */, false /* autocommit */);
+    _ocs.emplace(_opCtx.get(),
+                 true /* checkOutSession */,
+                 false /* autocommit */,
+                 true /* startTransaction */);
     OperationContextSession::get(opCtx())->unstashTransactionResources(opCtx());
 }
 
diff --git a/src/mongo/db/s/session_catalog_migration_destination_test.cpp b/src/mongo/db/s/session_catalog_migration_destination_test.cpp
index b37e7056c53..fdbe9167c58 100644
--- a/src/mongo/db/s/session_catalog_migration_destination_test.cpp
+++ b/src/mongo/db/s/session_catalog_migration_destination_test.cpp
@@ -242,7 +242,8 @@ public:
             // requests with txnNumbers aren't allowed. To get around this, we have to manually set
             // up the session state and perform the insert.
             initializeOperationSessionInfo(innerOpCtx.get(), insertBuilder.obj(), true, true, true);
-            OperationContextSession sessionTxnState(innerOpCtx.get(), true, boost::none);
+            OperationContextSession sessionTxnState(
+                innerOpCtx.get(), true, boost::none, boost::none);
 
             const auto reply = performInserts(innerOpCtx.get(), insertRequest);
             ASSERT(reply.results.size() == 1);
diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp
index 992d7a4b9dc..ad2d1faa7f9 100644
--- a/src/mongo/db/service_entry_point_common.cpp
+++ b/src/mongo/db/service_entry_point_common.cpp
@@ -541,17 +541,26 @@ void execCommandDatabase(OperationContext* opCtx,
         const bool shouldCheckoutSession = static_cast<bool>(opCtx->getTxnNumber()) &&
             sessionCheckoutWhitelist.find(command->getName()) != sessionCheckoutWhitelist.cend();
 
+        // Parse the arguments specific to multi-statement transactions.
+        boost::optional<bool> startMultiDocTxn = boost::none;
         boost::optional<bool> autocommitVal = boost::none;
-        if (sessionOptions && sessionOptions->getAutocommit()) {
-            autocommitVal = *sessionOptions->getAutocommit();
-        } else if (sessionOptions && command->getName() == "doTxn") {
-            // Autocommit is overridden specifically for doTxn to get the oplog entry generation
-            // behavior used for multi-document transactions.
-            // The doTxn command still logically behaves as a commit.
-            autocommitVal = false;
+        if (sessionOptions) {
+            startMultiDocTxn = sessionOptions->getStartTransaction();
+            autocommitVal = sessionOptions->getAutocommit();
+            if (command->getName() == "doTxn") {
+                // Autocommit and 'startMultiDocTxn' are overridden for 'doTxn' to get the oplog
+                // entry generation behavior used for multi-document transactions. The 'doTxn'
+                // command still logically behaves as a commit.
+                autocommitVal = false;
+                startMultiDocTxn = true;
+            }
         }
 
-        OperationContextSession sessionTxnState(opCtx, shouldCheckoutSession, autocommitVal);
+        // This constructor will check out the session and start a transaction, if necessary. It
+        // handles the appropriate state management for both multi-statement transactions and
+        // retryable writes.
+        OperationContextSession sessionTxnState(
+            opCtx, shouldCheckoutSession, autocommitVal, startMultiDocTxn);
 
         const auto dbname = request.getDatabase().toString();
         uassert(
diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp
index 382bdf136fc..0f600d613da 100644
--- a/src/mongo/db/session.cpp
+++ b/src/mongo/db/session.cpp
@@ -265,7 +265,8 @@ void Session::refreshFromStorageIfNeeded(OperationContext* opCtx) {
 
 void Session::beginOrContinueTxn(OperationContext* opCtx,
                                  TxnNumber txnNumber,
-                                 boost::optional<bool> autocommit) {
+                                 boost::optional<bool> autocommit,
+                                 boost::optional<bool> startTransaction) {
     if (opCtx->getClient()->isInDirectClient()) {
         return;
     }
@@ -273,7 +274,7 @@ void Session::beginOrContinueTxn(OperationContext* opCtx,
     invariant(!opCtx->lockState()->isLocked());
 
     stdx::lock_guard<stdx::mutex> lg(_mutex);
-    _beginOrContinueTxn(lg, txnNumber, autocommit);
+    _beginOrContinueTxn(lg, txnNumber, autocommit, startTransaction);
 }
 
 void Session::beginOrContinueTxnOnMigration(OperationContext* opCtx, TxnNumber txnNumber) {
@@ -414,24 +415,85 @@ bool Session::checkStatementExecutedNoOplogEntryFetch(TxnNumber txnNumber, StmtI
 
 void Session::_beginOrContinueTxn(WithLock wl,
                                   TxnNumber txnNumber,
-                                  boost::optional<bool> autocommit) {
+                                  boost::optional<bool> autocommit,
+                                  boost::optional<bool> startTransaction) {
+
+    // Check whether the session information needs to be refreshed from disk.
     _checkValid(wl);
+
+    // Check if the given transaction number is valid for this session. The transaction number must
+    // be >= the active transaction number.
     _checkTxnValid(wl, txnNumber);
 
+    // Reject argument combinations that are never valid.
+    uassert(ErrorCodes::InvalidOptions,
+            "Specifying autocommit=true is not allowed.",
+            autocommit != boost::optional<bool>(true));
+
+    uassert(ErrorCodes::InvalidOptions,
+            "Specifying startTransaction=false is not allowed.",
+            startTransaction != boost::optional<bool>(false));
+
+    uassert(ErrorCodes::InvalidOptions,
+            "Must specify autocommit=false on all operations of a multi-statement transaction.",
+            !(startTransaction == boost::optional<bool>(true) && autocommit == boost::none));
+
+    //
+    // Continue an active transaction.
+    //
     if (txnNumber == _activeTxnNumber) {
-        // Continuing an existing transaction.
-        uassert(ErrorCodes::IllegalOperation,
-                "Specifying 'autocommit' is only allowed at the beginning of a transaction",
-                autocommit == boost::none);
 
+        // It is never valid to specify 'startTransaction' on an active transaction.
+        uassert(ErrorCodes::ConflictingOperationInProgress,
+                str::stream() << "Cannot specify 'startTransaction' on transaction " << txnNumber
+                              << " since it is already in progress.",
+                startTransaction == boost::none);
+
+        // Continue a retryable write or a snapshot read.
+        if (_txnState == MultiDocumentTransactionState::kNone ||
+            _txnState == MultiDocumentTransactionState::kInSnapshotRead) {
+            uassert(ErrorCodes::InvalidOptions,
+                    "Cannot specify 'autocommit' on an operation not inside a multi-statement "
+                    "transaction.",
+                    autocommit == boost::none);
+            return;
+        }
+
+        // Continue a multi-statement transaction. In this case, it is required that
+        // autocommit=false be given as an argument on the request. Retryable writes and snapshot
+        // reads will have _autocommit=true, so that is why we verify that _autocommit=false here.
+        if (!_autocommit) {
+            uassert(
+                ErrorCodes::InvalidOptions,
+                "Must specify autocommit=false on all operations of a multi-statement transaction.",
+                autocommit == boost::optional<bool>(false));
+        }
         return;
     }
 
-    // Start a new transaction with an autocommit field
-    _setActiveTxn(wl, txnNumber);
-    _autocommit = (autocommit != boost::none) ? *autocommit : true;  // autocommit defaults to true
-    _txnState = _autocommit ? MultiDocumentTransactionState::kNone
-                            : MultiDocumentTransactionState::kInProgress;
+    //
+    // Start a new transaction.
+    //
+    // At this point, the given transaction number must be > _activeTxnNumber. Existence of an
+    // 'autocommit' field means we interpret this operation as part of a multi-document transaction.
+    invariant(txnNumber > _activeTxnNumber);
+    if (autocommit) {
+        invariant(*autocommit == false);
+        uassert(ErrorCodes::NoSuchTransaction,
+                str::stream() << "Given transaction number " << txnNumber
+                              << " does not match any in-progress transactions.",
+                startTransaction != boost::none);
+
+        _setActiveTxn(wl, txnNumber);
+        _txnState = MultiDocumentTransactionState::kInProgress;
+        _autocommit = false;
+    } else {
+        invariant(startTransaction == boost::none);
+        _setActiveTxn(wl, txnNumber);
+        _autocommit = true;
+        _txnState = MultiDocumentTransactionState::kNone;
+    }
+
     invariant(_transactionOperations.empty());
 }
 
@@ -509,7 +571,6 @@ void Session::stashTransactionResources(OperationContext* opCtx) {
     // effectively owns the Session. That is, a user might lock the Client to ensure it doesn't go
     // away, and then lock the Session owned by that client. We rely on the fact that we are not
     // using the DefaultLockerImpl to avoid deadlock.
-
     invariant(!isMMAPV1());
     stdx::lock_guard<Client> lk(*opCtx->getClient());
     stdx::unique_lock<stdx::mutex> lg(_mutex);
@@ -627,8 +688,8 @@ void Session::abortActiveTransaction(OperationContext* opCtx) {
 
 void Session::_abortTransaction(WithLock wl) {
     // TODO SERVER-33432 Disallow aborting committed transaction after we implement implicit abort.
-    // A transaction in kCommitting state will either commit or abort for storage-layer reasons;
-    // it is too late to abort externally.
+    // A transaction in kCommitting state will either commit or abort for storage-layer reasons; it
+    // is too late to abort externally.
     if (_txnState == MultiDocumentTransactionState::kCommitting ||
         _txnState == MultiDocumentTransactionState::kCommitted) {
         return;
@@ -720,9 +781,8 @@ void Session::_commitTransaction(stdx::unique_lock<stdx::mutex> lk, OperationCon
         invariant(opObserver);
         opObserver->onTransactionCommit(opCtx);
         lk.lock();
-        // It's possible some other thread aborted the transaction (e.g. through killSession)
-        // while the opObserver was running.  If that happened, the commit should be reported
-        // as failed.
+        // It's possible some other thread aborted the transaction (e.g. through killSession) while
+        // the opObserver was running.  If that happened, the commit should be reported as failed.
         uassert(ErrorCodes::TransactionAborted,
                 str::stream() << "Transaction " << opCtx->getTxnNumber()
                               << " aborted while attempting to commit",
@@ -732,8 +792,8 @@ void Session::_commitTransaction(stdx::unique_lock<stdx::mutex> lk, OperationCon
     _txnState = MultiDocumentTransactionState::kCommitting;
     bool committed = false;
     ON_BLOCK_EXIT([this, &committed, opCtx]() {
-        // If we're still "committing", the recovery unit failed to commit, and the lock
-        // is not held.  We can't safely use _txnState here, as it is protected by the lock.
+        // If we're still "committing", the recovery unit failed to commit, and the lock is not
+        // held.  We can't safely use _txnState here, as it is protected by the lock.
         if (!committed) {
             stdx::lock_guard<stdx::mutex> lk(_mutex);
             opCtx->setWriteUnitOfWork(nullptr);
@@ -853,7 +913,7 @@ void Session::_registerUpdateCacheOnCommit(OperationContext* opCtx,
                 // entry gets invalidated and immediately refreshed while there were no writes for
                 // newTxnNumber yet. In this case _activeTxnNumber will be less than newTxnNumber
                 // and we will fail to update the cache even though the write was successful.
-                _beginOrContinueTxn(lg, newTxnNumber, boost::none);
+                _beginOrContinueTxn(lg, newTxnNumber, boost::none, boost::none);
             }
 
             if (newTxnNumber == _activeTxnNumber) {
diff --git a/src/mongo/db/session.h b/src/mongo/db/session.h
index 53d72d1d746..486ff569053 100644
--- a/src/mongo/db/session.h
+++ b/src/mongo/db/session.h
@@ -108,26 +108,31 @@ public:
     void refreshFromStorageIfNeeded(OperationContext* opCtx);
 
     /**
-     * Starts a new transaction on the session, must be called after refreshFromStorageIfNeeded has
-     * been called. If an attempt is made to start a transaction with number less than the latest
-     * transaction this session has seen, an exception will be thrown.
+     * Starts a new transaction on the session, or continues an already active transaction. In this
+     * context, a "transaction" is a sequence of operations associated with a transaction number.
+     * This sequence of operations could be a retryable write or multi-statement transaction. Both
+     * utilize this method.
      *
-     * Sets the autocommit parameter for this transaction. If it is boost::none, no autocommit
-     * parameter was passed into the request. If this is the first statement of a transaction,
-     * the autocommit parameter will default to true.
+     * The 'autocommit' argument represents the value of the field given in the original client
+     * request. If it is boost::none, no autocommit parameter was passed into the request. Every
+     * operation that is part of a multi statement transaction must specify 'autocommit=false'.
+     * 'startTransaction' represents the value of the field given in the original client request,
+     * and indicates whether this operation is the beginning of a multi-statement transaction.
      *
-     * Autocommit can only be specified on the first statement of a transaction. If otherwise,
-     * this function will throw.
-     *
-     * Throws if the session has been invalidated or if an attempt is made to start a transaction
-     * older than the active.
+     * Throws an exception if:
+     *      - An attempt is made to start a transaction with number less than the latest
+     *        transaction this session has seen.
+     *      - The session has been invalidated.
+     *      - The values of 'autocommit' and/or 'startTransaction' are inconsistent with the current
+     *        state of the transaction.
      *
      * In order to avoid the possibility of deadlock, this method must not be called while holding a
-     * lock.
+     * lock. This method must also be called after refreshFromStorageIfNeeded has been called.
      */
     void beginOrContinueTxn(OperationContext* opCtx,
                             TxnNumber txnNumber,
-                            boost::optional<bool> autocommit);
+                            boost::optional<bool> autocommit,
+                            boost::optional<bool> startTransaction);
     /**
      * Similar to beginOrContinueTxn except it is used specifically for shard migrations and does
      * not check or modify the autocommit parameter.
@@ -313,7 +318,10 @@ public:
         const repl::OplogEntry& entry);
 
 private:
-    void _beginOrContinueTxn(WithLock, TxnNumber txnNumber, boost::optional<bool> autocommit);
+    void _beginOrContinueTxn(WithLock,
+                             TxnNumber txnNumber,
+                             boost::optional<bool> autocommit,
+                             boost::optional<bool> startTransaction);
 
     void _beginOrContinueTxnOnMigration(WithLock, TxnNumber txnNumber);
 
diff --git a/src/mongo/db/session_catalog.cpp b/src/mongo/db/session_catalog.cpp
index 48869e4c338..d70b68da890 100644
--- a/src/mongo/db/session_catalog.cpp
+++ b/src/mongo/db/session_catalog.cpp
@@ -237,7 +237,8 @@ void SessionCatalog::_releaseSession(const LogicalSessionId& lsid) {
 
 OperationContextSession::OperationContextSession(OperationContext* opCtx,
                                                  bool checkOutSession,
-                                                 boost::optional<bool> autocommit)
+                                                 boost::optional<bool> autocommit,
+                                                 boost::optional<bool> startTransaction)
     : _opCtx(opCtx) {
 
     if (!opCtx->getLogicalSessionId()) {
@@ -265,7 +266,8 @@ OperationContextSession::OperationContextSession(OperationContext* opCtx,
     checkedOutSession->get()->refreshFromStorageIfNeeded(opCtx);
 
     if (opCtx->getTxnNumber()) {
-        checkedOutSession->get()->beginOrContinueTxn(opCtx, *opCtx->getTxnNumber(), autocommit);
+        checkedOutSession->get()->beginOrContinueTxn(
+            opCtx, *opCtx->getTxnNumber(), autocommit, startTransaction);
     }
 }
 
diff --git a/src/mongo/db/session_catalog.h b/src/mongo/db/session_catalog.h
index 657f6baf405..fcc2164ca18 100644
--- a/src/mongo/db/session_catalog.h
+++ b/src/mongo/db/session_catalog.h
@@ -249,7 +249,8 @@ class OperationContextSession {
 public:
     OperationContextSession(OperationContext* opCtx,
                             bool checkOutSession,
-                            boost::optional<bool> autocommit);
+                            boost::optional<bool> autocommit,
+                            boost::optional<bool> startTransaction);
 
     ~OperationContextSession();
 
diff --git a/src/mongo/db/session_catalog_test.cpp b/src/mongo/db/session_catalog_test.cpp
index 57506630023..641c0880ca2 100644
--- a/src/mongo/db/session_catalog_test.cpp
+++ b/src/mongo/db/session_catalog_test.cpp
@@ -89,7 +89,7 @@ TEST_F(SessionCatalogTest, OperationContextCheckedOutSession) {
     const TxnNumber txnNum = 20;
     opCtx()->setTxnNumber(txnNum);
 
-    OperationContextSession ocs(opCtx(), true, boost::none);
+    OperationContextSession ocs(opCtx(), true, boost::none, boost::none);
     auto session = OperationContextSession::get(opCtx());
     ASSERT(session);
     ASSERT_EQ(*opCtx()->getLogicalSessionId(), session->getSessionId());
@@ -98,7 +98,7 @@ TEST_F(SessionCatalogTest, OperationContextCheckedOutSession) {
 TEST_F(SessionCatalogTest, OperationContextNonCheckedOutSession) {
     opCtx()->setLogicalSessionId(makeLogicalSessionIdForTest());
 
-    OperationContextSession ocs(opCtx(), false, boost::none);
+    OperationContextSession ocs(opCtx(), false, boost::none, boost::none);
     auto session = OperationContextSession::get(opCtx());
 
     ASSERT(!session);
@@ -117,7 +117,7 @@ TEST_F(SessionCatalogTest, GetOrCreateSessionAfterCheckOutSession) {
     opCtx()->setLogicalSessionId(lsid);
 
     boost::optional<OperationContextSession> ocs;
-    ocs.emplace(opCtx(), true, boost::none);
+    ocs.emplace(opCtx(), true, boost::none, false);
 
     stdx::async(stdx::launch::async, [&] {
         Client::initThreadIfNotAlready();
@@ -146,11 +146,11 @@ TEST_F(SessionCatalogTest, NestedOperationContextSession) {
     opCtx()->setLogicalSessionId(makeLogicalSessionIdForTest());
 
     {
-        OperationContextSession outerScopedSession(opCtx(), true, boost::none);
+        OperationContextSession outerScopedSession(opCtx(), true, boost::none, boost::none);
 
         {
             DirectClientSetter inDirectClient(opCtx());
-            OperationContextSession innerScopedSession(opCtx(), true, boost::none);
+            OperationContextSession innerScopedSession(opCtx(), true, boost::none, boost::none);
 
             auto session = OperationContextSession::get(opCtx());
             ASSERT(session);
@@ -173,7 +173,7 @@ TEST_F(SessionCatalogTest, StashInNestedSessionIsANoop) {
     opCtx()->setTxnNumber(1);
 
     {
-        OperationContextSession outerScopedSession(opCtx(), true, boost::none);
+        OperationContextSession outerScopedSession(opCtx(), true, boost::none, boost::none);
 
         Locker* originalLocker = opCtx()->lockState();
         RecoveryUnit* originalRecoveryUnit = opCtx()->recoveryUnit();
@@ -198,7 +198,7 @@ TEST_F(SessionCatalogTest, StashInNestedSessionIsANoop) {
         {
             // Make it look like we're in a DBDirectClient running a nested operation.
             DirectClientSetter inDirectClient(opCtx());
-            OperationContextSession innerScopedSession(opCtx(), true, boost::none);
+            OperationContextSession innerScopedSession(opCtx(), true, boost::none, boost::none);
 
             // Indicate that there is a stashed cursor. If we were not in a nested session, this
             // would ensure that stashing is not a noop.
@@ -220,7 +220,7 @@ TEST_F(SessionCatalogTest, UnstashInNestedSessionIsANoop) {
     opCtx()->setTxnNumber(1);
 
     {
-        OperationContextSession outerScopedSession(opCtx(), true, boost::none);
+        OperationContextSession outerScopedSession(opCtx(), true, boost::none, boost::none);
 
         Locker* originalLocker = opCtx()->lockState();
         RecoveryUnit* originalRecoveryUnit = opCtx()->recoveryUnit();
@@ -239,7 +239,7 @@ TEST_F(SessionCatalogTest, UnstashInNestedSessionIsANoop) {
         {
             // Make it look like we're in a DBDirectClient running a nested operation.
             DirectClientSetter inDirectClient(opCtx());
-            OperationContextSession innerScopedSession(opCtx(), true, boost::none);
+            OperationContextSession innerScopedSession(opCtx(), true, boost::none, boost::none);
 
             OperationContextSession::get(opCtx())->unstashTransactionResources(opCtx());
 
diff --git a/src/mongo/db/session_test.cpp b/src/mongo/db/session_test.cpp
index 5b03cec656c..6e00de8159f 100644
--- a/src/mongo/db/session_test.cpp
+++ b/src/mongo/db/session_test.cpp
@@ -164,7 +164,7 @@ TEST_F(SessionTest, SessionEntryNotWrittenOnBegin) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 20;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     ASSERT_EQ(sessionId, session.getSessionId());
     ASSERT(session.getLastWriteOpTime(txnNum).isNull());
@@ -182,7 +182,7 @@ TEST_F(SessionTest, SessionEntryWrittenAtFirstWrite) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 21;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     const auto opTime = [&] {
         AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
@@ -215,7 +215,7 @@ TEST_F(SessionTest, StartingNewerTransactionUpdatesThePersistedSession) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const auto writeTxnRecordFn = [&](TxnNumber txnNum, StmtId stmtId, repl::OpTime prevOpTime) {
-        session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+        session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
         AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
         WriteUnitOfWork wuow(opCtx());
@@ -254,9 +254,9 @@ TEST_F(SessionTest, StartingOldTxnShouldAssert) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 20;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
-    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum - 1, boost::none),
+    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum - 1, boost::none, boost::none),
                        AssertionException,
                        ErrorCodes::TransactionTooOld);
     ASSERT(session.getLastWriteOpTime(txnNum).isNull());
@@ -274,7 +274,7 @@ TEST_F(SessionTest, SessionTransactionsCollectionNotDefaultCreated) {
     ASSERT(client.runCommand(nss.db().toString(), BSON("drop" << nss.coll()), dropResult));
 
     const TxnNumber txnNum = 21;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
     WriteUnitOfWork wuow(opCtx());
@@ -289,7 +289,7 @@ TEST_F(SessionTest, CheckStatementExecuted) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 100;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     const auto writeTxnRecordFn = [&](StmtId stmtId, repl::OpTime prevOpTime) {
         AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
@@ -330,7 +330,7 @@ TEST_F(SessionTest, CheckStatementExecutedForOldTransactionThrows) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 100;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     ASSERT_THROWS_CODE(session.checkStatementExecuted(opCtx(), txnNum - 1, 0),
                        AssertionException,
@@ -353,7 +353,7 @@ TEST_F(SessionTest, WriteOpCompletedOnPrimaryForOldTransactionThrows) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 100;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     {
         AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
@@ -380,7 +380,7 @@ TEST_F(SessionTest, WriteOpCompletedOnPrimaryForInvalidatedTransactionThrows) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 100;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
     WriteUnitOfWork wuow(opCtx());
@@ -400,7 +400,7 @@ TEST_F(SessionTest, WriteOpCompletedOnPrimaryCommitIgnoresInvalidation) {
     session.refreshFromStorageIfNeeded(opCtx());
 
     const TxnNumber txnNum = 100;
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     {
         AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
@@ -495,7 +495,7 @@ TEST_F(SessionTest, ErrorOnlyWhenStmtIdBeingCheckedIsNotInCache) {
 
     Session session(sessionId);
     session.refreshFromStorageIfNeeded(opCtx());
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     auto firstOpTime = ([&]() {
         AutoGetCollection autoColl(opCtx(), kNss, MODE_IX);
@@ -581,7 +581,7 @@ TEST_F(SessionTest, StashAndUnstashResources) {
     Session session(sessionId);
     session.refreshFromStorageIfNeeded(opCtx());
 
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none);
 
     repl::ReadConcernArgs readConcernArgs;
     ASSERT_OK(readConcernArgs.initialize(BSON("find"
@@ -622,7 +622,55 @@ TEST_F(SessionTest, StashAndUnstashResources) {
     session.commitTransaction(opCtx());
 }
 
-TEST_F(SessionTest, CheckAutocommitOnlyAllowedAtBeginningOfTxn) {
+TEST_F(SessionTest, StartTransactionRequiredToStartTxn) {
+    const auto sessionId = makeLogicalSessionIdForTest();
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    // Autocommit should be true by default.
+    ASSERT(session.getAutocommit());
+
+    const TxnNumber txnNum = 100;
+
+    // Must specify startTransaction=true and autocommit=false to start a transaction.
+    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, false, false),
+                       AssertionException,
+                       ErrorCodes::InvalidOptions);
+
+    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, false, boost::none),
+                       AssertionException,
+                       ErrorCodes::NoSuchTransaction);
+
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
+
+    // Autocommit should be set to false and we should be in a mult-doc transaction.
+    ASSERT_FALSE(session.getAutocommit());
+    ASSERT_TRUE(session.inSnapshotReadOrMultiDocumentTransaction());
+}
+
+TEST_F(SessionTest, CannotSpecifyStartTransactionOnInProgressTxn) {
+    const auto sessionId = makeLogicalSessionIdForTest();
+    Session session(sessionId);
+    session.refreshFromStorageIfNeeded(opCtx());
+
+    // Autocommit should be true by default
+    ASSERT(session.getAutocommit());
+
+    const TxnNumber txnNum = 100;
+    // Must specify startTransaction=true and autocommit=false to start a transaction.
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
+
+    // Autocommit should be set to false and we should be in a mult-doc transaction.
+    ASSERT_FALSE(session.getAutocommit());
+    ASSERT_TRUE(session.inSnapshotReadOrMultiDocumentTransaction());
+
+    // Cannot try to start a transaction that already started.
+    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, false, true),
+                       AssertionException,
+                       ErrorCodes::ConflictingOperationInProgress);
+}
+
+TEST_F(SessionTest, AutocommitRequiredOnEveryTxnOp) {
     const auto sessionId = makeLogicalSessionIdForTest();
     Session session(sessionId);
     session.refreshFromStorageIfNeeded(opCtx());
@@ -631,16 +679,23 @@ TEST_F(SessionTest, CheckAutocommitOnlyAllowedAtBeginningOfTxn) {
     ASSERT(session.getAutocommit());
 
     const TxnNumber txnNum = 100;
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     // Autocommit should be set to false
     ASSERT_FALSE(session.getAutocommit());
 
-    // Trying to set autocommit after the first statement of a transaction
-    // should throw an error.
-    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, true),
+    // Omitting 'autocommit' after the first statement of a transaction should throw an error.
+    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, boost::none, boost::none),
+                       AssertionException,
+                       ErrorCodes::InvalidOptions);
+
+    // Setting 'autocommit=true' should throw an error.
+    ASSERT_THROWS_CODE(session.beginOrContinueTxn(opCtx(), txnNum, true, boost::none),
                        AssertionException,
-                       ErrorCodes::IllegalOperation);
+                       ErrorCodes::InvalidOptions);
+
+    // Including autocommit=false should succeed.
+    session.beginOrContinueTxn(opCtx(), txnNum, false, boost::none);
 }
 
 TEST_F(SessionTest, SameTransactionPreservesStoredStatements) {
@@ -651,14 +706,14 @@ TEST_F(SessionTest, SameTransactionPreservesStoredStatements) {
     const TxnNumber txnNum = 22;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
     WriteUnitOfWork wuow(opCtx());
     auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0));
     session.addTransactionOperation(opCtx(), operation);
     ASSERT_BSONOBJ_EQ(operation.toBSON(), session.transactionOperationsForTest()[0].toBSON());
 
     // Re-opening the same transaction should have no effect.
-    session.beginOrContinueTxn(opCtx(), txnNum, boost::none);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, boost::none);
     ASSERT_BSONOBJ_EQ(operation.toBSON(), session.transactionOperationsForTest()[0].toBSON());
 }
 
@@ -670,7 +725,7 @@ TEST_F(SessionTest, AbortClearsStoredStatements) {
     const TxnNumber txnNum = 24;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
     session.unstashTransactionResources(opCtx());
     auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0));
     session.addTransactionOperation(opCtx(), operation);
@@ -693,7 +748,7 @@ TEST_F(SessionTest, EmptyTransactionCommit) {
     const TxnNumber txnNum = 25;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
     session.unstashTransactionResources(opCtx());
     // The transaction machinery cannot store an empty locker.
     Lock::GlobalLock lk(opCtx(), MODE_IX, Date_t::now());
@@ -712,7 +767,7 @@ TEST_F(SessionTest, EmptyTransactionAbort) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
     session.unstashTransactionResources(opCtx());
     // The transaction machinery cannot store an empty locker.
     { Lock::GlobalLock lk(opCtx(), MODE_IX, Date_t::now()); }
@@ -729,7 +784,7 @@ TEST_F(SessionTest, ConcurrencyOfUnstashAndAbort) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     // The transaction may be aborted without checking out the session.
     session.abortArbitraryTransaction();
@@ -748,7 +803,7 @@ TEST_F(SessionTest, ConcurrencyOfUnstashAndMigration) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
     // The transaction machinery cannot store an empty locker.
@@ -775,7 +830,7 @@ TEST_F(SessionTest, ConcurrencyOfStashAndAbort) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
 
@@ -794,7 +849,7 @@ TEST_F(SessionTest, ConcurrencyOfStashAndMigration) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
     auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0));
@@ -818,7 +873,7 @@ TEST_F(SessionTest, ConcurrencyOfAddTransactionOperationAndAbort) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
 
@@ -840,7 +895,7 @@ TEST_F(SessionTest, ConcurrencyOfAddTransactionOperationAndMigration) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
     auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0));
@@ -865,7 +920,7 @@ TEST_F(SessionTest, ConcurrencyOfEndTransactionAndRetrieveOperationsAndAbort) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
 
@@ -886,7 +941,7 @@ TEST_F(SessionTest, ConcurrencyOfEndTransactionAndRetrieveOperationsAndMigration
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
     auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0));
@@ -911,7 +966,7 @@ TEST_F(SessionTest, ConcurrencyOfCommitTransactionAndAbort) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
 
@@ -931,7 +986,7 @@ TEST_F(SessionTest, ConcurrencyOfCommitTransactionAndMigration) {
     const TxnNumber txnNum = 26;
     opCtx()->setLogicalSessionId(sessionId);
     opCtx()->setTxnNumber(txnNum);
-    session.beginOrContinueTxn(opCtx(), txnNum, false);
+    session.beginOrContinueTxn(opCtx(), txnNum, false, true);
 
     session.unstashTransactionResources(opCtx());
     auto operation = repl::OplogEntry::makeInsertOperation(kNss, kUUID, BSON("TestValue" << 0));
diff --git a/src/mongo/shell/session.js b/src/mongo/shell/session.js
index 3b65fd32ce5..8914b0680b7 100644
--- a/src/mongo/shell/session.js
+++ b/src/mongo/shell/session.js
@@ -654,13 +654,13 @@ var {
                 cmdObjUnwrapped.txnNumber = new NumberLong(_txnNumber);
             }
 
-            // readConcern and autocommit can only be specified on the first statement in a
-            // transaction.
+            // All operations of a multi-statement transaction must specify autocommit=false.
+            cmdObjUnwrapped.autocommit = false;
+
+            // 'readConcern' and 'startTransaction' can only be specified on the first statement in
+            // a transaction.
             if (_firstStatement) {
-                // TODO: As a part of SERVER-34052, we might also need to specify
-                // `cmdObjUnwrapped.startTransaction = 1` on the first statement of a multi
-                // statement txn.
-                cmdObjUnwrapped.autocommit = false;
+                cmdObjUnwrapped.startTransaction = true;
                 if (_txnOptions.getTxnReadConcern() !== undefined) {
                     cmdObjUnwrapped.readConcern = _txnOptions.getTxnReadConcern();
                 }