diff options
author | Benety Goh <benety@mongodb.com> | 2017-09-02 22:24:19 -0400 |
---|---|---|
committer | Benety Goh <benety@mongodb.com> | 2017-09-05 21:41:53 -0400 |
commit | fa2f40a44ea649b801bfa3ba2bbeb0d36020629c (patch) | |
tree | d240cbdfb89ace85daf057884ce6068ce57c1a65 | |
parent | f85b99b90e754efa8d806d6124c902447ddc7481 (diff) | |
download | mongo-fa2f40a44ea649b801bfa3ba2bbeb0d36020629c.tar.gz |
SERVER-30371 downgrade global write lock when renaming across databases
This is done after creating the temporary collection and indexes and before
copying the documents from the source collection.
-rw-r--r-- | src/mongo/db/catalog/rename_collection.cpp | 23 | ||||
-rw-r--r-- | src/mongo/db/catalog/rename_collection_test.cpp | 25 |
2 files changed, 47 insertions, 1 deletions
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index f751accff49..bd603ae2bf6 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -85,9 +85,10 @@ Status renameCollectionCommon(OperationContext* opCtx, boost::optional<Lock::DBLock> dbWriteLock; // If the rename is known not to be a cross-database rename, just a database lock suffices. + auto lockState = opCtx->lockState(); if (source.db() == target.db()) dbWriteLock.emplace(opCtx, source.db(), MODE_X); - else + else if (!lockState->isW()) globalWriteLock.emplace(opCtx); // We stay in source context the whole time. This is mostly to set the CurOp namespace. @@ -334,6 +335,25 @@ Status renameCollectionCommon(OperationContext* opCtx, { // Copy over all the data from source collection to temporary collection. + // We do not need global write exclusive access after obtaining the collection locks on the + // source and temporary collections. After copying the documents, each remaining stage of + // the cross-database rename will be responsible for its own lock management. + // Therefore, unless the caller has already acquired the global write lock prior to invoking + // this function, we relinquish global write access to the database after acquiring the + // collection locks. + // Collection locks must be obtained while holding the global lock to avoid any possibility + // of a deadlock. + AutoGetCollectionForRead autoSourceColl(opCtx, source); + AutoGetCollection autoTmpColl(opCtx, tmpName, MODE_IX); + ctx.reset(); + if (globalWriteLock) { + const ResourceId globalLockResourceId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); + lockState->downgrade(globalLockResourceId, MODE_IX); + invariant(!lockState->isW()); + } else { + invariant(lockState->isW()); + } + auto cursor = sourceColl->getCursor(opCtx); while (auto record = cursor->next()) { opCtx->checkForInterrupt(); @@ -356,6 +376,7 @@ Status renameCollectionCommon(OperationContext* opCtx, } } } + globalWriteLock.reset(); // Getting here means we successfully built the target copy. We now do the final // in-place rename and remove the source collection. diff --git a/src/mongo/db/catalog/rename_collection_test.cpp b/src/mongo/db/catalog/rename_collection_test.cpp index 9e8f5ba2ac6..c0bfc7c792e 100644 --- a/src/mongo/db/catalog/rename_collection_test.cpp +++ b/src/mongo/db/catalog/rename_collection_test.cpp @@ -106,6 +106,7 @@ public: std::vector<std::string> oplogEntries; bool onInsertsThrows = false; + bool onInsertsIsGlobalWriteLockExclusive = false; bool onRenameCollectionCalled = false; repl::OpTime renameOpTime = {Timestamp(Seconds(100), 1U), 1LL}; @@ -138,6 +139,11 @@ void OpObserverMock::onInserts(OperationContext* opCtx, uasserted(ErrorCodes::OperationFailed, "insert failed"); } + // Check global lock state. + auto lockState = opCtx->lockState(); + ASSERT_TRUE(lockState->isWriteLocked()); + onInsertsIsGlobalWriteLockExclusive = lockState->isW(); + _logOp(opCtx, nss, "inserts"); OpObserverNoop::onInserts(opCtx, nss, uuid, begin, end, fromMigrate); } @@ -768,4 +774,23 @@ TEST_F(RenameCollectionTest, RenameCollectionAcrossDatabaseDropsTemporaryCollect _checkOplogEntries(_opObserver->oplogEntries, {"create", "index", "drop"}); } +TEST_F(RenameCollectionTest, + RenameCollectionAcrossDatabaseDowngradesGlobalWriteLockToNonExclusive) { + _createCollection(_opCtx.get(), _sourceNss); + _insertDocument(_opCtx.get(), _sourceNss, BSON("_id" << 0)); + ASSERT_OK(renameCollection(_opCtx.get(), _sourceNss, _targetNssDifferentDb, {})); + ASSERT_FALSE(_opObserver->onInsertsIsGlobalWriteLockExclusive); +} + +TEST_F(RenameCollectionTest, + RenameCollectionAcrossDatabaseKeepsGlobalWriteLockExclusiveIfCallerHasGlobalWriteLock) { + // This simulates the case when renameCollection is called using the applyOps command (different + // from secondary oplog application). + _createCollection(_opCtx.get(), _sourceNss); + _insertDocument(_opCtx.get(), _sourceNss, BSON("_id" << 0)); + Lock::GlobalWrite globalWrite(_opCtx.get()); + ASSERT_OK(renameCollection(_opCtx.get(), _sourceNss, _targetNssDifferentDb, {})); + ASSERT_TRUE(_opObserver->onInsertsIsGlobalWriteLockExclusive); +} + } // namespace |