SERVER-46865 Make collMod not take database MODE_X lock

6 files changed, 58 insertions, 12 deletions

diff --git a/jstests/noPassthroughWithMongod/collMod_no_conflicts.js b/jstests/noPassthroughWithMongod/collMod_no_conflicts.js
new file mode 100644
index 00000000000..130582d2f61
--- /dev/null
+++ b/jstests/noPassthroughWithMongod/collMod_no_conflicts.js
@@ -0,0 +1,42 @@
+/**
+ * Tests that collMod does not conflict with a database MODE_IX lock.
+ */
+(function() {
+"use strict";
+
+load("jstests/libs/parallel_shell_helpers.js");
+load("jstests/libs/wait_for_command.js");
+
+const collName = "collMod_no_conflicts";
+const viewName = "testView";
+const testDB = db.getSiblingDB("test");
+testDB.dropDatabase();
+
+const sleepFunction = function(sleepDB) {
+    // If collMod calls need to wait on this lock, holding this lock for 4 hours will
+    // trigger a test timeout.
+    assert.commandFailedWithCode(
+        db.getSiblingDB("test").adminCommand(
+            {sleep: 1, secs: 18000, lockTarget: sleepDB, lock: "iw", $comment: "Lock sleep"}),
+        ErrorCodes.Interrupted);
+};
+
+const sleepCommand = startParallelShell(funWithArgs(sleepFunction, "test"), testDB.getMongo().port);
+const sleepID =
+    waitForCommand("sleepCmd",
+                   op => (op["ns"] == "admin.$cmd" && op["command"]["$comment"] == "Lock sleep"),
+                   testDB.getSiblingDB("admin"));
+
+assert.commandWorked(testDB.createView(viewName, collName, [{$match: {a: 1}}]));
+const collModPipeline = [{$match: {a: 2}}];
+assert.commandWorked(
+    testDB.runCommand({collMod: viewName, viewOn: collName, pipeline: collModPipeline}));
+
+const res = db.getCollectionInfos({name: viewName});
+assert.eq(res.length, 1);
+assert.eq(res[0].options.pipeline, collModPipeline);
+
+// Interrupt the sleep command.
+assert.commandWorked(testDB.getSiblingDB("admin").killOp(sleepID));
+sleepCommand();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index 8d0b0f286bf..afb64d12476 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -272,10 +272,12 @@ Status _collModInternal(OperationContext* opCtx,
                         const BSONObj& cmdObj,
                         BSONObjBuilder* result) {
     StringData dbName = nss.db();
-    AutoGetDb autoDb(opCtx, dbName, MODE_X);
-    Database* const db = autoDb.getDb();
-    Collection* coll =
-        db ? CollectionCatalog::get(opCtx).lookupCollectionByNamespace(opCtx, nss) : nullptr;
+    AutoGetCollection autoColl(opCtx, nss, MODE_X, AutoGetCollection::ViewMode::kViewsPermitted);
+    Lock::CollectionLock systemViewsLock(
+        opCtx, NamespaceString(dbName, NamespaceString::kSystemDotViewsCollectionName), MODE_X);
+
+    Database* const db = autoColl.getDb();
+    Collection* coll = autoColl.getCollection();
 
     CurOpFailpointHelpers::waitWhileFailPointEnabled(
         &hangAfterDatabaseLock, opCtx, "hangAfterDatabaseLock", []() {}, false, nss);
diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp
index c42dee1d8a4..6b9ee596ba7 100644
--- a/src/mongo/db/op_observer_impl.cpp
+++ b/src/mongo/db/op_observer_impl.cpp
@@ -686,7 +686,7 @@ void OpObserverImpl::onCollMod(OperationContext* opCtx,
 
     // Make sure the UUID values in the Collection metadata, the Collection object, and the UUID
     // catalog are all present and equal.
-    invariant(opCtx->lockState()->isDbLockedForMode(nss.db(), MODE_X));
+    invariant(opCtx->lockState()->isCollectionLockedForMode(nss, MODE_X));
     auto databaseHolder = DatabaseHolder::get(opCtx);
     auto db = databaseHolder->getDb(opCtx, nss.db());
     // Some unit tests call the op observer on an unregistered Database.
diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp
index b66303045a6..4408c7494c4 100644
--- a/src/mongo/db/op_observer_impl_test.cpp
+++ b/src/mongo/db/op_observer_impl_test.cpp
@@ -282,7 +282,7 @@ TEST_F(OpObserverTest, CollModWithCollectionOptionsAndTTLInfo) {
 
     // Write to the oplog.
     {
-        AutoGetDb autoDb(opCtx.get(), nss.db(), MODE_X);
+        AutoGetCollection autoColl(opCtx.get(), nss, MODE_X);
         WriteUnitOfWork wunit(opCtx.get());
         opObserver.onCollMod(opCtx.get(), nss, uuid, collModCmd, oldCollOpts, ttlInfo);
         wunit.commit();
@@ -331,7 +331,7 @@ TEST_F(OpObserverTest, CollModWithOnlyCollectionOptions) {
 
     // Write to the oplog.
     {
-        AutoGetDb autoDb(opCtx.get(), nss.db(), MODE_X);
+        AutoGetCollection autoColl(opCtx.get(), nss, MODE_X);
         WriteUnitOfWork wunit(opCtx.get());
         opObserver.onCollMod(opCtx.get(), nss, uuid, collModCmd, oldCollOpts, boost::none);
         wunit.commit();
diff --git a/src/mongo/db/views/view_catalog.cpp b/src/mongo/db/views/view_catalog.cpp
index 9481a83c400..38031abc2c1 100644
--- a/src/mongo/db/views/view_catalog.cpp
+++ b/src/mongo/db/views/view_catalog.cpp
@@ -438,7 +438,9 @@ Status ViewCatalog::modifyView(OperationContext* opCtx,
                                const NamespaceString& viewName,
                                const NamespaceString& viewOn,
                                const BSONArray& pipeline) {
-    invariant(opCtx->lockState()->isDbLockedForMode(viewName.db(), MODE_X));
+    invariant(opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_X));
+    invariant(opCtx->lockState()->isCollectionLockedForMode(
+        NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName), MODE_X));
 
     stdx::lock_guard<Latch> lk(_mutex);
 
diff --git a/src/mongo/db/views/view_catalog_test.cpp b/src/mongo/db/views/view_catalog_test.cpp
index c186573e437..ded055683e1 100644
--- a/src/mongo/db/views/view_catalog_test.cpp
+++ b/src/mongo/db/views/view_catalog_test.cpp
@@ -133,8 +133,8 @@ public:
                       const NamespaceString& viewName,
                       const NamespaceString& viewOn,
                       const BSONArray& pipeline) {
-        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_X);
-        Lock::CollectionLock collLock(operationContext(), viewName, MODE_IX);
+        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_IX);
+        Lock::CollectionLock collLock(operationContext(), viewName, MODE_X);
         Lock::CollectionLock sysCollLock(
             operationContext(),
             NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName),
@@ -592,8 +592,8 @@ TEST_F(ViewCatalogFixture, LookupRIDAfterModifyRollback) {
         wunit.commit();
     }
     {
-        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_X);
-        Lock::CollectionLock collLock(operationContext(), viewName, MODE_IX);
+        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_IX);
+        Lock::CollectionLock collLock(operationContext(), viewName, MODE_X);
         Lock::CollectionLock sysCollLock(
             operationContext(),
             NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName),