diff options
author | Gregory Noma <gregory.noma@gmail.com> | 2020-04-10 14:30:30 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-04-10 18:42:34 +0000 |
commit | f5c988bbbcc5251f2ce8726ef399092b037245fe (patch) | |
tree | 5b4382321a0df3a4744293f5124148312d8d244e | |
parent | 494017bdd1cd12f66e44a5338d4a4a2d94578f35 (diff) | |
download | mongo-f5c988bbbcc5251f2ce8726ef399092b037245fe.tar.gz |
SERVER-47423 Take collection MODE_X locks instead of databse MODE_X lock in cloneCollectionAsCapped
(cherry picked from commit 4adcb62e1be77edfd448e3091d307a368181cc9e)
-rw-r--r-- | jstests/noPassthroughWithMongod/clone_collection_as_capped_no_conflicts.js | 44 | ||||
-rw-r--r-- | src/mongo/db/catalog/capped_utils.cpp | 15 | ||||
-rw-r--r-- | src/mongo/db/catalog/capped_utils.h | 4 | ||||
-rw-r--r-- | src/mongo/db/commands/collection_to_capped.cpp | 14 |
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; } |