diff options
author | Gregory Wlodarek <gregory.wlodarek@mongodb.com> | 2019-08-27 00:13:02 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-08-27 00:13:02 +0000 |
commit | 40f226b5a9bfb4863268334d287a46fb226a22cf (patch) | |
tree | 98b44b3a3bce352f30f0b05d264ab4e903b9aef9 /src | |
parent | dd2de577670e9461c31ca9e6fe5c9713b4401181 (diff) | |
download | mongo-40f226b5a9bfb4863268334d287a46fb226a22cf.tar.gz |
SERVER-33272 Proactively close newly empty databases
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/catalog/collection_catalog.h | 9 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_catalog_test.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/catalog/drop_collection.cpp | 34 | ||||
-rw-r--r-- | src/mongo/db/catalog/drop_collection.h | 6 | ||||
-rw-r--r-- | src/mongo/db/commands/sleep_command.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback.cpp | 19 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/storage_interface_impl.cpp | 28 | ||||
-rw-r--r-- | src/mongo/dbtests/plan_executor_invalidation_test.cpp | 8 | ||||
-rw-r--r-- | src/mongo/dbtests/rollbacktests.cpp | 8 |
10 files changed, 98 insertions, 25 deletions
diff --git a/src/mongo/db/catalog/collection_catalog.h b/src/mongo/db/catalog/collection_catalog.h index f3886c3fea0..308c31173c5 100644 --- a/src/mongo/db/catalog/collection_catalog.h +++ b/src/mongo/db/catalog/collection_catalog.h @@ -216,7 +216,7 @@ public: void onCloseCatalog(OperationContext* opCtx); /** - * Puts the catatlog back in open state, removing the pre-close state. See onCloseCatalog. + * Puts the catalog back in open state, removing the pre-close state. See onCloseCatalog. * * Must be called with the global lock acquired in exclusive mode. */ @@ -226,6 +226,13 @@ public: iterator end() const; /** + * Returns whether the database contains any collections. + */ + bool empty(StringData db) const { + return begin(db) == end(); + } + + /** * Lookup the name of a resource by its ResourceId. If there are multiple namespaces mapped to * the same ResourceId entry, we return the boost::none for those namespaces until there is * only one namespace in the set. If the ResourceId is not found, boost::none is returned. diff --git a/src/mongo/db/catalog/collection_catalog_test.cpp b/src/mongo/db/catalog/collection_catalog_test.cpp index 205a5647e75..0ae2f578d5f 100644 --- a/src/mongo/db/catalog/collection_catalog_test.cpp +++ b/src/mongo/db/catalog/collection_catalog_test.cpp @@ -64,8 +64,11 @@ public: auto collection = std::make_unique<CollectionMock>(nss); col = collection.get(); + ASSERT_TRUE(catalog.empty(nss.db())); + // Register dummy collection in catalog. catalog.registerCollection(colUUID, std::move(collection)); + ASSERT_FALSE(catalog.empty(nss.db())); } protected: diff --git a/src/mongo/db/catalog/drop_collection.cpp b/src/mongo/db/catalog/drop_collection.cpp index b5f30ae2a3e..18ab3311ca0 100644 --- a/src/mongo/db/catalog/drop_collection.cpp +++ b/src/mongo/db/catalog/drop_collection.cpp @@ -34,6 +34,7 @@ #include "mongo/db/catalog/drop_collection.h" #include "mongo/db/background.h" +#include "mongo/db/catalog/database_holder.h" #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/client.h" #include "mongo/db/concurrency/write_conflict_exception.h" @@ -147,6 +148,29 @@ Status _dropCollection(OperationContext* opCtx, return Status::OK(); } +void closeDatabaseIfEmpty(OperationContext* opCtx, StringData ns) { + invariant(opCtx->lockState()->isDbLockedForMode(ns, MODE_NONE)); + + bool empty = CollectionCatalog::get(opCtx).empty(ns); + if (!empty) { + return; + } + + AutoGetDb autoDb(opCtx, ns, MODE_X); + if (!autoDb.getDb()) { + return; + } + + // Double check that we're still empty. A new collection could've been created before we got the + // exclusive lock. + empty = CollectionCatalog::get(opCtx).empty(ns); + if (!empty) { + return; + } + + DatabaseHolder::get(opCtx)->close(opCtx, ns); +} + Status dropCollection(OperationContext* opCtx, const NamespaceString& collectionName, BSONObjBuilder& result, @@ -160,7 +184,7 @@ Status dropCollection(OperationContext* opCtx, log() << "Hanging drop collection before lock acquisition while fail point is set"; MONGO_FAIL_POINT_PAUSE_WHILE_SET(hangDropCollectionBeforeLockAcquisition); } - return writeConflictRetry(opCtx, "drop", collectionName.ns(), [&] { + Status status = writeConflictRetry(opCtx, "drop", collectionName.ns(), [&] { AutoGetDb autoDb(opCtx, collectionName.db(), MODE_IX); Database* db = autoDb.getDb(); if (!db) { @@ -175,6 +199,14 @@ Status dropCollection(OperationContext* opCtx, opCtx, db, collectionName, dropOpTime, systemCollectionMode, result); } }); + + if (!status.isOK()) { + return status; + } + + // If this dropped the last collection in the database, we should close the database. + closeDatabaseIfEmpty(opCtx, collectionName.db()); + return Status::OK(); } } // namespace mongo diff --git a/src/mongo/db/catalog/drop_collection.h b/src/mongo/db/catalog/drop_collection.h index 2646c632aa1..6b716e0666c 100644 --- a/src/mongo/db/catalog/drop_collection.h +++ b/src/mongo/db/catalog/drop_collection.h @@ -39,6 +39,12 @@ class OpTime; } // namespace repl /** + * Closes the specified database if it no longer has any collections remaining. Must be called + * without any locks acquired on the database. + */ +void closeDatabaseIfEmpty(OperationContext* opCtx, StringData ns); + +/** * Drops the collection "collectionName" and populates "result" with statistics about what * was removed. * diff --git a/src/mongo/db/commands/sleep_command.cpp b/src/mongo/db/commands/sleep_command.cpp index d2fd98c3180..1ac2cbb99bc 100644 --- a/src/mongo/db/commands/sleep_command.cpp +++ b/src/mongo/db/commands/sleep_command.cpp @@ -58,8 +58,8 @@ public: "If neither 'secs' nor 'millis' is set, command will sleep for 10 seconds. " "If both are set, command will sleep for the sum of 'secs' and 'millis.'\n" " w:<bool> (deprecated: use 'lock' instead) if true, takes a write lock.\n" - " lock: r, ir, w, iw, none. If r or w, db will block under a lock.\n" - " If ir or iw, db will block under an intent lock. Defaults to ir." + " lock: r, ir, w, iw, none. If r or w, db will sleep under an exclusive lock.\n" + " If ir or iw, db will sleep under an intent lock. Defaults to ir." " 'lock' and 'w' may not both be set.\n" " secs:<seconds> Amount of time to sleep, in seconds.\n" " millis:<milliseconds> Amount of time to sleep, in ms.\n" diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp index 1b685116242..1a7d03ef9e2 100644 --- a/src/mongo/db/repl/rs_rollback.cpp +++ b/src/mongo/db/repl/rs_rollback.cpp @@ -42,6 +42,7 @@ #include "mongo/db/catalog/collection_catalog.h" #include "mongo/db/catalog/database_holder.h" #include "mongo/db/catalog/document_validation.h" +#include "mongo/db/catalog/drop_collection.h" #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/catalog/rename_collection.h" #include "mongo/db/catalog_raii.h" @@ -1158,14 +1159,20 @@ void rollback_internal::syncFixUp(OperationContext* opCtx, log() << "This collection does not exist, UUID: " << uuid; } else { log() << "Dropping collection: " << *nss << ", UUID: " << uuid; - AutoGetDb dbLock(opCtx, nss->db(), MODE_X); - Database* db = dbLock.getDb(); - if (db) { - Collection* collection = CollectionCatalog::get(opCtx).lookupCollectionByUUID(uuid); - dropCollection(opCtx, *nss, collection, db); - LOG(1) << "Dropped collection: " << *nss << ", UUID: " << uuid; + { + AutoGetDb dbLock(opCtx, nss->db(), MODE_X); + + Database* db = dbLock.getDb(); + if (db) { + Collection* collection = + CollectionCatalog::get(opCtx).lookupCollectionByUUID(uuid); + dropCollection(opCtx, *nss, collection, db); + LOG(1) << "Dropped collection: " << *nss << ", UUID: " << uuid; + } } + + closeDatabaseIfEmpty(opCtx, nss->db()); } } diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp index 6408443af27..a4f5b6221dd 100644 --- a/src/mongo/db/repl/rs_rollback_test.cpp +++ b/src/mongo/db/repl/rs_rollback_test.cpp @@ -1648,9 +1648,7 @@ TEST_F(RSRollbackTest, RollbackCreateCollectionCommand) { { Lock::DBLock dbLock(_opCtx.get(), "test", MODE_S); auto databaseHolder = DatabaseHolder::get(_opCtx.get()); - auto db = databaseHolder->getDb(_opCtx.get(), "test"); - ASSERT_TRUE(db); - ASSERT_FALSE(db->getCollection(_opCtx.get(), NamespaceString("test.t"))); + ASSERT_FALSE(databaseHolder->getDb(_opCtx.get(), "test")); } } diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp index 88f4e2f36c7..ff17b6c7175 100644 --- a/src/mongo/db/repl/storage_interface_impl.cpp +++ b/src/mongo/db/repl/storage_interface_impl.cpp @@ -49,6 +49,7 @@ #include "mongo/db/catalog/collection_catalog.h" #include "mongo/db/catalog/database_holder.h" #include "mongo/db/catalog/document_validation.h" +#include "mongo/db/catalog/drop_collection.h" #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/client.h" #include "mongo/db/concurrency/d_concurrency.h" @@ -459,18 +460,23 @@ Status StorageInterfaceImpl::createCollection(OperationContext* opCtx, Status StorageInterfaceImpl::dropCollection(OperationContext* opCtx, const NamespaceString& nss) { return writeConflictRetry(opCtx, "StorageInterfaceImpl::dropCollection", nss.ns(), [&] { - AutoGetDb autoDb(opCtx, nss.db(), MODE_IX); - Lock::CollectionLock collLock(opCtx, nss, MODE_X); - if (!autoDb.getDb()) { - // Database does not exist - nothing to do. - return Status::OK(); - } - WriteUnitOfWork wunit(opCtx); - const auto status = autoDb.getDb()->dropCollectionEvenIfSystem(opCtx, nss); - if (!status.isOK()) { - return status; + { + AutoGetDb autoDb(opCtx, nss.db(), MODE_IX); + Lock::CollectionLock collLock(opCtx, nss, MODE_X); + if (!autoDb.getDb()) { + // Database does not exist - nothing to do. + return Status::OK(); + } + WriteUnitOfWork wunit(opCtx); + const auto status = autoDb.getDb()->dropCollectionEvenIfSystem(opCtx, nss); + if (!status.isOK()) { + return status; + } + wunit.commit(); } - wunit.commit(); + + // If this dropped the last collection in the database, we should close the database. + closeDatabaseIfEmpty(opCtx, nss.db()); return Status::OK(); }); } diff --git a/src/mongo/dbtests/plan_executor_invalidation_test.cpp b/src/mongo/dbtests/plan_executor_invalidation_test.cpp index 61eedeab72f..7a62f40afb8 100644 --- a/src/mongo/dbtests/plan_executor_invalidation_test.cpp +++ b/src/mongo/dbtests/plan_executor_invalidation_test.cpp @@ -59,6 +59,10 @@ static const NamespaceString nss("unittests.PlanExecutorInvalidationTest"); class PlanExecutorInvalidationTest : public unittest::Test { public: PlanExecutorInvalidationTest() : _client(&_opCtx) { + // Create an additional collection to prevent the database from closing when the other + // collection is dropped. + ASSERT_TRUE(_client.createCollection("unittests.PlanExecutorInvalidationTestUnused")); + _ctx.reset(new dbtests::WriteContextForTests(&_opCtx, nss.ns())); _client.dropCollection(nss.ns()); @@ -67,6 +71,10 @@ public: } } + ~PlanExecutorInvalidationTest() { + _client.dropCollection("unittests.PlanExecutorInvalidationTestUnused"); + } + /** * Return a plan executor that is going over the collection in nss.ns(). */ diff --git a/src/mongo/dbtests/rollbacktests.cpp b/src/mongo/dbtests/rollbacktests.cpp index fd48e6f46a3..d5e69fce2f3 100644 --- a/src/mongo/dbtests/rollbacktests.cpp +++ b/src/mongo/dbtests/rollbacktests.cpp @@ -53,7 +53,7 @@ namespace { const auto kIndexVersion = IndexDescriptor::IndexVersion::kV2; void dropDatabase(OperationContext* opCtx, const NamespaceString& nss) { - Lock::GlobalWrite globalWriteLock(opCtx); + AutoGetDb autoDB(opCtx, nss.db(), MODE_X); auto databaseHolder = DatabaseHolder::get(opCtx); auto db = databaseHolder->getDb(opCtx, nss.db()); @@ -394,6 +394,12 @@ public: assertGet(CollectionOptions::parse(BSONObj(), CollectionOptions::parseForCommand)); ASSERT_OK(ctx.db()->userCreateNS(&opCtx, nss, collectionOptions, defaultIndexes)); insertRecord(&opCtx, nss, oldDoc); + + // Create an additional collection to prevent the database from closing when the other + // collection is dropped. + NamespaceString unusedCollectionNSS("unittests.rollback_replace_collection_unused"); + ASSERT_OK(ctx.db()->userCreateNS( + &opCtx, unusedCollectionNSS, collectionOptions, defaultIndexes)); uow.commit(); } ASSERT(collectionExists(&opCtx, &ctx, nss.ns())); |