SERVER-37449 Remove CursorManager's PlanExecutor registry.

The registry was used to mark PlanExecutors killed. However,
this is no longer needed now that PlanExecutors check
whether they have been killed during yield recovery.

3 files changed, 33 insertions, 67 deletions

diff --git a/src/mongo/db/query/plan_executor.h b/src/mongo/db/query/plan_executor.h
index 86480562f98..ea1eaed31f6 100644
--- a/src/mongo/db/query/plan_executor.h
+++ b/src/mongo/db/query/plan_executor.h
@@ -79,18 +79,28 @@ public:
         // We're EOF.  We won't return any more results (edge case exception: capped+tailable).
         IS_EOF,
 
-        // We were killed. This is a special failure case in which we cannot rely on the
-        // collection or database to still be valid.
+        // The plan executor died, usually due to a concurrent catalog event such as a collection
+        // drop.
+        //
         // If the underlying PlanStage has any information on the error, it will be available in
         // the objOut parameter. Call WorkingSetCommon::toStatusString() to retrieve the error
         // details from the output BSON object.
+        //
+        // The PlanExecutor is no longer capable of executing. The caller may extract stats from the
+        // underlying plan stages, but should not attempt to do anything else with the executor
+        // other than dispose() and destroy it.
         DEAD,
 
-        // getNext was asked for data it cannot provide, or the underlying PlanStage had an
+        // getNext() was asked for data it cannot provide, or the underlying PlanStage had an
         // unrecoverable error.
+        //
         // If the underlying PlanStage has any information on the error, it will be available in
         // the objOut parameter. Call WorkingSetCommon::toStatusString() to retrieve the error
         // details from the output BSON object.
+        //
+        // The PlanExecutor is no longer capable of executing. The caller may extract stats from the
+        // underlying plan stages, but should not attempt to do anything else with the executor
+        // other than dispose() and destroy it.
         FAILURE,
     };
 
@@ -99,20 +109,27 @@ public:
      * (NO_YIELD).
      */
     enum YieldPolicy {
-        // Any call to getNext() may yield. In particular, the executor may be killed during any
-        // call to getNext().  If this occurs, getNext() will return DEAD. Additionally, this
-        // will handle all WriteConflictExceptions that occur while processing the query.
+        // Any call to getNext() may yield. In particular, the executor may die on any call to
+        // getNext() due to a required index or collection becoming invalid during yield. If this
+        // occurs, getNext() will produce an error during yield recovery and will return DEAD.
+        // Additionally, this will handle all WriteConflictExceptions that occur while processing
+        // the query.
         YIELD_AUTO,
 
         // This will handle WriteConflictExceptions that occur while processing the query, but will
         // not yield locks. abandonSnapshot() will be called if a WriteConflictException occurs so
         // callers must be prepared to get a new snapshot. The caller must hold their locks
-        // continuously from construction to destruction, since a PlanExecutor with this policy will
-        // not be registered to receive kill notifications.
+        // continuously from construction to destruction. Callers which do not want auto-yielding,
+        // but may release their locks during query execution must use the YIELD_MANUAL policy.
         WRITE_CONFLICT_RETRY_ONLY,
 
         // Use this policy if you want to disable auto-yielding, but will release locks while using
         // the PlanExecutor. Any WriteConflictExceptions will be raised to the caller of getNext().
+        //
+        // With this policy, an explicit call must be made to saveState() before releasing locks,
+        // and an explicit call to restoreState() must be made after reacquiring locks.
+        // restoreState() will throw if the PlanExecutor is now invalid due to a catalog operation
+        // (e.g. collection drop) during yield.
         YIELD_MANUAL,
 
         // Can be used in one of the following scenarios:
@@ -137,13 +154,6 @@ public:
     };
 
     /**
-     * RegistrationToken is the type of key used to register this PlanExecutor with the
-     * CursorManager.
-     */
-    using RegistrationToken =
-        boost::optional<Partitioned<stdx::unordered_set<PlanExecutor*>>::PartitionId>;
-
-    /**
      * This class will ensure a PlanExecutor is disposed before it is deleted.
      */
     class Deleter {
@@ -167,9 +177,8 @@ public:
         }
 
         /**
-         * If 'execPtr' hasn't already been disposed, will call dispose(). Also, if 'execPtr' has
-         * been registered with the CursorManager, will deregister it. If 'execPtr' is a yielding
-         * PlanExecutor, callers must hold a lock on the collection in at least MODE_IS.
+         * If 'execPtr' hasn't already been disposed, will call dispose(). If 'execPtr' is a
+         * yielding PlanExecutor, callers must hold a lock on the collection in at least MODE_IS.
          */
         inline void operator()(PlanExecutor* execPtr) {
             try {
@@ -204,8 +213,8 @@ public:
     //   - On any call to restoreState().
     //   - While executing the plan inside executePlan().
     //
-    // The executor will also be automatically registered to receive notifications in the case of
-    // YIELD_AUTO or YIELD_MANUAL.
+    // If auto-yielding is enabled, a yield during make() may result in the PlanExecutor being
+    // killed, in which case this method will return a non-OK status.
     //
 
     /**
@@ -386,12 +395,10 @@ public:
     //
 
     /**
-     * If we're yielding locks, the database we're operating over or any collection we're relying on
-     * may be dropped. Plan executors are notified of such events by calling markAsKilled().
-     * Callers must specify the reason for why this executor is being killed. Subsequent calls to
-     * getNext() will return DEAD, and fill 'objOut' with an error reflecting 'killStatus'. If this
-     * method is called multiple times, only the first 'killStatus' will be retained. It is an error
-     * to call this method with Status::OK.
+     * Notifies a PlanExecutor that it should die. Callers must specify the reason for why this
+     * executor is being killed. Subsequent calls to getNext() will return DEAD, and fill 'objOut'
+     * with an error reflecting 'killStatus'. If this method is called multiple times, only the
+     * first 'killStatus' will be retained. It is an error to call this method with Status::OK.
      */
     virtual void markAsKilled(Status killStatus) = 0;
 
@@ -435,15 +442,6 @@ public:
      */
     virtual BSONObjSet getOutputSorts() const = 0;
 
-    /**
-     * Communicate to this PlanExecutor that it is no longer registered with the CursorManager as a
-     * 'non-cached PlanExecutor'.
-     */
-    virtual void unsetRegistered() = 0;
-    virtual RegistrationToken getRegistrationToken() const& = 0;
-    void getRegistrationToken() && = delete;
-    virtual void setRegistrationToken(RegistrationToken token) & = 0;
-
     virtual bool isMarkedAsKilled() const = 0;
     virtual Status getKillStatus() = 0;
 
diff --git a/src/mongo/db/query/plan_executor_impl.cpp b/src/mongo/db/query/plan_executor_impl.cpp
index c703301205a..fd3dc43b110 100644
--- a/src/mongo/db/query/plan_executor_impl.cpp
+++ b/src/mongo/db/query/plan_executor_impl.cpp
@@ -239,9 +239,6 @@ PlanExecutorImpl::PlanExecutorImpl(OperationContext* opCtx,
 
     if (collection) {
         _nss = collection->ns();
-        if (_yieldPolicy->canReleaseLocksDuringExecution()) {
-            _registrationToken = collection->getCursorManager()->registerExecutor(this);
-        }
     } else {
         invariant(_cq);
         _nss = _cq->getQueryRequest().nss();
@@ -661,15 +658,6 @@ void PlanExecutorImpl::dispose(OperationContext* opCtx, CursorManager* cursorMan
         return;
     }
 
-    // If we are registered with the CursorManager we need to be sure to deregister ourselves.
-    // However, if we have been killed we should not attempt to deregister ourselves, since the
-    // caller of markAsKilled() will have done that already, and the CursorManager may no longer
-    // exist. Note that the caller's collection lock prevents us from being marked as killed during
-    // this method, since any interruption event requires a lock in at least MODE_IX.
-    if (cursorManager && _registrationToken && !isMarkedAsKilled()) {
-        dassert(opCtx->lockState()->isCollectionLockedForMode(_nss.ns(), MODE_IS));
-        cursorManager->deregisterExecutor(this);
-    }
     _root->dispose(opCtx);
     _currentState = kDisposed;
 }
@@ -703,19 +691,6 @@ void PlanExecutorImpl::enqueue(const BSONObj& obj) {
     _stash.push(obj.getOwned());
 }
 
-void PlanExecutorImpl::unsetRegistered() {
-    _registrationToken.reset();
-}
-
-PlanExecutor::RegistrationToken PlanExecutorImpl::getRegistrationToken() const& {
-    return _registrationToken;
-}
-
-void PlanExecutorImpl::setRegistrationToken(RegistrationToken token)& {
-    invariant(!_registrationToken);
-    _registrationToken = token;
-}
-
 bool PlanExecutorImpl::isMarkedAsKilled() const {
     return !_killStatus.isOK();
 }
diff --git a/src/mongo/db/query/plan_executor_impl.h b/src/mongo/db/query/plan_executor_impl.h
index 0a13cc25b57..9b9a0f4f7df 100644
--- a/src/mongo/db/query/plan_executor_impl.h
+++ b/src/mongo/db/query/plan_executor_impl.h
@@ -74,9 +74,6 @@ public:
     void dispose(OperationContext* opCtx, CursorManager* cursorManager) final;
     void enqueue(const BSONObj& obj) final;
     BSONObjSet getOutputSorts() const final;
-    void unsetRegistered() final;
-    RegistrationToken getRegistrationToken() const&;
-    void setRegistrationToken(RegistrationToken token) & final;
     bool isMarkedAsKilled() const final;
     Status getKillStatus() final;
     bool isDisposed() const final;
@@ -179,10 +176,6 @@ private:
 
     enum { kUsable, kSaved, kDetached, kDisposed } _currentState = kUsable;
 
-    // Set if this PlanExecutor is registered with the CursorManager.
-    boost::optional<Partitioned<stdx::unordered_set<PlanExecutor*>>::PartitionId>
-        _registrationToken;
-
     bool _everDetachedFromOperationContext = false;
 };