SERVER-49301 StorageEngine::loadCatalog() accepts previous shutdown state rather than checking decorator for handling orphaned idents

6 files changed, 30 insertions, 19 deletions

diff --git a/src/mongo/db/catalog/catalog_control.cpp b/src/mongo/db/catalog/catalog_control.cpp
index ed052ee2bf6..4d433798cb1 100644
--- a/src/mongo/db/catalog/catalog_control.cpp
+++ b/src/mongo/db/catalog/catalog_control.cpp
@@ -110,7 +110,10 @@ void openCatalog(OperationContext* opCtx, const MinVisibleTimestampMap& minVisib
     // Load the catalog in the storage engine.
     LOGV2(20273, "openCatalog: loading storage engine catalog");
     auto storageEngine = opCtx->getServiceContext()->getStorageEngine();
-    storageEngine->loadCatalog(opCtx);
+    // Ignore orphaned idents because this function is used during rollback and not at
+    // startup recovery, when we may try to recover orphaned idents.
+    auto loadingFromUncleanShutdown = false;
+    storageEngine->loadCatalog(opCtx, loadingFromUncleanShutdown);
 
     LOGV2(20274, "openCatalog: reconciling catalog and idents");
     // Retain unknown internal idents because this function is used during rollback and not at
diff --git a/src/mongo/db/storage/kv/storage_engine_test.cpp b/src/mongo/db/storage/kv/storage_engine_test.cpp
index af98cb84f70..9c2774215df 100644
--- a/src/mongo/db/storage/kv/storage_engine_test.cpp
+++ b/src/mongo/db/storage/kv/storage_engine_test.cpp
@@ -48,7 +48,6 @@
 #include "mongo/db/storage/storage_engine_impl.h"
 #include "mongo/db/storage/storage_engine_test_fixture.h"
 #include "mongo/db/storage/storage_repair_observer.h"
-#include "mongo/db/unclean_shutdown.h"
 #include "mongo/unittest/barrier.h"
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/periodic_runner_factory.h"
@@ -124,8 +123,8 @@ TEST_F(StorageEngineTest, LoadCatalogDropsOrphansAfterUncleanShutdown) {
     {
         Lock::GlobalWrite writeLock(opCtx.get(), Date_t::max(), Lock::InterruptBehavior::kThrow);
         _storageEngine->closeCatalog(opCtx.get());
-        startingAfterUncleanShutdown(getGlobalServiceContext()) = true;
-        _storageEngine->loadCatalog(opCtx.get());
+        auto loadingFromUncleanShutdown = true;
+        _storageEngine->loadCatalog(opCtx.get(), loadingFromUncleanShutdown);
     }
 
     ASSERT(!identExists(opCtx.get(), swCollInfo.getValue().ident));
@@ -359,7 +358,8 @@ TEST_F(StorageEngineRepairTest, LoadCatalogRecoversOrphans) {
     {
         Lock::GlobalWrite writeLock(opCtx.get(), Date_t::max(), Lock::InterruptBehavior::kThrow);
         _storageEngine->closeCatalog(opCtx.get());
-        _storageEngine->loadCatalog(opCtx.get());
+        auto loadingFromUncleanShutdown = false;
+        _storageEngine->loadCatalog(opCtx.get(), loadingFromUncleanShutdown);
     }
 
     ASSERT(identExists(opCtx.get(), swCollInfo.getValue().ident));
@@ -412,7 +412,8 @@ TEST_F(StorageEngineRepairTest, LoadCatalogRecoversOrphansInCatalog) {
     ASSERT(!collectionExists(opCtx.get(), collNs));
 
     // When in a repair context, loadCatalog() recreates catalog entries for orphaned idents.
-    _storageEngine->loadCatalog(opCtx.get());
+    auto loadingFromUncleanShutdown = false;
+    _storageEngine->loadCatalog(opCtx.get(), loadingFromUncleanShutdown);
     auto identNs = swCollInfo.getValue().ident;
     std::replace(identNs.begin(), identNs.end(), '-', '_');
     NamespaceString orphanNs = NamespaceString("local.orphan." + identNs);
@@ -445,7 +446,8 @@ TEST_F(StorageEngineTest, LoadCatalogDropsOrphans) {
 
     // When in a normal startup context, loadCatalog() does not recreate catalog entries for
     // orphaned idents.
-    _storageEngine->loadCatalog(opCtx.get());
+    auto loadingFromUncleanShutdown = false;
+    _storageEngine->loadCatalog(opCtx.get(), loadingFromUncleanShutdown);
     // reconcileCatalogAndIdents() drops orphaned idents.
     auto reconcileResult = unittest::assertGet(reconcile(opCtx.get()));
     ASSERT_EQUALS(0UL, reconcileResult.indexesToRebuild.size());
diff --git a/src/mongo/db/storage/storage_engine.h b/src/mongo/db/storage/storage_engine.h
index 0a21f9b4dbc..b7be1372d36 100644
--- a/src/mongo/db/storage/storage_engine.h
+++ b/src/mongo/db/storage/storage_engine.h
@@ -219,8 +219,12 @@ public:
      * engines that support recoverToStableTimestamp().
      *
      * Must be called with the global lock acquired in exclusive mode.
+     *
+     * Unrecognized idents require special handling based on the context known only to the
+     * caller. For example, on starting from a previous unclean shutdown, we may try to recover
+     * orphaned idents, which are known to the storage engine but not referenced in the catalog.
      */
-    virtual void loadCatalog(OperationContext* opCtx) = 0;
+    virtual void loadCatalog(OperationContext* opCtx, bool loadingFromUncleanShutdown) = 0;
     virtual void closeCatalog(OperationContext* opCtx) = 0;
 
     /**
diff --git a/src/mongo/db/storage/storage_engine_impl.cpp b/src/mongo/db/storage/storage_engine_impl.cpp
index 8611dd9fe0a..37264db0224 100644
--- a/src/mongo/db/storage/storage_engine_impl.cpp
+++ b/src/mongo/db/storage/storage_engine_impl.cpp
@@ -47,7 +47,6 @@
 #include "mongo/db/storage/kv/temporary_kv_record_store.h"
 #include "mongo/db/storage/storage_repair_observer.h"
 #include "mongo/db/storage/two_phase_index_build_knobs_gen.h"
-#include "mongo/db/unclean_shutdown.h"
 #include "mongo/logv2/log.h"
 #include "mongo/stdx/unordered_map.h"
 #include "mongo/util/assert_util.h"
@@ -81,10 +80,13 @@ StorageEngineImpl::StorageEngineImpl(std::unique_ptr<KVEngine> engine, StorageEn
             !(_options.directoryPerDB && !_engine->supportsDirectoryPerDB()));
 
     OperationContextNoop opCtx(_engine->newRecoveryUnit());
-    loadCatalog(&opCtx);
+    // If we are loading the catalog after an unclean shutdown, it's possible that there are
+    // collections in the catalog that are unknown to the storage engine. We should attempt to
+    // recover these orphaned idents.
+    loadCatalog(&opCtx, _options.lockFileCreatedByUncleanShutdown);
 }
 
-void StorageEngineImpl::loadCatalog(OperationContext* opCtx) {
+void StorageEngineImpl::loadCatalog(OperationContext* opCtx, bool loadingFromUncleanShutdown) {
     bool catalogExists = _engine->hasIdent(opCtx, catalogInfo);
     if (_options.forRepair && catalogExists) {
         auto repairObserver = StorageRepairObserver::get(getGlobalServiceContext());
@@ -128,10 +130,11 @@ void StorageEngineImpl::loadCatalog(OperationContext* opCtx) {
         _catalogRecordStore.get(), _options.directoryPerDB, _options.directoryForIndexes, this));
     _catalog->init(opCtx);
 
-    // We populate 'identsKnownToStorageEngine' only if we are loading after an unclean shutdown or
-    // doing repair.
-    const bool loadingFromUncleanShutdownOrRepair =
-        startingAfterUncleanShutdown(getGlobalServiceContext()) || _options.forRepair;
+    // We populate 'identsKnownToStorageEngine' only if:
+    // - doing repair; or
+    // - or asked to recover orphaned idents, which is the case when loading after an unclean
+    //   shutdown.
+    auto loadingFromUncleanShutdownOrRepair = loadingFromUncleanShutdown || _options.forRepair;
 
     std::vector<std::string> identsKnownToStorageEngine;
     if (loadingFromUncleanShutdownOrRepair) {
diff --git a/src/mongo/db/storage/storage_engine_impl.h b/src/mongo/db/storage/storage_engine_impl.h
index e00b5eb5bb8..316a02c3042 100644
--- a/src/mongo/db/storage/storage_engine_impl.h
+++ b/src/mongo/db/storage/storage_engine_impl.h
@@ -324,10 +324,9 @@ public:
     std::string getFilesystemPathForDb(const std::string& dbName) const override;
 
     /**
-     * When loading after an unclean shutdown, this performs cleanup on the DurableCatalogImpl and
-     * unsets the startingAfterUncleanShutdown decoration on the global ServiceContext.
+     * When loading after an unclean shutdown, this performs cleanup on the DurableCatalogImpl.
      */
-    void loadCatalog(OperationContext* opCtx) final;
+    void loadCatalog(OperationContext* opCtx, bool loadingFromUncleanShutdown) final;
 
     void closeCatalog(OperationContext* opCtx) final;
 
diff --git a/src/mongo/db/storage/storage_engine_mock.h b/src/mongo/db/storage/storage_engine_mock.h
index 7ffd0e7fc24..b1d24776235 100644
--- a/src/mongo/db/storage/storage_engine_mock.h
+++ b/src/mongo/db/storage/storage_engine_mock.h
@@ -60,7 +60,7 @@ public:
     bool isEphemeral() const final {
         return true;
     }
-    void loadCatalog(OperationContext* opCtx) final {}
+    void loadCatalog(OperationContext* opCtx, bool loadingFromUncleanShutdown) final {}
     void closeCatalog(OperationContext* opCtx) final {}
     Status closeDatabase(OperationContext* opCtx, StringData db) final {
         return Status::OK();