SERVER-47423 Take collection MODE_X locks instead of databse MODE_X lock in cloneCollectionAsCapped

4 files changed, 61 insertions, 16 deletions

diff --git a/jstests/noPassthroughWithMongod/clone_collection_as_capped_no_conflicts.js b/jstests/noPassthroughWithMongod/clone_collection_as_capped_no_conflicts.js
new file mode 100644
index 00000000000..b1deedfa1d0
--- /dev/null
+++ b/jstests/noPassthroughWithMongod/clone_collection_as_capped_no_conflicts.js
@@ -0,0 +1,44 @@
+/**
+ * Tests that cloneCollectionAsCapped 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 baseName = "clone_collection_as_capped_no_conflicts";
+const fromCollName = baseName + "_from";
+const toCollName = baseName + "_to";
+
+const testDB = db.getSiblingDB("test");
+testDB.dropDatabase();
+const fromColl = testDB.getCollection(fromCollName);
+const toColl = testDB.getCollection(toCollName);
+
+const sleepFunction = function(sleepDB) {
+    // If cloneCollectionAsCapped 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(fromColl.insert({a: 1}));
+assert(!fromColl.isCapped());
+assert.commandWorked(testDB.runCommand(
+    {cloneCollectionAsCapped: fromCollName, toCollection: toCollName, size: 100}));
+assert(toColl.isCapped());
+assert.eq(toColl.count(), 1);
+
+// 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 9c45c28eea4..d34b707ac9c 100644
--- a/src/mongo/db/catalog/capped_utils.cpp
+++ b/src/mongo/db/catalog/capped_utils.cpp
@@ -45,6 +45,7 @@
 #include "mongo/db/concurrency/write_conflict_exception.h"
 #include "mongo/db/curop.h"
 #include "mongo/db/index_builds_coordinator.h"
+#include "mongo/db/namespace_string.h"
 #include "mongo/db/op_observer.h"
 #include "mongo/db/query/internal_plans.h"
 #include "mongo/db/query/plan_yield_policy.h"
@@ -111,13 +112,10 @@ Status emptyCapped(OperationContext* opCtx, const NamespaceString& collectionNam
 
 void cloneCollectionAsCapped(OperationContext* opCtx,
                              Database* db,
-                             const std::string& shortFrom,
-                             const std::string& shortTo,
+                             const NamespaceString& fromNss,
+                             const NamespaceString& toNss,
                              long long size,
                              bool temp) {
-    NamespaceString fromNss(db->name(), shortFrom);
-    NamespaceString toNss(db->name(), shortTo);
-
     Collection* fromCollection =
         CollectionCatalog::get(opCtx).lookupCollectionByNamespace(opCtx, fromNss);
     if (!fromCollection) {
@@ -261,7 +259,7 @@ void convertToCapped(OperationContext* opCtx, const NamespaceString& ns, long lo
 
     // Generate a temporary collection name that will not collide with any existing collections.
     boost::optional<Lock::CollectionLock> collLock;
-    const auto longTmpName = [&] {
+    const auto tempNs = [&] {
         while (true) {
             auto tmpNameResult =
                 db->makeUniqueCollectionNamespace(opCtx, "tmp%%%%%.convertToCapped." + shortSource);
@@ -282,14 +280,13 @@ void convertToCapped(OperationContext* opCtx, const NamespaceString& ns, long lo
             collLock.reset();
         }
     }();
-    const auto shortTmpName = longTmpName.coll().toString();
 
-    cloneCollectionAsCapped(opCtx, db, shortSource.toString(), shortTmpName, size, true);
+    cloneCollectionAsCapped(opCtx, db, ns, tempNs, size, true);
 
     RenameCollectionOptions options;
     options.dropTarget = true;
     options.stayTemp = false;
-    uassertStatusOK(renameCollection(opCtx, longTmpName, ns, options));
+    uassertStatusOK(renameCollection(opCtx, tempNs, ns, options));
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/capped_utils.h b/src/mongo/db/catalog/capped_utils.h
index 59e0819d634..ae77850ceea 100644
--- a/src/mongo/db/catalog/capped_utils.h
+++ b/src/mongo/db/catalog/capped_utils.h
@@ -44,8 +44,8 @@ Status emptyCapped(OperationContext* opCtx, const NamespaceString& collectionNam
  */
 void cloneCollectionAsCapped(OperationContext* opCtx,
                              Database* db,
-                             const std::string& shortFrom,
-                             const std::string& shortTo,
+                             const NamespaceString& fromNss,
+                             const NamespaceString& toNss,
                              long long size,
                              bool temp);
 
diff --git a/src/mongo/db/commands/collection_to_capped.cpp b/src/mongo/db/commands/collection_to_capped.cpp
index 42b1ff38dfe..9cefe29e593 100644
--- a/src/mongo/db/commands/collection_to_capped.cpp
+++ b/src/mongo/db/commands/collection_to_capped.cpp
@@ -35,6 +35,7 @@
 #include "mongo/db/client.h"
 #include "mongo/db/commands.h"
 #include "mongo/db/db_raii.h"
+#include "mongo/db/namespace_string.h"
 #include "mongo/db/op_observer.h"
 #include "mongo/db/query/find.h"
 #include "mongo/db/query/internal_plans.h"
@@ -112,22 +113,25 @@ public:
             return false;
         }
 
-        AutoGetDb autoDb(opCtx, dbname, MODE_X);
+        NamespaceString fromNs(dbname, from);
+        NamespaceString toNs(dbname, to);
 
-        NamespaceString nss(dbname, to);
-        if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, nss)) {
+        AutoGetCollection autoColl(opCtx, fromNs, MODE_X);
+        Lock::CollectionLock collLock(opCtx, toNs, MODE_X);
+
+        if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, toNs)) {
             uasserted(ErrorCodes::NotMaster,
                       str::stream() << "Not primary while cloning collection " << from << " to "
                                     << to << " (as capped)");
         }
 
-        Database* const db = autoDb.getDb();
+        Database* const db = autoColl.getDb();
         if (!db) {
             uasserted(ErrorCodes::NamespaceNotFound,
                       str::stream() << "database " << dbname << " not found");
         }
 
-        cloneCollectionAsCapped(opCtx, db, from.toString(), to.toString(), size, temp);
+        cloneCollectionAsCapped(opCtx, db, fromNs, toNs, size, temp);
         return true;
     }