SERVER-37283 Use stronger locks for system.views

Readers of the view catalog depend on a MODE_IS DB lock preventing
concurrent writes to the view catalog. This is true for regular view
maintenance commands like collMod, create, and drop. However, on
secondaries these commands are replicated as direct writes to
system.views and do not hold as strong of a lock. Further, a user is
permitted to write directly to system.views and so could hit a similar
issue on the primary.

25 files changed, 198 insertions, 231 deletions

diff --git a/buildscripts/resmokeconfig/suites/concurrency_replication_multi_stmt_txn.yml b/buildscripts/resmokeconfig/suites/concurrency_replication_multi_stmt_txn.yml
index 415d5bdfcd4..cc33048a6e1 100644
--- a/buildscripts/resmokeconfig/suites/concurrency_replication_multi_stmt_txn.yml
+++ b/buildscripts/resmokeconfig/suites/concurrency_replication_multi_stmt_txn.yml
@@ -23,6 +23,9 @@ selector:
   # of a transaction (e.g. ErrorCodes.LockTimeout).
   - jstests/concurrency/fsm_workloads/drop_index_during_replan.js
 
+  # Performs direct writes to system.views
+  - jstests/concurrency/fsm_workloads/view_catalog_direct_system_writes.js
+
   exclude_with_any_tags:
   - requires_sharding
   # Sharing cursors between state functions will fail in this suite because it will attempt to use
diff --git a/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns.yml b/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns.yml
index 3676e0e1d31..0e4acd4efa7 100644
--- a/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns.yml
+++ b/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns.yml
@@ -106,8 +106,6 @@ selector:
   # collection.
   - jstests/concurrency/fsm_workloads/kill_aggregation.js
   - jstests/concurrency/fsm_workloads/kill_rooted_or.js
-  - jstests/concurrency/fsm_workloads/view_catalog_cycle_with_drop.js
-  - jstests/concurrency/fsm_workloads/view_catalog.js
 
   # Uses getmores.
   - jstests/concurrency/fsm_workloads/agg_base.js
@@ -151,8 +149,11 @@ selector:
   - jstests/concurrency/fsm_workloads/agg_sort.js
   - jstests/concurrency/fsm_workloads/collmod.js
   - jstests/concurrency/fsm_workloads/collmod_separate_collections.js
-  - jstests/concurrency/fsm_workloads/kill_multicollection_aggregation.js
   - jstests/concurrency/fsm_workloads/invalidated_cursors.js
+  - jstests/concurrency/fsm_workloads/kill_multicollection_aggregation.js
+  - jstests/concurrency/fsm_workloads/view_catalog.js
+  - jstests/concurrency/fsm_workloads/view_catalog_cycle_with_drop.js
+  - jstests/concurrency/fsm_workloads/view_catalog_direct_system_writes.js
 
   # The auto_retry_on_network_error.js override needs to overwrite the response from drop on
   # NamespaceNotFound, and since this workload only creates and drops collections there isn't
diff --git a/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns_and_balancer.yml b/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns_and_balancer.yml
index 74304822cd7..8560af6de3c 100644
--- a/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns_and_balancer.yml
+++ b/buildscripts/resmokeconfig/suites/concurrency_sharded_with_stepdowns_and_balancer.yml
@@ -109,8 +109,6 @@ selector:
   # collection.
   - jstests/concurrency/fsm_workloads/kill_aggregation.js
   - jstests/concurrency/fsm_workloads/kill_rooted_or.js
-  - jstests/concurrency/fsm_workloads/view_catalog_cycle_with_drop.js
-  - jstests/concurrency/fsm_workloads/view_catalog.js
 
   # Uses getmores.
   - jstests/concurrency/fsm_workloads/agg_base.js
@@ -153,8 +151,11 @@ selector:
   - jstests/concurrency/fsm_workloads/agg_sort.js
   - jstests/concurrency/fsm_workloads/collmod.js
   - jstests/concurrency/fsm_workloads/collmod_separate_collections.js
-  - jstests/concurrency/fsm_workloads/kill_multicollection_aggregation.js
   - jstests/concurrency/fsm_workloads/invalidated_cursors.js
+  - jstests/concurrency/fsm_workloads/kill_multicollection_aggregation.js
+  - jstests/concurrency/fsm_workloads/view_catalog.js
+  - jstests/concurrency/fsm_workloads/view_catalog_cycle_with_drop.js
+  - jstests/concurrency/fsm_workloads/view_catalog_direct_system_writes.js
 
   # The auto_retry_on_network_error.js override needs to overwrite the response from drop on
   # NamespaceNotFound, and since this workload only creates and drops collections there isn't
diff --git a/buildscripts/resmokeconfig/suites/multi_shard_multi_stmt_txn_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/multi_shard_multi_stmt_txn_jscore_passthrough.yml
index 6d30752f79d..1ba72b4bc5c 100644
--- a/buildscripts/resmokeconfig/suites/multi_shard_multi_stmt_txn_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/multi_shard_multi_stmt_txn_jscore_passthrough.yml
@@ -179,6 +179,9 @@ selector:
   # TODO: SERVER-38690 Mongos should throw write targeting errors in a transaction.
   - jstests/core/update_setOnInsert.js
   
+  # Reads from system.views.
+  - jstests/core/views/views_drop.js
+
   ##
   ## Some aggregation stages don't support snapshot readconcern.
   ##
diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_jscore_passthrough.yml
index 7b9234501cc..334ab02f41c 100644
--- a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_jscore_passthrough.yml
@@ -79,6 +79,9 @@ selector:
   # Windows debug builds than other builds.
   - jstests/core/insert1.js
 
+  # Reads from system.views.
+  - jstests/core/views/views_drop.js
+
   ##
   ## Some aggregation stages don't support snapshot readconcern.
   ##
diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml
index e018a67aa39..c5e9e3495e1 100644
--- a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml
@@ -78,6 +78,9 @@ selector:
   # Windows debug builds than other builds.
   - jstests/core/insert1.js
 
+  # Reads from system.views.
+  - jstests/core/views/views_drop.js
+
   ##
   ## Some aggregation stages don't support snapshot readconcern.
   ##
diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml
index 10b599d2ece..4e4584314f9 100644
--- a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml
@@ -78,6 +78,9 @@ selector:
   # Windows debug builds than other builds.
   - jstests/core/insert1.js
 
+  # Reads from system.views.
+  - jstests/core/views/views_drop.js
+
   ##
   ## Some aggregation stages don't support snapshot readconcern.
   ##
diff --git a/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml
index a2c80ad9dba..e5c200b5c2c 100644
--- a/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml
@@ -59,6 +59,9 @@ selector:
   # Test is only for stand alone mongod.
   - jstests/core/read_after_optime.js
 
+  # Reads from system.views.
+  - jstests/core/views/views_drop.js
+
   ##
   ## Limitations with the way the runner file injects transactions.
   ##
diff --git a/jstests/concurrency/fsm_workloads/view_catalog.js b/jstests/concurrency/fsm_workloads/view_catalog.js
index 0ba3ee3a34a..ebcd37a9c05 100644
--- a/jstests/concurrency/fsm_workloads/view_catalog.js
+++ b/jstests/concurrency/fsm_workloads/view_catalog.js
@@ -10,9 +10,10 @@
 var $config = (function() {
 
     var data = {
-        // Use the workload name as a prefix for the view name,
-        // since the workload name is assumed to be unique.
-        prefix: 'view_catalog'
+        // Use the workload name as a prefix for the view name, since the workload name is assumed
+        // to be unique.
+        prefix: 'view_catalog',
+
     };
 
     var states = (function() {
@@ -21,22 +22,38 @@ var $config = (function() {
             this.threadCollName = db[collName].getName();
             this.threadViewName = this.prefix + '_' + this.tid;
             this.counter = 0;
+            this.confirmViewDefinition = function confirmViewDefinition(
+                db, viewName, collName, pipeline, counter) {
+                assert.eq({_id: counter}, db[collName].aggregate(pipeline).toArray()[0]);
+                assert.eq({_id: counter}, db[viewName].findOne());
+                const res = db.runCommand({listCollections: 1, filter: {name: viewName}});
+                assertAlways.commandWorked(res);
+                assertAlways.eq(1, res.cursor.firstBatch.length, tojson(res));
+                assertAlways.eq({
+                    name: viewName,
+                    type: "view",
+                    options: {viewOn: collName, pipeline: pipeline},
+                    info: {readOnly: true}
+                },
+                                res.cursor.firstBatch[0],
+                                tojson(res));
+            };
         }
 
         function create(db, collName) {
             this.counter++;
-            let pipeline = [{$match: {a: this.counter}}];
+            let pipeline = [{$match: {_id: this.counter}}];
             assertAlways.commandWorked(
                 db.createView(this.threadViewName, this.threadCollName, pipeline));
-            confirmViewDefinition(db, this.threadViewName, collName, pipeline);
+            this.confirmViewDefinition(db, this.threadViewName, collName, pipeline, this.counter);
         }
 
         function modify(db, collName) {
             this.counter++;
-            let pipeline = [{$match: {a: this.counter}}];
+            let pipeline = [{$match: {_id: this.counter}}];
             assertAlways.commandWorked(db.runCommand(
                 {collMod: this.threadViewName, viewOn: this.threadCollName, pipeline: pipeline}));
-            confirmViewDefinition(db, this.threadViewName, collName, pipeline);
+            this.confirmViewDefinition(db, this.threadViewName, collName, pipeline, this.counter);
         }
 
         function drop(db, collName) {
@@ -47,20 +64,6 @@ var $config = (function() {
             assertAlways.eq(0, res.cursor.firstBatch.length, tojson(res));
         }
 
-        function confirmViewDefinition(db, viewName, collName, pipeline) {
-            let res = db.runCommand({listCollections: 1, filter: {name: viewName}});
-            assertAlways.commandWorked(res);
-            assertAlways.eq(1, res.cursor.firstBatch.length, tojson(res));
-            assertAlways.eq({
-                name: viewName,
-                type: "view",
-                options: {viewOn: collName, pipeline: pipeline},
-                info: {readOnly: true}
-            },
-                            res.cursor.firstBatch[0],
-                            tojson(res));
-        }
-
         return {init: init, create: create, modify: modify, drop: drop};
 
     })();
@@ -72,6 +75,14 @@ var $config = (function() {
         drop: {create: 1}
     };
 
+    var setup = function setup(db, collName, cluster) {
+        let bulk = db[collName].initializeOrderedBulkOp();
+        for (let i = 0; i < this.iterations; i++) {
+            bulk.insert({_id: i});
+        }
+        assertAlways.commandWorked(bulk.execute());
+    };
+
     // This test performs createCollection concurrently from many threads, and createCollection on a
     // sharded cluster takes a distributed lock. Since a distributed lock is acquired by repeatedly
     // attempting to grab the lock every half second for 20 seconds (a max of 40 attempts), it's
@@ -85,6 +96,7 @@ var $config = (function() {
         threadCount: 5,
         iterations: 5,
         data: data,
+        setup: setup,
         states: states,
         transitions: transitions,
     };
diff --git a/jstests/concurrency/fsm_workloads/view_catalog_direct_system_writes.js b/jstests/concurrency/fsm_workloads/view_catalog_direct_system_writes.js
new file mode 100644
index 00000000000..2363b9a51bb
--- /dev/null
+++ b/jstests/concurrency/fsm_workloads/view_catalog_direct_system_writes.js
@@ -0,0 +1,46 @@
+'use strict';
+
+/**
+ * view_catalog_direct_system_writes.js
+ *
+ * Extends 'view_catalog.js' in concurrently creating, modifying and dropping view namespaces, but
+ * does so via direct writes to system.views instead of using the collMod or drop commands. Each
+ * worker operates on their own view, built on a shared underlying collection.
+ */
+load('jstests/concurrency/fsm_libs/extend_workload.js');    // for extendWorkload
+load('jstests/concurrency/fsm_workloads/view_catalog.js');  // for $config
+
+var $config = extendWorkload($config, function($config, $super) {
+    $config.states.create = function create(db, collName) {
+        this.counter++;
+        let pipeline = [{$match: {_id: this.counter}}];
+        assertAlways.commandWorked(db.system.views.insert({
+            _id: db.getName() + "." + this.threadViewName,
+            viewOn: this.threadCollName,
+            pipeline: pipeline
+        }));
+        this.confirmViewDefinition(db, this.threadViewName, collName, pipeline, this.counter);
+    };
+
+    $config.states.drop = function drop(db, collName) {
+        assertAlways.commandWorked(
+            db.system.views.deleteOne({_id: db.getName() + "." + this.threadViewName}));
+
+        let res = db.runCommand({listCollections: 1, filter: {name: this.threadViewName}});
+        assertAlways.commandWorked(res);
+        assertAlways.eq(0, res.cursor.firstBatch.length, tojson(res));
+    };
+
+    // Unfortunately we cannot perform an update in the place of a collMod since the update would
+    // contain a $-prefixed field (the $match from the pipeline, and so would be rejected by the
+    // update system. This is okay, the drop override below is enough to reproduce the issue seen in
+    // SERVER-37283. Because of this, we modify the transitions to favor going to drop more often.
+    $config.transitions = {
+        init: {create: 1},
+        create: {modify: 0.5, drop: 0.5},
+        modify: {modify: 0.3, drop: 0.7},
+        drop: {create: 1}
+    };
+
+    return $config;
+});
diff --git a/jstests/core/txns/no_reads_from_system_dot_views_in_txn.js b/jstests/core/txns/no_reads_from_system_dot_views_in_txn.js
new file mode 100644
index 00000000000..2040ee8f8f3
--- /dev/null
+++ b/jstests/core/txns/no_reads_from_system_dot_views_in_txn.js
@@ -0,0 +1,43 @@
+// Tests that it is illegal to read from system.views within a transaction.
+// @tags: [uses_transactions]
+(function() {
+    "use strict";
+
+    load("jstests/libs/fixture_helpers.js");  // For 'FixtureHelpers'.
+
+    const session = db.getMongo().startSession({causalConsistency: false});
+
+    // Use a custom database to avoid conflict with other tests that use system.views.
+    const testDB = session.getDatabase("no_reads_from_system_dot_views_in_txn");
+    assert.commandWorked(testDB.dropDatabase());
+
+    testDB.runCommand({create: "foo", viewOn: "bar", pipeline: []});
+
+    session.startTransaction({readConcern: {level: "snapshot"}});
+    assert.commandFailedWithCode(testDB.runCommand({find: "system.views", filter: {}}), 51071);
+    assert.commandFailedWithCode(session.abortTransaction_forTesting(),
+                                 ErrorCodes.NoSuchTransaction);
+
+    if (FixtureHelpers.isMongos(testDB)) {
+        // The rest of the test is concerned with a find by UUID which is not supported against
+        // mongos.
+        return;
+    }
+
+    const collectionInfos =
+        new DBCommandCursor(testDB, assert.commandWorked(testDB.runCommand({listCollections: 1})));
+    let systemViewsUUID = null;
+    while (collectionInfos.hasNext()) {
+        const next = collectionInfos.next();
+        if (next.name === "system.views") {
+            systemViewsUUID = next.info.uuid;
+        }
+    }
+    assert.neq(null, systemViewsUUID, "did not find UUID for system.views");
+
+    session.startTransaction({readConcern: {level: "snapshot"}});
+    assert.commandFailedWithCode(testDB.runCommand({find: systemViewsUUID, filter: {}}), 51070);
+    assert.commandFailedWithCode(session.abortTransaction_forTesting(),
+                                 ErrorCodes.NoSuchTransaction);
+
+}());
diff --git a/jstests/core/txns/no_writes_to_system_collections_in_txn.js b/jstests/core/txns/no_writes_to_system_collections_in_txn.js
index f7684560cef..5b4c70d0b64 100644
--- a/jstests/core/txns/no_writes_to_system_collections_in_txn.js
+++ b/jstests/core/txns/no_writes_to_system_collections_in_txn.js
@@ -9,6 +9,7 @@
     const testDB = session.getDatabase("no_writes_system_collections_in_txn");
     assert.commandWorked(testDB.dropDatabase());
     const systemColl = testDB.getCollection("system.js");
+    const systemDotViews = testDB.getCollection("system.views");
 
     // Ensure that a collection exists with at least one document.
     assert.commandWorked(systemColl.insert({name: 0}, {writeConcern: {w: "majority"}}));
@@ -31,6 +32,12 @@
                                  ErrorCodes.NoSuchTransaction);
 
     session.startTransaction({readConcern: {level: "snapshot"}});
+    assert.commandFailedWithCode(
+        systemDotViews.insert({_id: "new.view", viewOn: "bar", pipeline: []}), 50791);
+    assert.commandFailedWithCode(session.abortTransaction_forTesting(),
+                                 ErrorCodes.NoSuchTransaction);
+
+    session.startTransaction({readConcern: {level: "snapshot"}});
     assert.commandFailedWithCode(systemColl.update({name: 0}, {$set: {name: "jungsoo"}}), 50791);
     assert.commandFailedWithCode(session.abortTransaction_forTesting(),
                                  ErrorCodes.NoSuchTransaction);
diff --git a/jstests/core/views/views_all_commands.js b/jstests/core/views/views_all_commands.js
index 21bc90b7138..f7f2a26d85c 100644
--- a/jstests/core/views/views_all_commands.js
+++ b/jstests/core/views/views_all_commands.js
@@ -316,7 +316,6 @@
         hostInfo: {skip: isUnrelated},
         insert: {command: {insert: "view", documents: [{x: 1}]}, expectFailure: true},
         invalidateUserCache: {skip: isUnrelated},
-        invalidateViewCatalog: {command: {invalidateViewCatalog: 1}},
         isdbgrid: {skip: isUnrelated},
         isMaster: {skip: isUnrelated},
         killCursors: {
diff --git a/jstests/noPassthroughWithMongod/invalidation_during_resolve_view.js b/jstests/noPassthroughWithMongod/invalidation_during_resolve_view.js
deleted file mode 100644
index e56f19fef9a..00000000000
--- a/jstests/noPassthroughWithMongod/invalidation_during_resolve_view.js
+++ /dev/null
@@ -1,86 +0,0 @@
-/**
- * Tests the behavior of the view catalog when resolving views in the face of concurrent
- * invalidations.
- *
- * We tag the test to only run on the WiredTiger storage engine as other storage engines may have
- * locking behavior that may cause the direct updates in system.views to hang.
- * @tags: [requires_wiredtiger]
- */
-(function() {
-    "use strict";
-
-    const name0 = "invalidation_during_resolution_collection";
-    const name1 = "invalidation_during_resolution_view1";
-    const name2 = "invalidation_during_resolution_view2";
-
-    const ns0 = db.getCollection(name0);
-    const ns1 = db.getCollection(name1);
-    const ns2 = db.getCollection(name2);
-    [ns0, ns1, ns2].forEach(ns => ns.drop());
-    db.system.views.drop();
-
-    // Populate the database with a collection and two views.
-    assert.commandWorked(ns0.insert({name: "max"}));
-    assert.commandWorked(ns0.insert({name: "xiangyu"}));
-    assert.commandWorked(ns0.insert({name: "kyle"}));
-    assert.commandWorked(ns0.insert({name: "jungsoo"}));
-    assert.commandWorked(db.createView(name1, name0, [{$skip: 1}]));
-    assert.commandWorked(db.createView(name2, name1, [{$skip: 1}]));
-
-    // Sanity check.
-    assert.eq(ns0.find().itcount(), 4);
-    assert.eq(ns1.find().itcount(), 3);
-    assert.eq(ns2.find().itcount(), 2);
-
-    // Set a failpoint such that the view catalog is hung in the middle of resolving views.
-    assert.commandWorked(
-        db.adminCommand({configureFailPoint: "hangDuringViewResolution", mode: {skip: 1}}));
-
-    // Start a parallel shell that will manipulate the view catalog. The original state of
-    //      ns2 -> ns1 -> ns0
-    // will become
-    //      ns2 -> ns1 -> ns3 -> ns0
-    let awaitViewCollMod = startParallelShell(() => {
-        const name0 = "invalidation_during_resolution_collection";
-        const name1 = "invalidation_during_resolution_view1";
-        const name2 = "invalidation_during_resolution_view2";
-        const name3 = "invalidation_during_resolution_view3";
-
-        // Wait for the find command below to make progress and hang.
-        jsTestLog("Parallel shell waiting for find to hang.");
-        sleep(5000);
-
-        try {
-            // Modify the view catalog via system.views. (We can't use the collMod command, since it
-            // requires an exclusive collection lock that is currently held in intent shared mode by
-            // the thread performing the find.)
-            jsTestLog("Parallel shell modifying view catalog.");
-            assert.commandWorked(
-                db.system.views.update({_id: `test.${name1}`}, {$set: {viewOn: name3}}));
-            assert.commandWorked(db.system.views.remove({_id: `test.${name3}`}));
-            assert.commandWorked(db.system.views.insert(
-                {_id: `test.${name3}`, viewOn: name0, pipeline: [{$skip: 1}]}));
-
-            // Force an invalidation of the view catalog, which will cause it to be reloaded again
-            // even during the middle of view resolution.
-            jsTestLog("Parallel shell invalidating view catalog.");
-            assert.commandWorked(db.runCommand({invalidateViewCatalog: 1}));
-        } finally {
-            // Now that we're done manipulating the view catalog, allow the command performing ns1
-            // resolution to complete.
-            jsTestLog("Parallel shell disabling fail point.");
-            assert.commandWorked(
-                db.adminCommand({configureFailPoint: "hangDuringViewResolution", mode: "off"}));
-        }
-    });
-
-    // Perform a find on the ns2 that requires multiple steps to resolve. We expect this to see the
-    // latest changes to the view catalog and see two documents, despite facing an invalidation
-    // mid-resolution.
-    const kFiveMinutesInMillis = 1000 * 60 * 5;
-    assert.soon(() => {
-        assert.eq(ns2.find().itcount(), 1);
-        return true;
-    }, "find on view timed out", kFiveMinutesInMillis);
-    awaitViewCollMod();
-}());
diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp
index fdefc58956a..1cf07f5e18f 100644
--- a/src/mongo/db/catalog_raii.cpp
+++ b/src/mongo/db/catalog_raii.cpp
@@ -92,6 +92,15 @@ AutoGetCollection::AutoGetCollection(OperationContext* opCtx,
                             << " disappeared after successufully resolving "
                             << nsOrUUID.toString());
 
+    // In most cases we expect modifications for system.views to upgrade MODE_IX to MODE_X before
+    // taking the lock. One exception is a query by UUID of system.views in a transaction. Usual
+    // queries of system.views (by name, not UUID) within a transaction are rejected. However, if
+    // the query is by UUID we can't determine whether the namespace is actually system.views until
+    // we take the lock here. So we have this one last assertion.
+    uassert(51070,
+            "Modifications to system.views must take an exclusive lock",
+            !_resolvedNss.isSystemDotViews() || modeColl != MODE_IX);
+
     // If the database doesn't exists, we can't obtain a collection or check for views
     if (!db)
         return;
diff --git a/src/mongo/db/commands/SConscript b/src/mongo/db/commands/SConscript
index 244f284fe33..b31517bfbc9 100644
--- a/src/mongo/db/commands/SConscript
+++ b/src/mongo/db/commands/SConscript
@@ -326,7 +326,6 @@ env.Library(
         "do_txn_cmd.cpp",
         "driverHelpers.cpp",
         "haystack.cpp",
-        "invalidate_view_catalog_command.cpp",
         "mr.cpp",
         "oplog_application_checks.cpp",
         "oplog_note.cpp",
diff --git a/src/mongo/db/commands/dbhash.cpp b/src/mongo/db/commands/dbhash.cpp
index 5cbcc0015ba..669f707111a 100644
--- a/src/mongo/db/commands/dbhash.cpp
+++ b/src/mongo/db/commands/dbhash.cpp
@@ -121,7 +121,7 @@ public:
         if (txnParticipant && txnParticipant->inMultiDocumentTransaction()) {
             // However, if we are inside a multi-statement transaction, then we only need to lock
             // the database in intent mode to ensure that none of the collections get dropped.
-            lockMode = getLockModeForQuery(opCtx);
+            lockMode = getLockModeForQuery(opCtx, boost::none);
         }
         AutoGetDb autoDb(opCtx, ns, lockMode);
         Database* db = autoDb.getDb();
@@ -227,8 +227,9 @@ private:
             // reading from the consistent snapshot doesn't overlap with any catalog operations on
             // the collection.
             invariant(
-                opCtx->lockState()->isDbLockedForMode(db->name(), getLockModeForQuery(opCtx)));
-            collLock.emplace(opCtx->lockState(), fullCollectionName, getLockModeForQuery(opCtx));
+                opCtx->lockState()->isDbLockedForMode(db->name(), getLockModeForQuery(opCtx, ns)));
+            collLock.emplace(
+                opCtx->lockState(), fullCollectionName, getLockModeForQuery(opCtx, ns));
 
             auto minSnapshot = collection->getMinimumVisibleSnapshot();
             auto mySnapshot = opCtx->recoveryUnit()->getPointInTimeReadTimestamp();
diff --git a/src/mongo/db/commands/invalidate_view_catalog_command.cpp b/src/mongo/db/commands/invalidate_view_catalog_command.cpp
deleted file mode 100644
index 5c3988fb466..00000000000
--- a/src/mongo/db/commands/invalidate_view_catalog_command.cpp
+++ /dev/null
@@ -1,95 +0,0 @@
-
-/**
- *    Copyright (C) 2018-present MongoDB, Inc.
- *
- *    This program is free software: you can redistribute it and/or modify
- *    it under the terms of the Server Side Public License, version 1,
- *    as published by MongoDB, Inc.
- *
- *    This program is distributed in the hope that it will be useful,
- *    but WITHOUT ANY WARRANTY; without even the implied warranty of
- *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *    Server Side Public License for more details.
- *
- *    You should have received a copy of the Server Side Public License
- *    along with this program. If not, see
- *    <http://www.mongodb.com/licensing/server-side-public-license>.
- *
- *    As a special exception, the copyright holders give permission to link the
- *    code of portions of this program with the OpenSSL library under certain
- *    conditions as described in each individual source file and distribute
- *    linked combinations including the program with the OpenSSL library. You
- *    must comply with the Server Side Public License in all respects for
- *    all of the code used other than as permitted herein. If you modify file(s)
- *    with this exception, you may extend this exception to your version of the
- *    file(s), but you are not obligated to do so. If you do not wish to do so,
- *    delete this exception statement from your version. If you delete this
- *    exception statement from all source files in the program, then also delete
- *    it in the license file.
- */
-
-#include "mongo/platform/basic.h"
-
-#include "mongo/db/catalog_raii.h"
-#include "mongo/db/commands.h"
-#include "mongo/db/commands/test_commands_enabled.h"
-
-namespace mongo {
-/**
- * Testing-only command that invalidates a database's view catalog.
- */
-class InvalidateViewCatalogCmd final : public BasicCommand {
-public:
-    InvalidateViewCatalogCmd() : BasicCommand("invalidateViewCatalog") {}
-
-    Status checkAuthForOperation(OperationContext* opCtx,
-                                 const std::string& dbname,
-                                 const BSONObj& cmdObj) const final {
-        // No auth checks as this is a testing-only command.
-        return Status::OK();
-    }
-
-    bool adminOnly() const final {
-        return false;
-    }
-
-    bool maintenanceMode() const final {
-        return true;
-    }
-
-    bool maintenanceOk() const final {
-        return true;
-    }
-
-    AllowedOnSecondary secondaryAllowed(ServiceContext*) const final {
-        return AllowedOnSecondary::kAlways;
-    }
-
-    bool supportsWriteConcern(const BSONObj& cmd) const final {
-        return false;
-    }
-
-    std::string help() const final {
-        return "invalidate view catalog\n"
-               "Internal command for testing only. Invalidates a database's view catalog,\n"
-               "forcing a reload on the next view operation.\n";
-    }
-
-    bool run(OperationContext* opCtx,
-             const std::string& dbName,
-             const BSONObj& cmdObj,
-             BSONObjBuilder& result) final {
-        AutoGetDb dblock(opCtx, dbName, LockMode::MODE_IS);
-        auto db = dblock.getDb();
-        uassert(ErrorCodes::NamespaceNotFound,
-                str::stream() << "database " << dbName << " does not exist",
-                db);
-
-        db->getViewCatalog()->invalidate();
-        return true;
-    }
-};
-
-MONGO_REGISTER_TEST_COMMAND(InvalidateViewCatalogCmd);
-
-}  // namespace mongo
diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp
index 8555132b7ba..a5a2faae528 100644
--- a/src/mongo/db/db_raii.cpp
+++ b/src/mongo/db/db_raii.cpp
@@ -98,7 +98,7 @@ AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx,
         opCtx->getServiceContext()->getStorageEngine()->supportsReadConcernSnapshot()) {
         _shouldNotConflictWithSecondaryBatchApplicationBlock.emplace(opCtx->lockState());
     }
-    const auto collectionLockMode = getLockModeForQuery(opCtx);
+    const auto collectionLockMode = getLockModeForQuery(opCtx, nsOrUUID.nss());
     _autoColl.emplace(opCtx, nsOrUUID, collectionLockMode, viewMode, deadline);
 
     // If the read source is explicitly set to kNoTimestamp, we read the most up to date data and do
@@ -330,12 +330,15 @@ OldClientContext::~OldClientContext() {
                 currentOp->getReadWriteType());
 }
 
-LockMode getLockModeForQuery(OperationContext* opCtx) {
+LockMode getLockModeForQuery(OperationContext* opCtx, const boost::optional<NamespaceString>& nss) {
     invariant(opCtx);
 
     // Use IX locks for autocommit:false multi-statement transactions; otherwise, use IS locks.
     auto txnParticipant = TransactionParticipant::get(opCtx);
     if (txnParticipant && txnParticipant->inMultiDocumentTransaction()) {
+        uassert(51071,
+                "Cannot query system.views within a transaction",
+                !nss || !nss->isSystemDotViews());
         return MODE_IX;
     }
     return MODE_IS;
diff --git a/src/mongo/db/db_raii.h b/src/mongo/db/db_raii.h
index 39a30282630..40ede468e0d 100644
--- a/src/mongo/db/db_raii.h
+++ b/src/mongo/db/db_raii.h
@@ -215,7 +215,8 @@ private:
 /**
  * Returns a MODE_IX LockMode if a read is performed under readConcern level snapshot, or a MODE_IS
  * lock otherwise. MODE_IX acquisition will allow a read to participate in two-phase locking.
+ * Throws an exception if 'system.views' is being queried within a transaction.
  */
-LockMode getLockModeForQuery(OperationContext* opCtx);
+LockMode getLockModeForQuery(OperationContext* opCtx, const boost::optional<NamespaceString>& nss);
 
 }  // namespace mongo
diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp
index 200f263a283..1007bb72d3c 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -302,6 +302,10 @@ SingleWriteResult createIndex(OperationContext* opCtx,
     return result;
 }
 
+LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
+    return nss.isSystemDotViews() ? MODE_X : mode;
+}
+
 void insertDocuments(OperationContext* opCtx,
                      Collection* collection,
                      std::vector<InsertStatement>::iterator begin,
@@ -373,7 +377,10 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx,
                 uasserted(ErrorCodes::InternalError, "failAllInserts failpoint active!");
             }
 
-            collection.emplace(opCtx, wholeOp.getNamespace(), MODE_IX);
+            collection.emplace(
+                opCtx,
+                wholeOp.getNamespace(),
+                fixLockModeForSystemDotViewsChanges(wholeOp.getNamespace(), MODE_IX));
             if (collection->getCollection())
                 break;
 
@@ -605,7 +612,7 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx,
         collection.emplace(opCtx,
                            ns,
                            MODE_IX,  // DB is always IX, even if collection is X.
-                           MODE_IX);
+                           fixLockModeForSystemDotViewsChanges(ns, MODE_IX));
         if (collection->getCollection() || !updateRequest.isUpsert())
             break;
 
@@ -845,7 +852,7 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx,
     AutoGetCollection collection(opCtx,
                                  ns,
                                  MODE_IX,  // DB is always IX, even if collection is X.
-                                 MODE_IX);
+                                 fixLockModeForSystemDotViewsChanges(ns, MODE_IX));
     if (collection.getDb()) {
         curOp.raiseDbProfileLevel(collection.getDb()->getProfilingLevel());
     }
diff --git a/src/mongo/db/pipeline/document_source_cursor.cpp b/src/mongo/db/pipeline/document_source_cursor.cpp
index fe78bda703c..8a75b77ed1c 100644
--- a/src/mongo/db/pipeline/document_source_cursor.cpp
+++ b/src/mongo/db/pipeline/document_source_cursor.cpp
@@ -216,7 +216,7 @@ Value DocumentSourceCursor::serialize(boost::optional<ExplainOptions::Verbosity>
 
     {
         auto opCtx = pExpCtx->opCtx;
-        auto lockMode = getLockModeForQuery(opCtx);
+        auto lockMode = getLockModeForQuery(opCtx, _exec->nss());
         AutoGetDb dbLock(opCtx, _exec->nss().db(), lockMode);
         Lock::CollectionLock collLock(opCtx->lockState(), _exec->nss().ns(), lockMode);
         auto collection =
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index b473d08aa62..d46302c1517 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -91,6 +91,9 @@ using UniqueLock = stdx::unique_lock<stdx::mutex>;
 
 const auto kIdIndexName = "_id_"_sd;
 
+LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
+    return nss.isSystemDotViews() ? MODE_X : mode;
+}
 }  // namespace
 
 StorageInterfaceImpl::StorageInterfaceImpl()
@@ -219,7 +222,7 @@ StorageInterfaceImpl::createCollectionForBulkLoading(
 
         // Get locks and create the collection.
         AutoGetOrCreateDb db(opCtx.get(), nss.db(), MODE_X);
-        AutoGetCollection coll(opCtx.get(), nss, MODE_IX);
+        AutoGetCollection coll(opCtx.get(), nss, fixLockModeForSystemDotViewsChanges(nss, MODE_IX));
 
         if (coll.getCollection()) {
             return Status(ErrorCodes::NamespaceExists,
@@ -232,7 +235,8 @@ StorageInterfaceImpl::createCollectionForBulkLoading(
             wunit.commit();
         }
 
-        autoColl = stdx::make_unique<AutoGetCollection>(opCtx.get(), nss, MODE_IX);
+        autoColl = stdx::make_unique<AutoGetCollection>(
+            opCtx.get(), nss, fixLockModeForSystemDotViewsChanges(nss, MODE_IX));
 
         // Build empty capped indexes.  Capped indexes cannot be built by the MultiIndexBlock
         // because the cap might delete documents off the back while we are inserting them into
diff --git a/src/mongo/db/repl/sync_tail.cpp b/src/mongo/db/repl/sync_tail.cpp
index e7b666ff784..1b879b6ba4b 100644
--- a/src/mongo/db/repl/sync_tail.cpp
+++ b/src/mongo/db/repl/sync_tail.cpp
@@ -268,6 +268,10 @@ Status finishAndLogApply(ClockSource* clockSource,
     return finalStatus;
 }
 
+LockMode fixLockModeForSystemDotViewsChanges(const NamespaceString& nss, LockMode mode) {
+    return nss.isSystemDotViews() ? MODE_X : mode;
+}
+
 }  // namespace
 
 // static
@@ -331,7 +335,8 @@ Status SyncTail::syncApply(OperationContext* opCtx,
         return finishApply(writeConflictRetry(opCtx, "syncApply_CRUD", nss.ns(), [&] {
             // Need to throw instead of returning a status for it to be properly ignored.
             try {
-                AutoGetCollection autoColl(opCtx, getNsOrUUID(nss, op), MODE_IX);
+                AutoGetCollection autoColl(
+                    opCtx, getNsOrUUID(nss, op), fixLockModeForSystemDotViewsChanges(nss, MODE_IX));
                 auto db = autoColl.getDb();
                 uassert(ErrorCodes::NamespaceNotFound,
                         str::stream() << "missing database (" << nss.db() << ")",
diff --git a/src/mongo/db/views/view_catalog.cpp b/src/mongo/db/views/view_catalog.cpp
index 67aa5b27cab..e9763fc488d 100644
--- a/src/mongo/db/views/view_catalog.cpp
+++ b/src/mongo/db/views/view_catalog.cpp
@@ -60,7 +60,6 @@
 
 namespace mongo {
 namespace {
-MONGO_FAIL_POINT_DEFINE(hangDuringViewResolution);
 
 StatusWith<std::unique_ptr<CollatorInterface>> parseCollator(OperationContext* opCtx,
                                                              BSONObj collationSpec) {
@@ -460,13 +459,6 @@ StatusWith<ResolvedView> ViewCatalog::resolveView(OperationContext* opCtx,
 
         int depth = 0;
         for (; depth < ViewGraph::kMaxViewDepth; depth++) {
-            while (MONGO_FAIL_POINT(hangDuringViewResolution)) {
-                log() << "Yielding mutex and hanging due to 'hangDuringViewResolution' failpoint";
-                lock.unlock();
-                sleepmillis(1000);
-                lock.lock();
-            }
-
             // If the catalog has been invalidated, bail and restart.
             if (!_valid.load()) {
                 uassertStatusOK(_reloadIfNeeded(lock, opCtx));