SERVER-23933 Remove OpDebug from ops/update.cpp::update()

11 files changed, 20 insertions, 32 deletions

diff --git a/src/mongo/db/dbhelpers.cpp b/src/mongo/db/dbhelpers.cpp
index 8b2416da36b..e0a5b6ffcd3 100644
--- a/src/mongo/db/dbhelpers.cpp
+++ b/src/mongo/db/dbhelpers.cpp
@@ -243,7 +243,6 @@ void Helpers::upsert(OperationContext* txn, const string& ns, const BSONObj& o,
     verify(e.type());
     BSONObj id = e.wrap();
 
-    OpDebug debug;
     OldClientContext context(txn, ns);
 
     const NamespaceString requestNs(ns);
@@ -256,11 +255,10 @@ void Helpers::upsert(OperationContext* txn, const string& ns, const BSONObj& o,
     UpdateLifecycleImpl updateLifecycle(requestNs);
     request.setLifecycle(&updateLifecycle);
 
-    update(txn, context.db(), request, &debug);
+    update(txn, context.db(), request);
 }
 
 void Helpers::putSingleton(OperationContext* txn, const char* ns, BSONObj obj) {
-    OpDebug debug;
     OldClientContext context(txn, ns);
 
     const NamespaceString requestNs(ns);
@@ -271,7 +269,7 @@ void Helpers::putSingleton(OperationContext* txn, const char* ns, BSONObj obj) {
     UpdateLifecycleImpl updateLifecycle(requestNs);
     request.setLifecycle(&updateLifecycle);
 
-    update(txn, context.db(), request, &debug);
+    update(txn, context.db(), request);
 
     CurOp::get(txn)->done();
 }
diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp
index 003bf497721..b06197460ac 100644
--- a/src/mongo/db/exec/update.cpp
+++ b/src/mongo/db/exec/update.cpp
@@ -1031,6 +1031,7 @@ const UpdateStats* UpdateStage::getUpdateStats(const PlanExecutor* exec) {
 }
 
 void UpdateStage::recordUpdateStatsInOpDebug(const UpdateStats* updateStats, OpDebug* opDebug) {
+    invariant(opDebug);
     opDebug->nMatched = updateStats->nMatched;
     opDebug->nModified = updateStats->nModified;
     opDebug->upsert = updateStats->inserted;
diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index 82a816e5b3c..d5b83ae666b 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -57,10 +57,7 @@
 
 namespace mongo {
 
-UpdateResult update(OperationContext* txn,
-                    Database* db,
-                    const UpdateRequest& request,
-                    OpDebug* opDebug) {
+UpdateResult update(OperationContext* txn, Database* db, const UpdateRequest& request) {
     invariant(db);
 
     // Explain should never use this helper.
@@ -113,8 +110,9 @@ UpdateResult update(OperationContext* txn,
     ParsedUpdate parsedUpdate(txn, &request);
     uassertStatusOK(parsedUpdate.parseRequest());
 
+    OpDebug* const nullOpDebug = nullptr;
     std::unique_ptr<PlanExecutor> exec =
-        uassertStatusOK(getExecutorUpdate(txn, opDebug, collection, &parsedUpdate));
+        uassertStatusOK(getExecutorUpdate(txn, nullOpDebug, collection, &parsedUpdate));
 
     uassertStatusOK(exec->executePlan());
     if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) {
@@ -124,11 +122,7 @@ UpdateResult update(OperationContext* txn,
         lastOpSetterGuard.Dismiss();
     }
 
-    PlanSummaryStats summaryStats;
-    Explain::getSummaryStats(*exec, &summaryStats);
     const UpdateStats* updateStats = UpdateStage::getUpdateStats(exec.get());
-    UpdateStage::recordUpdateStatsInOpDebug(updateStats, opDebug);
-    opDebug->setPlanSummaryMetrics(summaryStats);
 
     return UpdateStage::makeUpdateResult(updateStats);
 }
diff --git a/src/mongo/db/ops/update.h b/src/mongo/db/ops/update.h
index 06cec7fa90f..ff21054e25a 100644
--- a/src/mongo/db/ops/update.h
+++ b/src/mongo/db/ops/update.h
@@ -47,10 +47,7 @@ class UpdateDriver;
  *
  * Caller must hold the appropriate database locks.
  */
-UpdateResult update(OperationContext* txn,
-                    Database* db,
-                    const UpdateRequest& request,
-                    OpDebug* opDebug);
+UpdateResult update(OperationContext* txn, Database* db, const UpdateRequest& request);
 
 /**
  * takes the from document and returns a new document
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 7bb171ecbd5..c43aa2da372 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -800,7 +800,9 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorUpdate(OperationContext* txn,
     }
 
     // TODO: This seems a bit circuitious.
-    opDebug->updateobj = request->getUpdates();
+    if (opDebug) {
+        opDebug->updateobj = request->getUpdates();
+    }
 
     // If this is a user-issued update, then we want to return an error: you cannot perform
     // writes on a secondary. If this is an update to a secondary from the replication system,
diff --git a/src/mongo/db/query/get_executor.h b/src/mongo/db/query/get_executor.h
index f96203b7fea..0488aeccf7d 100644
--- a/src/mongo/db/query/get_executor.h
+++ b/src/mongo/db/query/get_executor.h
@@ -135,6 +135,8 @@ StatusWith<std::unique_ptr<PlanExecutor>> getExecutorCount(OperationContext* txn
  * and delete flags like 'isMulti'. The caller must hold the appropriate MODE_X or MODE_IX
  * locks, and must not release these locks until after the returned PlanExecutor is deleted.
  *
+ * 'opDebug' Optional argument. When not null, will be used to record operation statistics.
+ *
  * The returned PlanExecutor will used the YieldPolicy returned by parsedDelete->yieldPolicy().
  *
  * Does not take ownership of its arguments.
@@ -155,6 +157,8 @@ StatusWith<std::unique_ptr<PlanExecutor>> getExecutorDelete(OperationContext* tx
  * to calling this function, and must not release these locks until after the returned
  * PlanExecutor is deleted.
  *
+ * 'opDebug' Optional argument. When not null, will be used to record operation statistics.
+ *
  * The returned PlanExecutor will used the YieldPolicy returned by parsedUpdate->yieldPolicy().
  *
  * Does not take ownership of its arguments.
diff --git a/src/mongo/db/repl/master_slave.cpp b/src/mongo/db/repl/master_slave.cpp
index 4aa6ed40159..a368b4b91c6 100644
--- a/src/mongo/db/repl/master_slave.cpp
+++ b/src/mongo/db/repl/master_slave.cpp
@@ -218,8 +218,6 @@ void ReplSource::save(OperationContext* txn) {
     LOG(1) << "Saving repl source: " << o << endl;
 
     {
-        OpDebug debug;
-
         OldClientContext ctx(txn, "local.sources", false);
 
         const NamespaceString requestNs("local.sources");
@@ -229,7 +227,7 @@ void ReplSource::save(OperationContext* txn) {
         request.setUpdates(o);
         request.setUpsert();
 
-        UpdateResult res = update(txn, ctx.db(), request, &debug);
+        UpdateResult res = update(txn, ctx.db(), request);
 
         verify(!res.modifiers);
         verify(res.numMatched == 1 || !res.upserted.isEmpty());
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 71b2e1b9e82..b48843f0119 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -862,7 +862,6 @@ Status applyOperation_inlock(OperationContext* txn,
                 // Do update on DuplicateKey errors.
                 // This will only be on the _id field in replication,
                 // since we disable non-_id unique constraint violations.
-                OpDebug debug;
                 BSONObjBuilder b;
                 b.append(o.getField("_id"));
 
@@ -875,7 +874,7 @@ Status applyOperation_inlock(OperationContext* txn,
                 UpdateLifecycleImpl updateLifecycle(requestNs);
                 request.setLifecycle(&updateLifecycle);
 
-                UpdateResult res = update(txn, db, request, &debug);
+                UpdateResult res = update(txn, db, request);
                 if (res.numMatched == 0 && res.upserted.isEmpty()) {
                     error() << "No document was updated even though we got a DuplicateKey "
                                "error when inserting";
@@ -889,7 +888,6 @@ Status applyOperation_inlock(OperationContext* txn,
     } else if (*opType == 'u') {
         opCounters->gotUpdate();
 
-        OpDebug debug;
         BSONObj updateCriteria = o2;
         const bool upsert = valueB || convertUpdateToUpsert;
 
@@ -906,7 +904,7 @@ Status applyOperation_inlock(OperationContext* txn,
         UpdateLifecycleImpl updateLifecycle(requestNs);
         request.setLifecycle(&updateLifecycle);
 
-        UpdateResult ur = update(txn, db, request, &debug);
+        UpdateResult ur = update(txn, db, request);
 
         if (ur.numMatched == 0 && ur.upserted.isEmpty()) {
             if (ur.modifiers) {
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index 02c3323fa76..bb5531dbf34 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -742,7 +742,6 @@ void syncFixUp(OperationContext* txn,
                     }
                 } else {
                     // TODO faster...
-                    OpDebug debug;
                     updates++;
 
                     const NamespaceString requestNs(doc.ns);
@@ -755,7 +754,7 @@ void syncFixUp(OperationContext* txn,
                     UpdateLifecycleImpl updateLifecycle(requestNs);
                     request.setLifecycle(&updateLifecycle);
 
-                    update(txn, ctx.db(), request, &debug);
+                    update(txn, ctx.db(), request);
                 }
             } catch (const DBException& e) {
                 log() << "exception in rollback ns:" << doc.ns << ' ' << pattern.toString() << ' '
diff --git a/src/mongo/db/s/sharding_state.cpp b/src/mongo/db/s/sharding_state.cpp
index 85c8916b4d0..0d89f81f678 100644
--- a/src/mongo/db/s/sharding_state.cpp
+++ b/src/mongo/db/s/sharding_state.cpp
@@ -974,12 +974,10 @@ Status ShardingState::updateShardIdentityConfigString(OperationContext* txn,
     UpdateLifecycleImpl updateLifecycle(NamespaceString::kConfigCollectionNamespace);
     updateReq.setLifecycle(&updateLifecycle);
 
-    OpDebug opDebug;
-
     try {
         AutoGetOrCreateDb autoDb(txn, NamespaceString::kConfigCollectionNamespace.db(), MODE_X);
 
-        auto result = update(txn, autoDb.getDb(), updateReq, &opDebug);
+        auto result = update(txn, autoDb.getDb(), updateReq);
         if (result.numMatched == 0) {
             warning() << "failed to update config string of shard identity document because "
                       << "it does not exist. This shard could have been removed from the cluster";
diff --git a/src/mongo/db/s/sharding_state_recovery.cpp b/src/mongo/db/s/sharding_state_recovery.cpp
index 85d4fd3698e..1574f611403 100644
--- a/src/mongo/db/s/sharding_state_recovery.cpp
+++ b/src/mongo/db/s/sharding_state_recovery.cpp
@@ -192,7 +192,6 @@ Status modifyRecoveryDocument(OperationContext* txn,
 
         LOG(1) << "Changing sharding recovery document " << updateObj;
 
-        OpDebug opDebug;
         UpdateRequest updateReq(NamespaceString::kConfigCollectionNamespace);
         updateReq.setQuery(RecoveryDocument::getQuery());
         updateReq.setUpdates(updateObj);
@@ -200,7 +199,7 @@ Status modifyRecoveryDocument(OperationContext* txn,
         UpdateLifecycleImpl updateLifecycle(NamespaceString::kConfigCollectionNamespace);
         updateReq.setLifecycle(&updateLifecycle);
 
-        UpdateResult result = update(txn, autoGetOrCreateDb->getDb(), updateReq, &opDebug);
+        UpdateResult result = update(txn, autoGetOrCreateDb->getDb(), updateReq);
         invariant(result.numDocsModified == 1 || !result.upserted.isEmpty());
         invariant(result.numMatched <= 1);