SERVER-33272 Proactively close newly empty databases

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()));