SERVER-47469 Upgrade lock mode for applyOps + system.views

(cherry picked from commit bd74e903563e9c12bbaf3a027a8dc39ae6a1613d)
(cherry picked from commit bf7268ee79e5561032114b37cc0fcadf72dc8e54)

9 files changed, 59 insertions, 12 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 988850040d7..d34615984dd 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -87,6 +87,8 @@ suites:
   concurrency_sharded_replication_multiversion:
 
   replica_sets_jscore_multiversion_passthrough:
+  - ticket: SERVER-47469
+    test_file: jstests/core/apply_ops_system_dot_views.js
 
   replica_sets_multiversion:
   - ticket: SERVER-48518
@@ -97,5 +99,9 @@ suites:
   sharding_multiversion:
 
   sharding_jscore_multiversion_passthrough:
+  - ticket: SERVER-47469
+    test_file: jstests/core/apply_ops_system_dot_views.js
 
   sharded_collections_jscore_multiversion_passthrough:
+  - ticket: SERVER-47469
+    test_file: jstests/core/apply_ops_system_dot_views.js
diff --git a/jstests/core/apply_ops_system_dot_views.js b/jstests/core/apply_ops_system_dot_views.js
new file mode 100644
index 00000000000..da9d09760ba
--- /dev/null
+++ b/jstests/core/apply_ops_system_dot_views.js
@@ -0,0 +1,41 @@
+// Tests that applyOps can include operations on the system.views namespace.
+// @tags: [
+//   assumes_superuser_permissions,
+//   # Views don't support OP_QUERY
+//   requires_find_command,
+//   requires_non_retryable_commands,
+//   # applyOps uses the oplog that require replication support
+//   requires_replication,
+// ]
+(function() {
+"use strict";
+
+const backingColl = db.apply_ops_system_dot_views_backing;
+backingColl.drop();
+const view = db.apply_ops_view;
+view.drop();
+
+// Make sure system.views exists before we try to insert to it with applyOps.
+db["unused_apply_ops_view"].drop();
+assert.commandWorked(
+    db.runCommand({create: "unused_apply_ops_view", viewOn: backingColl.getName(), pipeline: []}));
+
+assert.commandWorked(backingColl.insert([{a: 0}, {a: 1}]));
+const ops = [{
+    "ts": new Timestamp(1585942982, 2),
+    "t": new NumberLong("1"),
+    "h": new NumberLong("0"),
+    "v": new NumberInt("2"),
+    "op": "i",
+    "ns": db["system.views"].getFullName(),
+    "wall": new Date(1585942982442),
+    "o": {
+        "_id": view.getFullName(),
+        "viewOn": backingColl.getName(),
+        "pipeline": [{"$match": {"a": 0}}]
+    }
+}];
+
+assert.commandWorked(db.adminCommand({applyOps: ops}));
+assert.eq(view.find({}, {_id: 0, a: 1}).toArray(), [{a: 0}]);
+}());
diff --git a/jstests/libs/parallelTester.js b/jstests/libs/parallelTester.js
index b046964a0d2..a5b06b31bde 100644
--- a/jstests/libs/parallelTester.js
+++ b/jstests/libs/parallelTester.js
@@ -216,6 +216,7 @@ if (typeof _threadInject != "undefined") {
         // The following tests cannot run when shell readMode is legacy.
         if (db.getMongo().readMode() === "legacy") {
             var requires_find_command = [
+                "apply_ops_system_dot_views.js",
                 "explode_for_sort_collation.js",
                 "update_pipeline_shell_helpers.js",
                 "update_with_pipeline.js",
diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp
index 8318826971c..7f3847c5a6a 100644
--- a/src/mongo/db/catalog_raii.cpp
+++ b/src/mongo/db/catalog_raii.cpp
@@ -164,6 +164,10 @@ NamespaceString AutoGetCollection::resolveNamespaceStringOrUUID(OperationContext
     return *resolvedNss;
 }
 
+LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
+    return nss.isSystemDotViews() ? MODE_X : mode;
+}
+
 AutoGetOrCreateDb::AutoGetOrCreateDb(OperationContext* opCtx,
                                      StringData dbName,
                                      LockMode mode,
diff --git a/src/mongo/db/catalog_raii.h b/src/mongo/db/catalog_raii.h
index 5461d4af7e9..392fa6077b7 100644
--- a/src/mongo/db/catalog_raii.h
+++ b/src/mongo/db/catalog_raii.h
@@ -154,6 +154,11 @@ private:
 };
 
 /**
+ * Writes to system.views need to use a stronger lock to prevent inconsistencies like view cycles.
+ */
+LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode);
+
+/**
  * RAII-style class, which acquires a lock on the specified database in the requested mode and
  * obtains a reference to the database, creating it was non-existing. Used as a shortcut for
  * calls to DatabaseHolder::get(opCtx)->openDb(), taking care of locking details. The
diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp
index 324caf762ab..9e457ebb0e5 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -282,10 +282,6 @@ bool handleError(OperationContext* opCtx,
     return !wholeOp.getOrdered();
 }
 
-LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
-    return nss.isSystemDotViews() ? MODE_X : mode;
-}
-
 void insertDocuments(OperationContext* opCtx,
                      Collection* collection,
                      std::vector<InsertStatement>::iterator begin,
diff --git a/src/mongo/db/repl/apply_ops.cpp b/src/mongo/db/repl/apply_ops.cpp
index 675573b8a38..78a5dc8e5e7 100644
--- a/src/mongo/db/repl/apply_ops.cpp
+++ b/src/mongo/db/repl/apply_ops.cpp
@@ -218,7 +218,8 @@ Status _applyOps(OperationContext* opCtx,
                             return Status::OK();
                         }
 
-                        AutoGetCollection autoColl(opCtx, nss, MODE_IX);
+                        AutoGetCollection autoColl(
+                            opCtx, nss, fixLockModeForSystemDotViewsChanges(nss, MODE_IX));
                         if (!autoColl.getCollection()) {
                             // For idempotency reasons, return success on delete operations.
                             if (*opType == 'd') {
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index cac6bcd7eb3..693f2199c70 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -91,9 +91,6 @@ using UniqueLock = stdx::unique_lock<Latch>;
 
 const auto kIdIndexName = "_id_"_sd;
 
-LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
-    return nss.isSystemDotViews() ? MODE_X : mode;
-}
 }  // namespace
 
 StorageInterfaceImpl::StorageInterfaceImpl()
diff --git a/src/mongo/db/repl/sync_tail.cpp b/src/mongo/db/repl/sync_tail.cpp
index 115fb9e4259..bdcccdfd384 100644
--- a/src/mongo/db/repl/sync_tail.cpp
+++ b/src/mongo/db/repl/sync_tail.cpp
@@ -270,10 +270,6 @@ Status finishAndLogApply(ClockSource* clockSource,
     return finalStatus;
 }
 
-LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
-    return nss.isSystemDotViews() ? MODE_X : mode;
-}
-
 }  // namespace
 
 // static