SERVER-16807 update and delete stages should skip over invalidated documents (MMAP v1)

4 files changed, 33 insertions, 10 deletions

diff --git a/src/mongo/db/exec/delete.cpp b/src/mongo/db/exec/delete.cpp
index 232edd27416..a35413b1c5a 100644
--- a/src/mongo/db/exec/delete.cpp
+++ b/src/mongo/db/exec/delete.cpp
@@ -35,6 +35,7 @@
 #include "mongo/db/catalog/collection.h"
 #include "mongo/db/exec/scoped_timer.h"
 #include "mongo/db/exec/working_set_common.h"
+#include "mongo/db/global_environment_experiment.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/util/log.h"
@@ -86,11 +87,14 @@ namespace mongo {
         if (PlanStage::ADVANCED == status) {
             WorkingSetMember* member = _ws->get(id);
             if (!member->hasLoc()) {
+                // We expect to be here because of an invalidation causing a force-fetch, and
+                // doc-locking storage engines do not issue invalidations.
+                dassert(!supportsDocLocking());
+
                 _ws->free(id);
-                const std::string errmsg = "delete stage failed to read member w/ loc from child";
-                *out = WorkingSetCommon::allocateStatusMember(_ws, Status(ErrorCodes::InternalError,
-                                                                          errmsg));
-                return PlanStage::FAILURE;
+                ++_specificStats.nInvalidateSkips;
+                ++_commonStats.needTime;
+                return PlanStage::NEED_TIME;
             }
             RecordId rloc = member->loc;
 
diff --git a/src/mongo/db/exec/idhack.cpp b/src/mongo/db/exec/idhack.cpp
index ff3e02b6187..15071729a88 100644
--- a/src/mongo/db/exec/idhack.cpp
+++ b/src/mongo/db/exec/idhack.cpp
@@ -186,6 +186,11 @@ namespace mongo {
     void IDHackStage::invalidate(OperationContext* txn, const RecordId& dl, InvalidationType type) {
         ++_commonStats.invalidates;
 
+        // Since updates can't mutate the '_id' field, we can ignore mutation invalidations.
+        if (INVALIDATION_MUTATION == type) {
+            return;
+        }
+
         // It's possible that the loc getting invalidated is the one we're about to
         // fetch. In this case we do a "forced fetch" and put the WSM in owned object state.
         if (WorkingSet::INVALID_ID != _idBeingPagedIn) {
diff --git a/src/mongo/db/exec/plan_stats.h b/src/mongo/db/exec/plan_stats.h
index a36dbe1f839..59ce77035cb 100644
--- a/src/mongo/db/exec/plan_stats.h
+++ b/src/mongo/db/exec/plan_stats.h
@@ -266,13 +266,17 @@ namespace mongo {
     };
 
     struct DeleteStats : public SpecificStats {
-        DeleteStats() : docsDeleted(0) { }
+        DeleteStats() : docsDeleted(0), nInvalidateSkips(0) { }
 
         virtual SpecificStats* clone() const {
             return new DeleteStats(*this);
         }
 
         size_t docsDeleted;
+
+        // Invalidated documents can be force-fetched, causing the now invalid RecordId to
+        // be thrown out. The delete stage skips over any results which do not have a RecordId.
+        size_t nInvalidateSkips;
     };
 
     struct DistinctScanStats : public SpecificStats {
@@ -596,7 +600,8 @@ namespace mongo {
               isDocReplacement(false),
               fastmod(false),
               fastmodinsert(false),
-              inserted(false) { }
+              inserted(false),
+              nInvalidateSkips(0) { }
 
         virtual SpecificStats* clone() const {
             return new UpdateStats(*this);
@@ -626,6 +631,11 @@ namespace mongo {
 
         // The object that was inserted. This is an empty document if no insert was performed.
         BSONObj objInserted;
+
+        // Invalidated documents can be force-fetched, causing the now invalid RecordId to
+        // be thrown out. The update stage skips over any results which do not have the
+        // RecordId to update.
+        size_t nInvalidateSkips;
     };
 
     struct TextStats : public SpecificStats {
diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp
index 96f23972ffe..63ba241c644 100644
--- a/src/mongo/db/exec/update.cpp
+++ b/src/mongo/db/exec/update.cpp
@@ -36,6 +36,7 @@
 #include "mongo/db/concurrency/write_conflict_exception.h"
 #include "mongo/db/exec/scoped_timer.h"
 #include "mongo/db/exec/working_set_common.h"
+#include "mongo/db/global_environment_experiment.h"
 #include "mongo/db/ops/update_lifecycle.h"
 #include "mongo/db/query/explain.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
@@ -759,11 +760,14 @@ namespace mongo {
             WorkingSetMember* member = _ws->get(id);
 
             if (!member->hasLoc()) {
+                // We expect to be here because of an invalidation causing a force-fetch, and
+                // doc-locking storage engines do not issue invalidations.
+                dassert(!supportsDocLocking());
+
                 _ws->free(id);
-                const std::string errmsg = "update stage failed to read member w/ loc from child";
-                *out = WorkingSetCommon::allocateStatusMember(_ws, Status(ErrorCodes::InternalError,
-                                                                          errmsg));
-                return PlanStage::FAILURE;
+                ++_specificStats.nInvalidateSkips;
+                ++_commonStats.needTime;
+                return PlanStage::NEED_TIME;
             }
             loc = member->loc;