SERVER-47317 Take collection MODE_X locks instead of database MODE_X lock in covertToCapped

(cherry picked from commit ff483e300c692dd2a47a649c4bddc3632178b2fd)

5 files changed, 87 insertions, 35 deletions

diff --git a/jstests/noPassthroughWithMongod/convert_to_capped_no_conflicts.js b/jstests/noPassthroughWithMongod/convert_to_capped_no_conflicts.js
new file mode 100644
index 00000000000..f037b0075ee
--- /dev/null
+++ b/jstests/noPassthroughWithMongod/convert_to_capped_no_conflicts.js
@@ -0,0 +1,38 @@
+/**
+ * Tests that convertToCapped 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 = "convert_to_capped_no_conflicts";
+const testDB = db.getSiblingDB("test");
+testDB.dropDatabase();
+const testColl = testDB.getCollection(collName);
+
+const sleepFunction = function(sleepDB) {
+    // If convertToCapped 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(testColl.insert({a: 1}));
+assert(!testColl.isCapped());
+assert.commandWorked(testDB.runCommand({convertToCapped: collName, size: 1}));
+assert(testColl.isCapped());
+
+// 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/capped_utils.cpp b/src/mongo/db/catalog/capped_utils.cpp
index 6aa842def14..9c45c28eea4 100644
--- a/src/mongo/db/catalog/capped_utils.cpp
+++ b/src/mongo/db/catalog/capped_utils.cpp
@@ -237,38 +237,51 @@ void cloneCollectionAsCapped(OperationContext* opCtx,
     MONGO_UNREACHABLE;
 }
 
-void convertToCapped(OperationContext* opCtx,
-                     const NamespaceString& collectionName,
-                     long long size) {
-    StringData dbname = collectionName.db();
-    StringData shortSource = collectionName.coll();
+void convertToCapped(OperationContext* opCtx, const NamespaceString& ns, long long size) {
+    StringData dbname = ns.db();
+    StringData shortSource = ns.coll();
 
-    AutoGetDb autoDb(opCtx, collectionName.db(), MODE_X);
+    AutoGetCollection autoColl(opCtx, ns, MODE_X);
 
     bool userInitiatedWritesAndNotPrimary = opCtx->writesAreReplicated() &&
-        !repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, collectionName);
+        !repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, ns);
 
     uassert(ErrorCodes::NotMaster,
-            str::stream() << "Not primary while converting " << collectionName
-                          << " to a capped collection",
+            str::stream() << "Not primary while converting " << ns << " to a capped collection",
             !userInitiatedWritesAndNotPrimary);
 
-    Database* const db = autoDb.getDb();
+    Database* const db = autoColl.getDb();
     uassert(
         ErrorCodes::NamespaceNotFound, str::stream() << "database " << dbname << " not found", db);
 
-    BackgroundOperation::assertNoBgOpInProgForDb(dbname);
-    IndexBuildsCoordinator::get(opCtx)->assertNoBgOpInProgForDb(dbname);
+    BackgroundOperation::assertNoBgOpInProgForNs(ns);
+    if (Collection* coll = autoColl.getCollection()) {
+        IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection(coll->uuid());
+    }
 
     // Generate a temporary collection name that will not collide with any existing collections.
-    auto tmpNameResult =
-        db->makeUniqueCollectionNamespace(opCtx, "tmp%%%%%.convertToCapped." + shortSource);
-    uassertStatusOKWithContext(tmpNameResult,
-                               str::stream()
-                                   << "Cannot generate temporary collection namespace to convert "
-                                   << collectionName << " to a capped collection");
-
-    const auto& longTmpName = tmpNameResult.getValue();
+    boost::optional<Lock::CollectionLock> collLock;
+    const auto longTmpName = [&] {
+        while (true) {
+            auto tmpNameResult =
+                db->makeUniqueCollectionNamespace(opCtx, "tmp%%%%%.convertToCapped." + shortSource);
+            uassertStatusOKWithContext(
+                tmpNameResult,
+                str::stream() << "Cannot generate temporary collection namespace to convert " << ns
+                              << " to a capped collection");
+
+            collLock.emplace(opCtx, tmpNameResult.getValue(), MODE_X);
+            if (!CollectionCatalog::get(opCtx).lookupCollectionByNamespace(
+                    opCtx, tmpNameResult.getValue())) {
+                return std::move(tmpNameResult.getValue());
+            }
+
+            // The temporary collection was created by someone else between the name being
+            // generated and acquiring the lock on the collection, so try again with a new
+            // temporary collection name.
+            collLock.reset();
+        }
+    }();
     const auto shortTmpName = longTmpName.coll().toString();
 
     cloneCollectionAsCapped(opCtx, db, shortSource.toString(), shortTmpName, size, true);
@@ -276,7 +289,7 @@ void convertToCapped(OperationContext* opCtx,
     RenameCollectionOptions options;
     options.dropTarget = true;
     options.stayTemp = false;
-    uassertStatusOK(renameCollection(opCtx, longTmpName, collectionName, options));
+    uassertStatusOK(renameCollection(opCtx, longTmpName, ns, options));
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/database.h b/src/mongo/db/catalog/database.h
index 2b2edbac1dd..f27e2c8f3be 100644
--- a/src/mongo/db/catalog/database.h
+++ b/src/mongo/db/catalog/database.h
@@ -163,10 +163,10 @@ public:
      * Returns NamespaceExists if we are unable to generate a collection name that does not conflict
      * with an existing collection in this database.
      *
-     * The database must be locked in MODE_X when calling this function.
+     * The database must be locked in MODE_IX when calling this function.
      */
     virtual StatusWith<NamespaceString> makeUniqueCollectionNamespace(
-        OperationContext* opCtx, StringData collectionNameModel) = 0;
+        OperationContext* opCtx, StringData collectionNameModel) const = 0;
 
     /**
      * If we are in a replset, every replicated collection must have an _id index.  As we scan each
diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index 1fc42feb943..20456310cb0 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -100,6 +100,12 @@ Status validateDBNameForWindows(StringData dbname) {
                       str::stream() << "db name \"" << dbname << "\" is a reserved name");
     return Status::OK();
 }
+
+// Random number generator used to create unique collection namespaces suitable for temporary
+// collections.
+PseudoRandom uniqueCollectionNamespacePseudoRandom(Date_t::now().asInt64());
+
+Mutex uniqueCollectionNamespaceMutex = MONGO_MAKE_LATCH("DatabaseUniqueCollectionNamespaceMutex");
 }  // namespace
 
 Status DatabaseImpl::validateDBName(StringData dbname) {
@@ -126,8 +132,7 @@ DatabaseImpl::DatabaseImpl(const StringData name, uint64_t epoch)
     : _name(name.toString()),
       _epoch(epoch),
       _profileName(_name + ".system.profile"),
-      _viewsName(_name + "." + DurableViewCatalog::viewsCollectionName().toString()),
-      _uniqueCollectionNamespacePseudoRandom(Date_t::now().asInt64()) {
+      _viewsName(_name + "." + DurableViewCatalog::viewsCollectionName().toString()) {
     auto durableViewCatalog = std::make_unique<DurableViewCatalogImpl>(this);
     auto viewCatalog = std::make_unique<ViewCatalog>(std::move(durableViewCatalog));
 
@@ -763,7 +768,7 @@ Collection* DatabaseImpl::createCollection(OperationContext* opCtx,
 }
 
 StatusWith<NamespaceString> DatabaseImpl::makeUniqueCollectionNamespace(
-    OperationContext* opCtx, StringData collectionNameModel) {
+    OperationContext* opCtx, StringData collectionNameModel) const {
     invariant(opCtx->lockState()->isDbLockedForMode(name(), MODE_IX));
 
     // There must be at least one percent sign in the collection name model.
@@ -782,11 +787,13 @@ StatusWith<NamespaceString> DatabaseImpl::makeUniqueCollectionNamespace(
         "abcdefghijklmnopqrstuvwxyz"_sd;
     invariant((10U + 26U * 2) == charsToChooseFrom.size());
 
+    stdx::lock_guard<Latch> lk(uniqueCollectionNamespaceMutex);
+
     auto replacePercentSign = [&, this](char c) {
         if (c != '%') {
             return c;
         }
-        auto i = _uniqueCollectionNamespacePseudoRandom.nextInt32(charsToChooseFrom.size());
+        auto i = uniqueCollectionNamespacePseudoRandom.nextInt32(charsToChooseFrom.size());
         return charsToChooseFrom[i];
     };
 
diff --git a/src/mongo/db/catalog/database_impl.h b/src/mongo/db/catalog/database_impl.h
index 0b5e55d151e..5f8cd5472b2 100644
--- a/src/mongo/db/catalog/database_impl.h
+++ b/src/mongo/db/catalog/database_impl.h
@@ -31,8 +31,6 @@
 
 #include "mongo/db/catalog/database.h"
 
-#include "mongo/platform/random.h"
-
 namespace mongo {
 
 class DatabaseImpl final : public Database {
@@ -124,8 +122,8 @@ public:
      * Returns a NamespaceExists error status if multiple attempts fail to generate a possible
      * unique name.
      */
-    StatusWith<NamespaceString> makeUniqueCollectionNamespace(OperationContext* opCtx,
-                                                              StringData collectionNameModel) final;
+    StatusWith<NamespaceString> makeUniqueCollectionNamespace(
+        OperationContext* opCtx, StringData collectionNameModel) const final;
 
     void checkForIdIndexesAndDropPendingCollections(OperationContext* opCtx) const final;
 
@@ -179,10 +177,6 @@ private:
     // collections may be created in this Database.
     // This variable may only be read/written while the database is locked in MODE_X.
     AtomicWord<bool> _dropPending{false};
-
-    // Random number generator used to create unique collection namespaces suitable for temporary
-    // collections.
-    PseudoRandom _uniqueCollectionNamespacePseudoRandom;
 };
 
 }  // namespace mongo