From 8393d898d70421a944adfb39835ec82fe3e3b860 Mon Sep 17 00:00:00 2001
From: Maria van Keulen <maria@mongodb.com>
Date: Tue, 10 Mar 2020 13:32:51 -0400
Subject: SERVER-46761 Don't skip dbs with uncommitted collections during
 lookup

---
 src/mongo/db/catalog/collection_catalog.cpp      |  6 +++
 src/mongo/db/catalog/collection_catalog_test.cpp | 59 ++++++++++++++++++++++++
 src/mongo/db/catalog/collection_mock.h           |  9 ++++
 3 files changed, 74 insertions(+)

(limited to 'src')

diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index 007d99945ee..ec80653054a 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -405,7 +405,13 @@ std::vector<std::string> CollectionCatalog::getAllDbNames() const {
         auto dbName = iter->first.first;
         if (iter->second->isCommitted()) {
             ret.push_back(dbName);
+        } else {
+            // If the first collection found for `dbName` is not yet committed, increment the
+            // iterator to find the next visible collection (possibly under a different `dbName`).
+            iter++;
+            continue;
         }
+        // Move on to the next database after `dbName`.
         iter = _orderedCollections.upper_bound(std::make_pair(dbName, maxUuid));
     }
     return ret;
diff --git a/src/mongo/db/catalog/collection_catalog_test.cpp b/src/mongo/db/catalog/collection_catalog_test.cpp
index 55142418bad..573ee9f5b3e 100644
--- a/src/mongo/db/catalog/collection_catalog_test.cpp
+++ b/src/mongo/db/catalog/collection_catalog_test.cpp
@@ -608,6 +608,65 @@ TEST_F(CollectionCatalogTest, GetAllCollectionNamesAndGetAllDbNames) {
     catalog.deregisterAllCollections();
 }
 
+TEST_F(CollectionCatalogTest, GetAllCollectionNamesAndGetAllDbNamesWithUncommittedCollections) {
+    NamespaceString aColl("dbA", "collA");
+    NamespaceString b1Coll("dbB", "collB1");
+    NamespaceString b2Coll("dbB", "collB2");
+    NamespaceString cColl("dbC", "collC");
+    NamespaceString d1Coll("dbD", "collD1");
+    NamespaceString d2Coll("dbD", "collD2");
+    NamespaceString d3Coll("dbD", "collD3");
+
+    std::vector<NamespaceString> nsss = {aColl, b1Coll, b2Coll, cColl, d1Coll, d2Coll, d3Coll};
+    for (auto& nss : nsss) {
+        std::unique_ptr<Collection> newColl = std::make_unique<CollectionMock>(nss);
+        auto uuid = CollectionUUID::gen();
+        catalog.registerCollection(uuid, &newColl);
+    }
+
+    // One dbName with only an invisible collection does not appear in dbNames.
+    auto invisibleCollA = catalog.lookupCollectionByNamespace(&opCtx, aColl);
+    invisibleCollA->setCommitted(false);
+
+    auto res = catalog.getAllCollectionNamesFromDb(&opCtx, "dbA");
+    ASSERT(res.empty());
+
+    std::vector<std::string> dbNames = {"dbB", "dbC", "dbD", "testdb"};
+    ASSERT(catalog.getAllDbNames() == dbNames);
+
+    // One dbName with both visible and invisible collections is still visible.
+    std::vector<NamespaceString> dbDNss = {d1Coll, d2Coll, d3Coll};
+    for (auto& nss : dbDNss) {
+        // Test each combination of one collection in dbD being invisible while the other two are
+        // visible.
+        std::vector<NamespaceString> dCollList = dbDNss;
+        dCollList.erase(std::find(dCollList.begin(), dCollList.end(), nss));
+
+        auto invisibleCollD = catalog.lookupCollectionByNamespace(&opCtx, nss);
+        invisibleCollD->setCommitted(false);
+
+        res = catalog.getAllCollectionNamesFromDb(&opCtx, "dbD");
+        std::sort(res.begin(), res.end());
+        ASSERT(res == dCollList);
+
+        ASSERT(catalog.getAllDbNames() == dbNames);
+        invisibleCollD->setCommitted(true);
+    }
+
+    invisibleCollA->setCommitted(true);  // reset visibility.
+
+    // If all dbNames consist only of invisible collections, none of these dbs is visible.
+    for (auto& nss : nsss) {
+        auto invisibleColl = catalog.lookupCollectionByNamespace(&opCtx, nss);
+        invisibleColl->setCommitted(false);
+    }
+
+    std::vector<std::string> dbList = {"testdb"};
+    ASSERT(catalog.getAllDbNames() == dbList);
+
+    catalog.deregisterAllCollections();
+}
+
 class ForEachCollectionFromDbTest : public CatalogTestFixture {
 public:
     void createTestData() {
diff --git a/src/mongo/db/catalog/collection_mock.h b/src/mongo/db/catalog/collection_mock.h
index ecab3451f6d..9e3f97f1cc5 100644
--- a/src/mongo/db/catalog/collection_mock.h
+++ b/src/mongo/db/catalog/collection_mock.h
@@ -279,6 +279,14 @@ public:
         return _uuid;
     }
 
+    bool isCommitted() const final {
+        return _committed;
+    }
+
+    void setCommitted(bool val) final {
+        _committed = val;
+    }
+
     void indexBuildSuccess(OperationContext* opCtx, IndexCatalogEntry* index) {
         std::abort();
     }
@@ -288,6 +296,7 @@ private:
     NamespaceString _ns;
     RecordId _catalogId{0};
     std::unique_ptr<IndexCatalog> _indexCatalog;
+    bool _committed = true;
 };
 
 }  // namespace mongo
-- 
cgit v1.2.1