SERVER-40681 Make AutoGetCollection interface match AutoGetDB

11 files changed, 30 insertions, 35 deletions

diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp
index ef6074eaaff..b74a55f00cf 100644
--- a/src/mongo/db/catalog/drop_indexes.cpp
+++ b/src/mongo/db/catalog/drop_indexes.cpp
@@ -200,7 +200,7 @@ Status dropIndexes(OperationContext* opCtx,
                    const BSONObj& cmdObj,
                    BSONObjBuilder* result) {
     return writeConflictRetry(opCtx, "dropIndexes", nss.db(), [opCtx, &nss, &cmdObj, result] {
-        AutoGetCollection autoColl(opCtx, nss, MODE_IX, MODE_X);
+        AutoGetCollection autoColl(opCtx, nss, MODE_X);
 
         bool userInitiatedWritesAndNotPrimary = opCtx->writesAreReplicated() &&
             !repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, nss);
diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp
index d3e4c1f5e60..efd30fb762d 100644
--- a/src/mongo/db/catalog_raii.cpp
+++ b/src/mongo/db/catalog_raii.cpp
@@ -58,13 +58,12 @@ AutoGetDb::AutoGetDb(OperationContext* opCtx, StringData dbName, LockMode mode,
 
 AutoGetCollection::AutoGetCollection(OperationContext* opCtx,
                                      const NamespaceStringOrUUID& nsOrUUID,
-                                     LockMode modeDB,
                                      LockMode modeColl,
                                      ViewMode viewMode,
                                      Date_t deadline)
     : _autoDb(opCtx,
               !nsOrUUID.dbname().empty() ? nsOrUUID.dbname() : nsOrUUID.nss()->db(),
-              modeDB,
+              isSharedLockMode(modeColl) ? MODE_IS : MODE_IX,
               deadline),
       _resolvedNss(resolveNamespaceStringOrUUID(opCtx, nsOrUUID)) {
 
diff --git a/src/mongo/db/catalog_raii.h b/src/mongo/db/catalog_raii.h
index 8522f33c92b..36043e60564 100644
--- a/src/mongo/db/catalog_raii.h
+++ b/src/mongo/db/catalog_raii.h
@@ -73,8 +73,15 @@ private:
 };
 
 /**
- * RAII-style class, which acquires a locks on the specified database and collection in the
- * requested modes and obtains references to both.
+ * RAII-style class, which acquires global, database, and collection locks according to the chart
+ * below.
+ *
+ * | modeColl | Global Lock Result | DB Lock Result | Collection Lock Result |
+ * |----------+--------------------+----------------+------------------------|
+ * | MODE_IX  | MODE_IX            | MODE_IX        | MODE_IX                |
+ * | MODE_X   | MODE_IX            | MODE_IX        | MODE_X                 |
+ * | MODE_IS  | MODE_IS            | MODE_IS        | MODE_IS                |
+ * | MODE_S   | MODE_IS            | MODE_IS        | MODE_S                 |
  *
  * NOTE: Throws NamespaceNotFound if the collection UUID cannot be resolved to a name.
  *
@@ -90,14 +97,6 @@ public:
 
     AutoGetCollection(OperationContext* opCtx,
                       const NamespaceStringOrUUID& nsOrUUID,
-                      LockMode modeAll,
-                      ViewMode viewMode = kViewsForbidden,
-                      Date_t deadline = Date_t::max())
-        : AutoGetCollection(opCtx, nsOrUUID, modeAll, modeAll, viewMode, deadline) {}
-
-    AutoGetCollection(OperationContext* opCtx,
-                      const NamespaceStringOrUUID& nsOrUUID,
-                      LockMode modeDB,
                       LockMode modeColl,
                       ViewMode viewMode = kViewsForbidden,
                       Date_t deadline = Date_t::max());
diff --git a/src/mongo/db/catalog_raii_test.cpp b/src/mongo/db/catalog_raii_test.cpp
index ac0e64f80ed..b01e508db03 100644
--- a/src/mongo/db/catalog_raii_test.cpp
+++ b/src/mongo/db/catalog_raii_test.cpp
@@ -139,7 +139,6 @@ TEST_F(CatalogRAIITestFixture, AutoGetCollectionCollLockDeadline) {
         [&] {
             AutoGetCollection coll(client2.second.get(),
                                    nss,
-                                   MODE_IX,
                                    MODE_X,
                                    AutoGetCollection::ViewMode::kViewsForbidden,
                                    Date_t::now() + timeoutMs);
@@ -155,7 +154,6 @@ TEST_F(CatalogRAIITestFixture, AutoGetCollectionDBLockDeadline) {
             AutoGetCollection coll(client2.second.get(),
                                    nss,
                                    MODE_X,
-                                   MODE_X,
                                    AutoGetCollection::ViewMode::kViewsForbidden,
                                    Date_t::now() + timeoutMs);
         },
@@ -171,7 +169,6 @@ TEST_F(CatalogRAIITestFixture, AutoGetCollectionGlobalLockDeadline) {
             AutoGetCollection coll(client2.second.get(),
                                    nss,
                                    MODE_X,
-                                   MODE_X,
                                    AutoGetCollection::ViewMode::kViewsForbidden,
                                    Date_t::now() + timeoutMs);
         },
@@ -188,7 +185,6 @@ TEST_F(CatalogRAIITestFixture, AutoGetCollectionDeadlineNow) {
         [&] {
             AutoGetCollection coll(client2.second.get(),
                                    nss,
-                                   MODE_IX,
                                    MODE_X,
                                    AutoGetCollection::ViewMode::kViewsForbidden,
                                    Date_t::now());
@@ -206,7 +202,6 @@ TEST_F(CatalogRAIITestFixture, AutoGetCollectionDeadlineMin) {
         [&] {
             AutoGetCollection coll(client2.second.get(),
                                    nss,
-                                   MODE_IX,
                                    MODE_X,
                                    AutoGetCollection::ViewMode::kViewsForbidden,
                                    Date_t());
@@ -214,6 +209,13 @@ TEST_F(CatalogRAIITestFixture, AutoGetCollectionDeadlineMin) {
         Milliseconds(0));
 }
 
+TEST_F(CatalogRAIITestFixture, AutoGetCollectionDBLockCompatibleX) {
+    Lock::DBLock dbLock1(client1.second.get(), nss.db(), MODE_IX);
+    ASSERT(client1.second->lockState()->isDbLockedForMode(nss.db(), MODE_IX));
+
+    AutoGetCollection coll(client2.second.get(), nss, MODE_X);
+}
+
 using ReadSource = RecoveryUnit::ReadSource;
 
 class RecoveryUnitMock : public RecoveryUnitNoop {
diff --git a/src/mongo/db/commands/test_commands.cpp b/src/mongo/db/commands/test_commands.cpp
index 1410bb5a3bc..51373ac0841 100644
--- a/src/mongo/db/commands/test_commands.cpp
+++ b/src/mongo/db/commands/test_commands.cpp
@@ -141,7 +141,7 @@ public:
         }
 
         // Lock the database in mode IX and lock the collection exclusively.
-        AutoGetCollection autoColl(opCtx, fullNs, MODE_IX, MODE_X);
+        AutoGetCollection autoColl(opCtx, fullNs, MODE_X);
         Collection* collection = autoColl.getCollection();
         if (!collection) {
             uasserted(ErrorCodes::NamespaceNotFound,
diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp
index 6991f274277..70b087f4dde 100644
--- a/src/mongo/db/index_builds_coordinator.cpp
+++ b/src/mongo/db/index_builds_coordinator.cpp
@@ -557,7 +557,7 @@ IndexBuildsCoordinator::_registerAndSetUpIndexBuild(
                                     << "' because the collection no longer exists.");
     }
 
-    AutoGetCollection autoColl(opCtx, *nss, /*modeDB=*/MODE_IX, /*modeColl=*/MODE_X);
+    AutoGetCollection autoColl(opCtx, *nss, MODE_X);
     if (!autoColl.getDb()) {
         return Status(ErrorCodes::NamespaceNotFound,
                       str::stream() << "Failed to create index(es) on collection '" << *nss
diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp
index 0e4f2dc212b..3bc99ed80d4 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -612,7 +612,7 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx,
 
     boost::optional<AutoGetCollection> collection;
     while (true) {
-        collection.emplace(opCtx, ns, MODE_IX, fixLockModeForSystemDotViewsChanges(ns, MODE_IX));
+        collection.emplace(opCtx, ns, fixLockModeForSystemDotViewsChanges(ns, MODE_IX));
 
         // If this is an upsert, which is an insert, we must have a collection.
         // An update on a non-existant collection is okay and handled later.
@@ -867,8 +867,7 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx,
         uasserted(ErrorCodes::InternalError, "failAllRemoves failpoint active!");
     }
 
-    AutoGetCollection collection(
-        opCtx, ns, MODE_IX, fixLockModeForSystemDotViewsChanges(ns, MODE_IX));
+    AutoGetCollection collection(opCtx, ns, fixLockModeForSystemDotViewsChanges(ns, MODE_IX));
 
     if (collection.getDb()) {
         curOp.raiseDbProfileLevel(collection.getDb()->getProfilingLevel());
diff --git a/src/mongo/db/s/collection_sharding_runtime.cpp b/src/mongo/db/s/collection_sharding_runtime.cpp
index 05fbb152475..9c2c1386978 100644
--- a/src/mongo/db/s/collection_sharding_runtime.cpp
+++ b/src/mongo/db/s/collection_sharding_runtime.cpp
@@ -182,7 +182,6 @@ CollectionCriticalSection::CollectionCriticalSection(OperationContext* opCtx, Na
     : _nss(std::move(ns)), _opCtx(opCtx) {
     AutoGetCollection autoColl(_opCtx,
                                _nss,
-                               MODE_IX,
                                MODE_X,
                                AutoGetCollection::ViewMode::kViewsForbidden,
                                opCtx->getServiceContext()->getPreciseClockSource()->now() +
@@ -195,7 +194,7 @@ CollectionCriticalSection::CollectionCriticalSection(OperationContext* opCtx, Na
 
 CollectionCriticalSection::~CollectionCriticalSection() {
     UninterruptibleLockGuard noInterrupt(_opCtx->lockState());
-    AutoGetCollection autoColl(_opCtx, _nss, MODE_IX, MODE_IX);
+    AutoGetCollection autoColl(_opCtx, _nss, MODE_IX);
     auto* const csr = CollectionShardingRuntime::get(_opCtx, _nss);
     auto csrLock = CollectionShardingRuntime::CSRLock::lockExclusive(_opCtx, csr);
 
@@ -205,7 +204,6 @@ CollectionCriticalSection::~CollectionCriticalSection() {
 void CollectionCriticalSection::enterCommitPhase() {
     AutoGetCollection autoColl(_opCtx,
                                _nss,
-                               MODE_IX,
                                MODE_X,
                                AutoGetCollection::ViewMode::kViewsForbidden,
                                _opCtx->getServiceContext()->getPreciseClockSource()->now() +
diff --git a/src/mongo/db/s/migration_destination_manager.cpp b/src/mongo/db/s/migration_destination_manager.cpp
index 7df4fdeef0b..ce8fb419f08 100644
--- a/src/mongo/db/s/migration_destination_manager.cpp
+++ b/src/mongo/db/s/migration_destination_manager.cpp
@@ -1145,7 +1145,7 @@ bool MigrationDestinationManager::_flushPendingWrites(OperationContext* opCtx,
 CollectionShardingRuntime::CleanupNotification MigrationDestinationManager::_notePending(
     OperationContext* opCtx, ChunkRange const& range) {
 
-    AutoGetCollection autoColl(opCtx, _nss, MODE_IX, MODE_X);
+    AutoGetCollection autoColl(opCtx, _nss, MODE_X);
     auto* const css = CollectionShardingRuntime::get(opCtx, _nss);
     const auto optMetadata = css->getCurrentMetadataIfKnown();
 
@@ -1176,7 +1176,7 @@ void MigrationDestinationManager::_forgetPending(OperationContext* opCtx, ChunkR
     }
 
     UninterruptibleLockGuard noInterrupt(opCtx->lockState());
-    AutoGetCollection autoColl(opCtx, _nss, MODE_IX, MODE_IX);
+    AutoGetCollection autoColl(opCtx, _nss, MODE_IX);
     auto* const css = CollectionShardingRuntime::get(opCtx, _nss);
     const auto optMetadata = css->getCurrentMetadataIfKnown();
 
diff --git a/src/mongo/db/s/migration_source_manager.cpp b/src/mongo/db/s/migration_source_manager.cpp
index 8935b676e45..221ade92b96 100644
--- a/src/mongo/db/s/migration_source_manager.cpp
+++ b/src/mongo/db/s/migration_source_manager.cpp
@@ -255,14 +255,12 @@ Status MigrationSourceManager::startClone(OperationContext* opCtx) {
             autoColl.emplace(opCtx,
                              getNss(),
                              MODE_IX,
-                             MODE_IX,
                              AutoGetCollection::ViewMode::kViewsForbidden,
                              opCtx->getServiceContext()->getPreciseClockSource()->now() +
                                  Milliseconds(migrationLockAcquisitionMaxWaitMS.load()));
         } else {
             autoColl.emplace(opCtx,
                              getNss(),
-                             MODE_IX,
                              MODE_X,
                              AutoGetCollection::ViewMode::kViewsForbidden,
                              opCtx->getServiceContext()->getPreciseClockSource()->now() +
@@ -474,7 +472,7 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC
         // this node can accept writes for this collection as a proxy for it being primary.
         if (!status.isOK()) {
             UninterruptibleLockGuard noInterrupt(opCtx->lockState());
-            AutoGetCollection autoColl(opCtx, getNss(), MODE_IX, MODE_IX);
+            AutoGetCollection autoColl(opCtx, getNss(), MODE_IX);
             if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, getNss())) {
                 CollectionShardingRuntime::get(opCtx, getNss())->clearFilteringMetadata();
                 uassertStatusOK(status.withContext(
@@ -510,7 +508,7 @@ Status MigrationSourceManager::commitChunkMetadataOnConfig(OperationContext* opC
 
     if (!refreshStatus.isOK()) {
         UninterruptibleLockGuard noInterrupt(opCtx->lockState());
-        AutoGetCollection autoColl(opCtx, getNss(), MODE_IX, MODE_IX);
+        AutoGetCollection autoColl(opCtx, getNss(), MODE_IX);
 
         CollectionShardingRuntime::get(opCtx, getNss())->clearFilteringMetadata();
 
@@ -709,7 +707,7 @@ void MigrationSourceManager::_cleanup(OperationContext* opCtx) {
     auto cloneDriver = [&]() {
         // Unregister from the collection's sharding state and exit the migration critical section.
         UninterruptibleLockGuard noInterrupt(opCtx->lockState());
-        AutoGetCollection autoColl(opCtx, getNss(), MODE_IX, MODE_IX);
+        AutoGetCollection autoColl(opCtx, getNss(), MODE_IX);
         auto* const csr = CollectionShardingRuntime::get(opCtx, getNss());
         auto csrLock = CollectionShardingState::CSRLock::lockExclusive(opCtx, csr);
 
diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
index 26bfe4e897e..e8de649a76e 100644
--- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
+++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
@@ -154,7 +154,7 @@ ChunkVersion forceShardFilteringMetadataRefresh(OperationContext* opCtx,
         // No chunk manager, so unsharded.
 
         // Exclusive collection lock needed since we're now changing the metadata
-        AutoGetCollection autoColl(opCtx, nss, MODE_IX, MODE_X);
+        AutoGetCollection autoColl(opCtx, nss, MODE_X);
         CollectionShardingRuntime::get(opCtx, nss)
             ->setFilteringMetadata(opCtx, CollectionMetadata());
 
@@ -181,7 +181,7 @@ ChunkVersion forceShardFilteringMetadataRefresh(OperationContext* opCtx,
     }
 
     // Exclusive collection lock needed since we're now changing the metadata
-    AutoGetCollection autoColl(opCtx, nss, MODE_IX, MODE_X);
+    AutoGetCollection autoColl(opCtx, nss, MODE_X);
     auto* const css = CollectionShardingRuntime::get(opCtx, nss);
 
     {