Revert "SERVER-60742 Maintain RecoveryUnit and storage resources across getMores for non-exchange aggregation operations"

This reverts commit d8afa17c615c274390899917d19180e374040f3e.

5 files changed, 20 insertions, 47 deletions

diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 7bce39af9f3..6898ff287a5 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -1270,7 +1270,6 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorFind
     const CollectionPtr* collection,
     std::unique_ptr<CanonicalQuery> canonicalQuery,
     std::function<void(CanonicalQuery*)> extractAndAttachPipelineStages,
-    bool allowMaintainValidCursorsAcrossCommands,
     bool permitYield,
     size_t plannerOptions) {
     auto yieldPolicy = (permitYield && !opCtx->inMultiDocumentTransaction())
@@ -1280,25 +1279,12 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorFind
     if (OperationShardingState::isOperationVersioned(opCtx)) {
         plannerOptions |= QueryPlannerParams::INCLUDE_SHARD_FILTER;
     }
-    auto executor = getExecutor(opCtx,
-                                collection,
-                                std::move(canonicalQuery),
-                                extractAndAttachPipelineStages,
-                                yieldPolicy,
-                                plannerOptions);
-
-    // If the executor supports it and the operation is eligible, we will maintain the storage
-    // engine state across commands.
-    if (allowMaintainValidCursorsAcrossCommands && executor.isOK() &&
-        serverGlobalParams.featureCompatibility.isVersionInitialized() &&
-        feature_flags::gYieldingSupportForSBE.isEnabled(serverGlobalParams.featureCompatibility) &&
-        !opCtx->inMultiDocumentTransaction() &&
-        repl::ReadConcernArgs::get(opCtx).getLevel() !=
-            repl::ReadConcernLevel::kSnapshotReadConcern) {
-        executor.getValue()->enableSaveRecoveryUnitAcrossCommandsIfSupported();
-    }
-
-    return executor;
+    return getExecutor(opCtx,
+                       collection,
+                       std::move(canonicalQuery),
+                       extractAndAttachPipelineStages,
+                       yieldPolicy,
+                       plannerOptions);
 }
 
 namespace {
diff --git a/src/mongo/db/query/get_executor.h b/src/mongo/db/query/get_executor.h
index 0f8c9bd4bb7..84d1ca7f82a 100644
--- a/src/mongo/db/query/get_executor.h
+++ b/src/mongo/db/query/get_executor.h
@@ -154,7 +154,6 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorFind
     const CollectionPtr* collection,
     std::unique_ptr<CanonicalQuery> canonicalQuery,
     std::function<void(CanonicalQuery*)> extractAndAttachPipelineStages,
-    bool allowMaintainValidCursorsAcrossCommands = true,
     bool permitYield = false,
     size_t plannerOptions = QueryPlannerParams::DEFAULT);
 
diff --git a/src/mongo/db/query/plan_executor_sbe.cpp b/src/mongo/db/query/plan_executor_sbe.cpp
index 3646464dde9..3b7a2db857f 100644
--- a/src/mongo/db/query/plan_executor_sbe.cpp
+++ b/src/mongo/db/query/plan_executor_sbe.cpp
@@ -124,12 +124,11 @@ void PlanExecutorSBE::saveState() {
     if (_isSaveRecoveryUnitAcrossCommandsEnabled) {
         _root->saveState(false /* NOT relinquishing cursor */);
 
-        // Ensure the RU is in 'kCommit' mode so that the following call to abandonSnapshot() keeps
+        // Put the RU into 'kCommit' mode so that subsequent calls to abandonSnapshot() keep
         // cursors positioned. This ensures that no pointers into memory owned by the storage
         // engine held by the SBE PlanStage tree become invalid while the executor is in a saved
         // state.
-        invariant(_opCtx->recoveryUnit()->abandonSnapshotMode() ==
-                  RecoveryUnit::AbandonSnapshotMode::kCommit);
+        _opCtx->recoveryUnit()->setAbandonSnapshotMode(RecoveryUnit::AbandonSnapshotMode::kCommit);
         _opCtx->recoveryUnit()->abandonSnapshot();
     } else {
         _root->saveState(true /* relinquish cursor */);
@@ -143,9 +142,12 @@ void PlanExecutorSBE::restoreState(const RestoreContext& context) {
     _yieldPolicy->setYieldable(context.collection());
 
     if (_isSaveRecoveryUnitAcrossCommandsEnabled) {
-        invariant(_opCtx->recoveryUnit()->abandonSnapshotMode() ==
-                  RecoveryUnit::AbandonSnapshotMode::kCommit);
         _root->restoreState(false /* NOT relinquishing cursor */);
+
+        // Put the RU back into 'kAbort' mode. Since the executor is now in a restored state, calls
+        // to doAbandonSnapshot() only happen if the query has failed and the executor will not be
+        // used again. In this case, we do not rely on the guarantees provided by 'kCommit' mode.
+        _opCtx->recoveryUnit()->setAbandonSnapshotMode(RecoveryUnit::AbandonSnapshotMode::kAbort);
     } else {
         _root->restoreState(true /* relinquish cursor */);
     }
@@ -393,17 +395,4 @@ sbe::PlanState fetchNext(sbe::PlanStage* root,
     return state;
 }
 
-void PlanExecutorSBE::enableSaveRecoveryUnitAcrossCommandsIfSupported() {
-    // If we enable saving the recovery unit across commands, we must put the RecoveryUnit into a
-    // state where calls to abandonSnapshot() result in a transaction commit, instead of abort. The
-    // transaction commit guarantees that all open cursors will remain positioned and valid.
-    //
-    // To put the RecoveryUnit in this state, we bump a reference counter on it which tracks how
-    // many callers require that it be in commit mode. Note that under one RecoveryUnit there may
-    // be multiple PlanExecutors which require cursors to remain valid across abandonSnapshot()
-    // calls (for example, a pipeline containing $lookup).
-    _recoveryUnitCommitModeBlock.emplace(_opCtx->recoveryUnit());
-    _isSaveRecoveryUnitAcrossCommandsEnabled = true;
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/query/plan_executor_sbe.h b/src/mongo/db/query/plan_executor_sbe.h
index 4fadc99ebe1..a0f7a62f57b 100644
--- a/src/mongo/db/query/plan_executor_sbe.h
+++ b/src/mongo/db/query/plan_executor_sbe.h
@@ -50,7 +50,6 @@ public:
                     NamespaceString nss,
                     bool isOpen,
                     std::unique_ptr<PlanYieldPolicySBE> yieldPolicy);
-    ~PlanExecutorSBE() {}
 
     CanonicalQuery* getCanonicalQuery() const override {
         return _cq.get();
@@ -132,7 +131,9 @@ public:
         return *_planExplainer;
     }
 
-    void enableSaveRecoveryUnitAcrossCommandsIfSupported() override;
+    void enableSaveRecoveryUnitAcrossCommandsIfSupported() override {
+        _isSaveRecoveryUnitAcrossCommandsEnabled = true;
+    }
     bool isSaveRecoveryUnitAcrossCommandsEnabled() const override {
         return _isSaveRecoveryUnitAcrossCommandsEnabled;
     }
@@ -178,10 +179,6 @@ private:
     bool _isDisposed{false};
 
     bool _isSaveRecoveryUnitAcrossCommandsEnabled = false;
-    // If engaged, forces the recovery unit to commit instead of abort on calls to
-    // abandonSnapshot(). This field is only used when '_isSaveRecoveryUnitAcrossCommandsEnabled'
-    // is true.
-    boost::optional<AbandonSnapshotCommitModeBlock> _recoveryUnitCommitModeBlock;
 };
 
 /**
diff --git a/src/mongo/db/query/plan_yield_policy.cpp b/src/mongo/db/query/plan_yield_policy.cpp
index 3121e2a07e7..58064f76d6e 100644
--- a/src/mongo/db/query/plan_yield_policy.cpp
+++ b/src/mongo/db/query/plan_yield_policy.cpp
@@ -105,11 +105,13 @@ Status PlanYieldPolicy::yieldOrInterrupt(OperationContext* opCtx,
                 // flag for the duration of yield will force any calls to abandonSnapshot() to
                 // commit the transaction, rather than abort it, in order to leave the cursors
                 // valid.
-                opCtx->recoveryUnit()->incAbandonSnapshotCommitModeCount();
+                opCtx->recoveryUnit()->setAbandonSnapshotMode(
+                    RecoveryUnit::AbandonSnapshotMode::kCommit);
                 exitGuard.emplace([&] {
                     invariant(opCtx->recoveryUnit()->abandonSnapshotMode() ==
                               RecoveryUnit::AbandonSnapshotMode::kCommit);
-                    opCtx->recoveryUnit()->decAbandonSnapshotCommitModeCount();
+                    opCtx->recoveryUnit()->setAbandonSnapshotMode(
+                        RecoveryUnit::AbandonSnapshotMode::kAbort);
                 });
             }