SERVER-27067 Remove unnecessary references to setLastOpToSystemLastOpTime

6 files changed, 23 insertions, 66 deletions

diff --git a/jstests/replsets/noop_writes_wait_for_write_concern.js b/jstests/replsets/noop_writes_wait_for_write_concern.js
index e525d68503b..61ffc518df4 100644
--- a/jstests/replsets/noop_writes_wait_for_write_concern.js
+++ b/jstests/replsets/noop_writes_wait_for_write_concern.js
@@ -48,6 +48,20 @@
     // }
     var commands = [];
 
+    commands.push({
+        req: {applyOps: [{op: "i", ns: coll.getFullName(), o: {_id: 1}}]},
+        setupFunc: function() {
+            assert.writeOK(coll.insert({_id: 1}));
+        },
+        confirmFunc: function(res) {
+            assert.commandWorked(res);
+            assert.eq(res.applied, 1);
+            assert.eq(res.results[0], true);
+            assert.eq(coll.find().itcount(), 1);
+            assert.eq(coll.count({_id: 1}), 1);
+        }
+    });
+
     // 'update' where the document to update does not exist.
     commands.push({
         req: {update: collName, updates: [{q: {a: 1}, u: {b: 2}}]},
diff --git a/src/mongo/db/commands/apply_ops_cmd.cpp b/src/mongo/db/commands/apply_ops_cmd.cpp
index 3c09e1110aa..504abd19079 100644
--- a/src/mongo/db/commands/apply_ops_cmd.cpp
+++ b/src/mongo/db/commands/apply_ops_cmd.cpp
@@ -287,22 +287,17 @@ public:
             }
         }
 
-        auto client = opCtx->getClient();
-        auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
-        ScopeGuard lastOpSetterGuard =
-            MakeObjGuard(repl::ReplClientInfo::forClient(client),
-                         &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
-                         opCtx);
-
+        // TODO (SERVER-30217): When a write concern is provided to the applyOps command, we
+        // normally wait on the OpTime of whichever operation successfully completed last. This is
+        // erroneous, however, if the last operation in the array happens to be a write no-op and
+        // thus isn’t assigned an OpTime. Let the second to last operation in the applyOps be write
+        // A, the last operation in applyOps be write B. Let B do a no-op write and let the
+        // operation that caused B to be a no-op be C. If C has an OpTime after A but before B,
+        // then we won’t wait for C to be replicated and it could be rolled back, even though B
+        // was acknowledged. To fix this, we should wait for replication of the node’s last applied
+        // OpTime if the last write operation was a no-op write.
         auto applyOpsStatus = appendCommandStatus(result, applyOps(opCtx, dbname, cmdObj, &result));
 
-        if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) {
-            // If this operation has already generated a new lastOp, don't bother setting it
-            // here. No-op applyOps will not generate a new lastOp, so we still need the guard to
-            // fire in that case.
-            lastOpSetterGuard.Dismiss();
-        }
-
         return applyOpsStatus;
     }
 
diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp
index 230ca733659..16b2724f3ba 100644
--- a/src/mongo/db/commands/create_indexes.cpp
+++ b/src/mongo/db/commands/create_indexes.cpp
@@ -291,12 +291,6 @@ public:
         const int numIndexesBefore = collection->getIndexCatalog()->numIndexesTotal(opCtx);
         result.append("numIndexesBefore", numIndexesBefore);
 
-        auto client = opCtx->getClient();
-        ScopeGuard lastOpSetterGuard =
-            MakeObjGuard(repl::ReplClientInfo::forClient(client),
-                         &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
-                         opCtx);
-
         MultiIndexBlock indexer(opCtx, collection);
         indexer.allowBackgroundBuilding();
         indexer.allowInterruption();
@@ -401,8 +395,6 @@ public:
 
         result.append("numIndexesAfter", collection->getIndexCatalog()->numIndexesTotal(opCtx));
 
-        lastOpSetterGuard.Dismiss();
-
         return true;
     }
 
diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp
index 8014c67ecd0..c11466cff22 100644
--- a/src/mongo/db/commands/find_and_modify.cpp
+++ b/src/mongo/db/commands/find_and_modify.cpp
@@ -360,18 +360,6 @@ public:
         if (shouldBypassDocumentValidationForCommand(cmdObj))
             maybeDisableValidation.emplace(opCtx);
 
-        auto client = opCtx->getClient();
-        auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
-        ScopeGuard lastOpSetterGuard =
-            MakeObjGuard(repl::ReplClientInfo::forClient(client),
-                         &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
-                         opCtx);
-
-        // If this is the local database, don't set last op.
-        if (dbName == "local") {
-            lastOpSetterGuard.Dismiss();
-        }
-
         auto curOp = CurOp::get(opCtx);
         OpDebug* opDebug = &curOp->debug();
 
@@ -577,13 +565,6 @@ public:
             return false;
         }
 
-        if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) {
-            // If this operation has already generated a new lastOp, don't bother setting it here.
-            // No-op updates will not generate a new lastOp, so we still need the guard to fire in
-            // that case.
-            lastOpSetterGuard.Dismiss();
-        }
-
         return true;
     }
 
diff --git a/src/mongo/db/ops/delete.cpp b/src/mongo/db/ops/delete.cpp
index 1ac92749e3f..d0c1e3e5fce 100644
--- a/src/mongo/db/ops/delete.cpp
+++ b/src/mongo/db/ops/delete.cpp
@@ -55,19 +55,11 @@ long long deleteObjects(OperationContext* opCtx,
     ParsedDelete parsedDelete(opCtx, &request);
     uassertStatusOK(parsedDelete.parseRequest());
 
-    auto client = opCtx->getClient();
-    auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
-
     auto exec = uassertStatusOK(
         getExecutorDelete(opCtx, &CurOp::get(opCtx)->debug(), collection, &parsedDelete));
 
     uassertStatusOK(exec->executePlan());
 
-    // No-ops need to reset lastOp in the client, for write concern.
-    if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
-        repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(opCtx);
-    }
-
     return DeleteStage::getNumDeleted(*exec);
 }
 
diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index 575df5d35df..9c661c03925 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -63,20 +63,9 @@ UpdateResult update(OperationContext* opCtx, Database* db, const UpdateRequest&
     // Explain should never use this helper.
     invariant(!request.isExplain());
 
-    auto client = opCtx->getClient();
-    auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
-    ScopeGuard lastOpSetterGuard = MakeObjGuard(repl::ReplClientInfo::forClient(client),
-                                                &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
-                                                opCtx);
-
     const NamespaceString& nsString = request.getNamespaceString();
     Collection* collection = db->getCollection(opCtx, nsString);
 
-    // If this is the local database, don't set last op.
-    if (db->name() == "local") {
-        lastOpSetterGuard.Dismiss();
-    }
-
     // The update stage does not create its own collection.  As such, if the update is
     // an upsert, create the collection that the update stage inserts into beforehand.
     if (!collection && request.isUpsert()) {
@@ -113,12 +102,6 @@ UpdateResult update(OperationContext* opCtx, Database* db, const UpdateRequest&
     auto exec = uassertStatusOK(getExecutorUpdate(opCtx, nullOpDebug, collection, &parsedUpdate));
 
     uassertStatusOK(exec->executePlan());
-    if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) {
-        // If this operation has already generated a new lastOp, don't bother setting it here.
-        // No-op updates will not generate a new lastOp, so we still need the guard to fire in that
-        // case.
-        lastOpSetterGuard.Dismiss();
-    }
 
     const UpdateStats* updateStats = UpdateStage::getUpdateStats(exec.get());