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

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 e0e2c3e20e9..469938a0b38 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -40,18 +40,24 @@ replica_sets_jscore_multiversion_passthrough:
   test_file: jstests/core/crud_ops_do_not_throw_locktimeout.js
 - ticket: SERVER-43614
   test_file: jstests/core/txns/new_transactions_on_session_with_prepared_txn_block_behind_prepare.js
+- ticket: SERVER-47469
+  test_file: jstests/core/apply_ops_system_dot_views.js
 
 sharding_jscore_multiversion_passthrough:
 - ticket: SERVER-47773
   test_file: jstests/core/geo_near_tailable.js
 - ticket: SERVER-43614
   test_file: jstests/core/crud_ops_do_not_throw_locktimeout.js
+- ticket: SERVER-47469
+  test_file: jstests/core/apply_ops_system_dot_views.js
 
 sharded_collections_jscore_multiversion_passthrough:
 - ticket: SERVER-47773
   test_file: jstests/core/geo_near_tailable.js
 - ticket: SERVER-43614
   test_file: jstests/core/crud_ops_do_not_throw_locktimeout.js
+- ticket: SERVER-47469
+  test_file: jstests/core/apply_ops_system_dot_views.js
 
 sharding_multiversion:
 - ticket: SERVER-43614
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 2a8f2ba5863..dcc15a0c87e 100644
--- a/jstests/libs/parallelTester.js
+++ b/jstests/libs/parallelTester.js
@@ -218,6 +218,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",
                 "update_pipeline_shell_helpers.js",
                 "update_with_pipeline.js",
                 "views/dbref_projection.js",
diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp
index dd6361c75f3..20fec3ab116 100644
--- a/src/mongo/db/catalog_raii.cpp
+++ b/src/mongo/db/catalog_raii.cpp
@@ -147,6 +147,10 @@ AutoGetCollection::AutoGetCollection(OperationContext* opCtx,
             !_view || viewMode == kViewsPermitted);
 }
 
+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 8f313913127..7a54a02cc04 100644
--- a/src/mongo/db/catalog_raii.h
+++ b/src/mongo/db/catalog_raii.h
@@ -161,6 +161,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 baa7adeee47..8750ef6fe38 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -278,10 +278,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 0027f545115..7e590f14580 100644
--- a/src/mongo/db/repl/apply_ops.cpp
+++ b/src/mongo/db/repl/apply_ops.cpp
@@ -230,7 +230,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/oplog_applier_impl.cpp b/src/mongo/db/repl/oplog_applier_impl.cpp
index 95d5237a969..39f7a1ac94a 100644
--- a/src/mongo/db/repl/oplog_applier_impl.cpp
+++ b/src/mongo/db/repl/oplog_applier_impl.cpp
@@ -133,10 +133,6 @@ Status finishAndLogApply(OperationContext* opCtx,
     return finalStatus;
 }
 
-LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
-    return nss.isSystemDotViews() ? MODE_X : mode;
-}
-
 /**
  * Caches per-collection properties which are relevant for oplog application, so that they don't
  * have to be retrieved repeatedly for each op.
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index f09894da4c8..4d3e2cc9e81 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -94,9 +94,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()