SERVER-18522 set lastOp to last system optime, on no-op updates or deletes

9 files changed, 92 insertions, 15 deletions

diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp
index 9b9277cafdc..74c4eeb7a54 100644
--- a/src/mongo/db/commands/create_indexes.cpp
+++ b/src/mongo/db/commands/create_indexes.cpp
@@ -46,6 +46,7 @@
 #include "mongo/db/operation_context_impl.h"
 #include "mongo/db/op_observer.h"
 #include "mongo/db/ops/insert.h"
+#include "mongo/db/repl/repl_client_info.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/db/s/collection_metadata.h"
 #include "mongo/db/s/sharding_state.h"
@@ -187,6 +188,8 @@ public:
         if (specs.size() == 0) {
             result.append("numIndexesAfter", numIndexesBefore);
             result.append("note", "all indexes already exist");
+            // No-ops need to reset lastOp in the client, for write concern.
+            repl::ReplClientInfo::forClient(txn->getClient()).setLastOpToSystemLastOpTime(txn);
             return true;
         }
 
@@ -309,6 +312,5 @@ private:
 
         return Status::OK();
     }
-
 } cmdCreateIndex;
 }
diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp
index 2cc0887ae9d..bfd5102f221 100644
--- a/src/mongo/db/commands/find_and_modify.cpp
+++ b/src/mongo/db/commands/find_and_modify.cpp
@@ -344,6 +344,7 @@ public:
             maybeDisableValidation.emplace(txn);
 
         auto client = txn->getClient();
+        auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
 
         // We may encounter a WriteConflictException when creating a collection during an
         // upsert, even when holding the exclusive lock on the database (due to other load on
@@ -465,6 +466,11 @@ public:
         }
         MONGO_WRITE_CONFLICT_RETRY_LOOP_END(txn, "findAndModify", nsString.ns());
 
+        // No-ops need to reset lastOp in the client, for write concern.
+        if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
+            repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(txn);
+        }
+
         WriteConcernResult res;
         auto waitForWCStatus = waitForWriteConcern(
             txn, repl::ReplClientInfo::forClient(txn->getClient()).getLastOp(), &res);
diff --git a/src/mongo/db/commands/write_commands/batch_executor.cpp b/src/mongo/db/commands/write_commands/batch_executor.cpp
index e491a29e41b..f87912a1c6c 100644
--- a/src/mongo/db/commands/write_commands/batch_executor.cpp
+++ b/src/mongo/db/commands/write_commands/batch_executor.cpp
@@ -1144,6 +1144,9 @@ static void multiUpdate(OperationContext* txn,
     // Updates from the write commands path can yield.
     request.setYieldPolicy(PlanExecutor::YIELD_AUTO);
 
+    auto client = txn->getClient();
+    auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
+
     int attempt = 0;
     bool createCollection = false;
     for (int fakeLoop = 0; fakeLoop < 1; fakeLoop++) {
@@ -1257,6 +1260,12 @@ static void multiUpdate(OperationContext* txn,
             result->getStats().nModified = didInsert ? 0 : numDocsModified;
             result->getStats().n = didInsert ? 1 : numMatched;
             result->getStats().upsertedID = resUpsertedID;
+
+            // No-ops need to reset lastOp in the client, for write concern.
+            if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
+                repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(txn);
+            }
+
         } catch (const WriteConflictException& dle) {
             debug->writeConflicts++;
             if (isMulti) {
@@ -1303,6 +1312,9 @@ static void multiRemove(OperationContext* txn,
     // Deletes running through the write commands path can yield.
     request.setYieldPolicy(PlanExecutor::YIELD_AUTO);
 
+    auto client = txn->getClient();
+    auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
+
     int attempt = 1;
     while (1) {
         try {
@@ -1341,6 +1353,11 @@ static void multiRemove(OperationContext* txn,
             uassertStatusOK(exec->executePlan());
             result->getStats().n = DeleteStage::getNumDeleted(*exec);
 
+            // No-ops need to reset lastOp in the client, for write concern.
+            if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
+                repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(txn);
+            }
+
             break;
         } catch (const WriteConflictException& dle) {
             CurOp::get(txn)->debug().writeConflicts++;
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index 11c5938da7d..93638a041dd 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -76,6 +76,7 @@
 #include "mongo/db/query/find.h"
 #include "mongo/db/query/get_executor.h"
 #include "mongo/db/repl/oplog.h"
+#include "mongo/db/repl/repl_client_info.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/db/s/sharded_connection_info.h"
 #include "mongo/db/service_context.h"
@@ -634,6 +635,8 @@ void receivedUpdate(OperationContext* txn, const NamespaceString& nsString, Mess
     uassertStatusOK(userAllowedWriteNS(nsString));
     int flags = d.pullInt();
     BSONObj query = d.nextJsObj();
+    auto client = txn->getClient();
+    auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
 
     verify(d.moreJSObjs());
     verify(query.objsize() < m.header().dataLen());
@@ -647,15 +650,14 @@ void receivedUpdate(OperationContext* txn, const NamespaceString& nsString, Mess
 
     op.debug().query = query;
     {
-        stdx::lock_guard<Client> lk(*txn->getClient());
+        stdx::lock_guard<Client> lk(*client);
         op.setNS_inlock(nsString.ns());
         op.setQuery_inlock(query);
     }
 
-    Status status = AuthorizationSession::get(txn->getClient())
-                        ->checkAuthForUpdate(nsString, query, toupdate, upsert);
-    audit::logUpdateAuthzCheck(
-        txn->getClient(), nsString, query, toupdate, upsert, multi, status.code());
+    Status status =
+        AuthorizationSession::get(client)->checkAuthForUpdate(nsString, query, toupdate, upsert);
+    audit::logUpdateAuthzCheck(client, nsString, query, toupdate, upsert, multi, status.code());
     uassertStatusOK(status);
 
     UpdateRequest request(nsString);
@@ -695,8 +697,13 @@ void receivedUpdate(OperationContext* txn, const NamespaceString& nsString, Mess
                 UpdateResult res = UpdateStage::makeUpdateResult(*exec, &op.debug());
 
                 // for getlasterror
-                LastError::get(txn->getClient())
-                    .recordUpdate(res.existing, res.numMatched, res.upserted);
+                LastError::get(client).recordUpdate(res.existing, res.numMatched, res.upserted);
+
+                // No-ops need to reset lastOp in the client, for write concern.
+                if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
+                    repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(txn);
+                }
+
                 return;
             }
             break;
@@ -741,7 +748,12 @@ void receivedUpdate(OperationContext* txn, const NamespaceString& nsString, Mess
         uassertStatusOK(exec->executePlan());
         UpdateResult res = UpdateStage::makeUpdateResult(*exec, &op.debug());
 
-        LastError::get(txn->getClient()).recordUpdate(res.existing, res.numMatched, res.upserted);
+        LastError::get(client).recordUpdate(res.existing, res.numMatched, res.upserted);
+
+        // No-ops need to reset lastOp in the client, for write concern.
+        if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
+            repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(txn);
+        }
     }
     MONGO_WRITE_CONFLICT_RETRY_LOOP_END(txn, "update", nsString.ns());
 }
@@ -755,16 +767,18 @@ void receivedDelete(OperationContext* txn, const NamespaceString& nsString, Mess
     verify(d.moreJSObjs());
     BSONObj pattern = d.nextJsObj();
 
+    auto client = txn->getClient();
+    auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
+
     op.debug().query = pattern;
     {
-        stdx::lock_guard<Client> lk(*txn->getClient());
+        stdx::lock_guard<Client> lk(*client);
         op.setQuery_inlock(pattern);
         op.setNS_inlock(nsString.ns());
     }
 
-    Status status =
-        AuthorizationSession::get(txn->getClient())->checkAuthForDelete(nsString, pattern);
-    audit::logDeleteAuthzCheck(txn->getClient(), nsString, pattern, status.code());
+    Status status = AuthorizationSession::get(client)->checkAuthForDelete(nsString, pattern);
+    audit::logDeleteAuthzCheck(client, nsString, pattern, status.code());
     uassertStatusOK(status);
 
     DeleteRequest request(nsString);
@@ -795,9 +809,14 @@ void receivedDelete(OperationContext* txn, const NamespaceString& nsString, Mess
             // Run the plan and get the number of docs deleted.
             uassertStatusOK(exec->executePlan());
             long long n = DeleteStage::getNumDeleted(*exec);
-            LastError::get(txn->getClient()).recordDelete(n);
+            LastError::get(client).recordDelete(n);
             op.debug().ndeleted = n;
 
+            // No-ops need to reset lastOp in the client, for write concern.
+            if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) {
+                repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(txn);
+            }
+
             break;
         } catch (const WriteConflictException& dle) {
             op.debug().writeConflicts++;
diff --git a/src/mongo/db/ops/delete.cpp b/src/mongo/db/ops/delete.cpp
index 5cf5e113cd4..4cc01162bdd 100644
--- a/src/mongo/db/ops/delete.cpp
+++ b/src/mongo/db/ops/delete.cpp
@@ -67,6 +67,10 @@ long long deleteObjects(OperationContext* txn,
     ParsedDelete parsedDelete(txn, &request);
     uassertStatusOK(parsedDelete.parseRequest());
 
+    // Replicated writes are disallowed with deleteObjects, as we are not properly setting
+    // lastOp for no-op deletes.
+    fassert(22001, !txn->writesAreReplicated());
+
     std::unique_ptr<PlanExecutor> exec =
         uassertStatusOK(getExecutorDelete(txn, collection, &parsedDelete));
 
diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index 80c20fecd91..645fa6d26c8 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -48,6 +48,7 @@
 #include "mongo/db/ops/update_lifecycle.h"
 #include "mongo/db/query/explain.h"
 #include "mongo/db/query/get_executor.h"
+#include "mongo/db/repl/repl_client_info.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/db/update_index_data.h"
 #include "mongo/util/log.h"
@@ -63,6 +64,10 @@ UpdateResult update(OperationContext* txn,
     // Explain should never use this helper.
     invariant(!request.isExplain());
 
+    auto client = txn->getClient();
+    auto lastOpHolder = repl::ReplClientInfo::forClient(client);
+    auto lastOpAtOperationStart = lastOpHolder.getLastOp();
+
     const NamespaceString& nsString = request.getNamespaceString();
     Collection* collection = db->getCollection(nsString.ns());
 
@@ -103,6 +108,12 @@ UpdateResult update(OperationContext* txn,
         uassertStatusOK(getExecutorUpdate(txn, collection, &parsedUpdate, opDebug));
 
     uassertStatusOK(exec->executePlan());
+
+    // No-ops need to reset lastOp in the client, for write concern.
+    if (lastOpHolder.getLastOp() == lastOpAtOperationStart) {
+        lastOpHolder.setLastOpToSystemLastOpTime(txn);
+    }
+
     return UpdateStage::makeUpdateResult(*exec, opDebug);
 }
 
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 67ee2e3d1d6..a499f64ed5b 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -167,7 +167,6 @@ std::pair<OpTime, long long> getNextOpTime(OperationContext* txn,
     }
 
     OpTime opTime(ts, term);
-    replCoord->setMyLastOptime(opTime);
     return std::pair<OpTime, long long>(opTime, hashNew);
 }
 
@@ -315,8 +314,11 @@ void _logOp(OperationContext* txn,
     BSONObj partial = b.done();
 
     OplogDocWriter writer(partial, obj);
+    // This transaction might roll back.
     checkOplogInsert(_localOplogCollection->insertDocument(txn, &writer, false));
 
+    // Set this only after we're sure the insert didn't abort and roll back.
+    replCoord->setMyLastOptime(slot.first);
     ReplClientInfo::forClient(txn->getClient()).setLastOp(slot.first);
 }
 
diff --git a/src/mongo/db/repl/repl_client_info.cpp b/src/mongo/db/repl/repl_client_info.cpp
index 18191bfaffc..e4c6ddbdbed 100644
--- a/src/mongo/db/repl/repl_client_info.cpp
+++ b/src/mongo/db/repl/repl_client_info.cpp
@@ -33,6 +33,7 @@
 #include "mongo/base/init.h"
 #include "mongo/db/client.h"
 #include "mongo/db/jsobj.h"
+#include "mongo/db/operation_context.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/util/decorable.h"
 
@@ -49,5 +50,12 @@ long long ReplClientInfo::getTerm() {
     return _cachedTerm;
 }
 
+void ReplClientInfo::setLastOpToSystemLastOpTime(OperationContext* txn) {
+    ReplicationCoordinator* replCoord = repl::ReplicationCoordinator::get(txn->getServiceContext());
+    if (replCoord->isReplEnabled()) {
+        setLastOp(replCoord->getMyLastOptime());
+    }
+}
+
 }  // namespace repl
 }  // namespace mongo
diff --git a/src/mongo/db/repl/repl_client_info.h b/src/mongo/db/repl/repl_client_info.h
index 576142fd9cd..806dfeed9cb 100644
--- a/src/mongo/db/repl/repl_client_info.h
+++ b/src/mongo/db/repl/repl_client_info.h
@@ -36,6 +36,7 @@ namespace mongo {
 
 class BSONObjBuilder;
 class Client;
+class OperationContext;
 
 namespace repl {
 
@@ -68,6 +69,13 @@ public:
     // in the case of yielding.
     long long getTerm();
 
+    /**
+     * Use this to set the LastOp to the latest known OpTime in the oplog.
+     * This is necessary when doing no-op writes, as we need to set the client's lastOp to a proper
+     * value for write concern wait to work.
+     */
+    void setLastOpToSystemLastOpTime(OperationContext* txn);
+
 private:
     static const long long kUninitializedTerm = -1;