SERVER-73340 Drop abandoned idents at startup using the stable timestamp

With point-in-time catalog lookups, the index could have existed between the oldest and the stable timestamp. To allow reads in this window we defer the index drop until the stable timestamp using the two-phase drop reaper.

7 files changed, 35 insertions, 22 deletions

diff --git a/src/mongo/db/catalog/catalog_control.cpp b/src/mongo/db/catalog/catalog_control.cpp
index 95774bbe155..109ee48793c 100644
--- a/src/mongo/db/catalog/catalog_control.cpp
+++ b/src/mongo/db/catalog/catalog_control.cpp
@@ -254,9 +254,10 @@ void openCatalog(OperationContext* opCtx,
     storageEngine->loadCatalog(opCtx, stableTimestamp, StorageEngine::LastShutdownState::kClean);
 
     LOGV2(20274, "openCatalog: reconciling catalog and idents");
-    auto reconcileResult = fassert(
-        40688,
-        storageEngine->reconcileCatalogAndIdents(opCtx, StorageEngine::LastShutdownState::kClean));
+    auto reconcileResult =
+        fassert(40688,
+                storageEngine->reconcileCatalogAndIdents(
+                    opCtx, stableTimestamp, StorageEngine::LastShutdownState::kClean));
 
     // Determine which indexes need to be rebuilt. rebuildIndexesOnCollection() requires that all
     // indexes on that collection are done at once, so we use a map to group them together.
diff --git a/src/mongo/db/startup_recovery.cpp b/src/mongo/db/startup_recovery.cpp
index 9745ef353c9..57546bd2bca 100644
--- a/src/mongo/db/startup_recovery.cpp
+++ b/src/mongo/db/startup_recovery.cpp
@@ -340,8 +340,11 @@ void reconcileCatalogAndRebuildUnfinishedIndexes(
     OperationContext* opCtx,
     StorageEngine* storageEngine,
     StorageEngine::LastShutdownState lastShutdownState) {
+
     auto reconcileResult =
-        fassert(40593, storageEngine->reconcileCatalogAndIdents(opCtx, lastShutdownState));
+        fassert(40593,
+                storageEngine->reconcileCatalogAndIdents(
+                    opCtx, storageEngine->getStableTimestamp(), lastShutdownState));
 
     auto tempDir = boost::filesystem::path(storageGlobalParams.dbpath).append("_tmp");
     if (reconcileResult.indexBuildsToResume.empty() ||
diff --git a/src/mongo/db/storage/storage_engine.h b/src/mongo/db/storage/storage_engine.h
index 3c323017e70..ae718b5a925 100644
--- a/src/mongo/db/storage/storage_engine.h
+++ b/src/mongo/db/storage/storage_engine.h
@@ -628,8 +628,9 @@ public:
     };
 
     /**
-     * Drop abandoned idents. If successful, returns a ReconcileResult with indexes that need to be
-     * rebuilt or builds that need to be restarted.
+     * Drop abandoned idents using two-phase drop at the stable timestamp. Idents may be needed for
+     * reads between the oldest and stable timestamps. If successful, returns a ReconcileResult with
+     * indexes that need to be rebuilt or builds that need to be restarted.
      *
      * Abandoned internal idents require special handling based on the context known only to the
      * caller. For example, on starting from a previous unclean shutdown, we would always drop all
@@ -637,7 +638,7 @@ public:
      * information for resuming index builds.
      */
     virtual StatusWith<ReconcileResult> reconcileCatalogAndIdents(
-        OperationContext* opCtx, LastShutdownState lastShutdownState) = 0;
+        OperationContext* opCtx, Timestamp stableTs, LastShutdownState lastShutdownState) = 0;
 
     /**
      * Returns the all_durable timestamp. All transactions with timestamps earlier than the
diff --git a/src/mongo/db/storage/storage_engine_impl.cpp b/src/mongo/db/storage/storage_engine_impl.cpp
index 2c56c11ac81..6b96b63526b 100644
--- a/src/mongo/db/storage/storage_engine_impl.cpp
+++ b/src/mongo/db/storage/storage_engine_impl.cpp
@@ -50,6 +50,7 @@
 #include "mongo/db/storage/durable_history_pin.h"
 #include "mongo/db/storage/historical_ident_tracker.h"
 #include "mongo/db/storage/kv/kv_engine.h"
+#include "mongo/db/storage/storage_parameters_gen.h"
 #include "mongo/db/storage/storage_repair_observer.h"
 #include "mongo/db/storage/storage_util.h"
 #include "mongo/db/storage/two_phase_index_build_knobs_gen.h"
@@ -589,7 +590,7 @@ bool StorageEngineImpl::_handleInternalIdent(OperationContext* opCtx,
  * rebuild the index.
  */
 StatusWith<StorageEngine::ReconcileResult> StorageEngineImpl::reconcileCatalogAndIdents(
-    OperationContext* opCtx, LastShutdownState lastShutdownState) {
+    OperationContext* opCtx, Timestamp stableTs, LastShutdownState lastShutdownState) {
     // Gather all tables known to the storage engine and drop those that aren't cross-referenced
     // in the _mdb_catalog. This can happen for two reasons.
     //
@@ -656,16 +657,23 @@ StatusWith<StorageEngine::ReconcileResult> StorageEngineImpl::reconcileCatalogAn
         }
 
         const auto& toRemove = it;
-        LOGV2(22251, "Dropping unknown ident", "ident"_attr = toRemove);
-        WriteUnitOfWork wuow(opCtx);
-        Status status = _engine->dropIdent(opCtx->recoveryUnit(), toRemove);
-        if (!status.isOK()) {
-            // A concurrent operation, such as a checkpoint could be holding an open data handle on
-            // the ident. Handoff the ident drop to the ident reaper to retry later.
-            addDropPendingIdent(
-                Timestamp::min(), std::make_shared<Ident>(toRemove), /*onDrop=*/nullptr);
+        Timestamp identDropTs = feature_flags::gPointInTimeCatalogLookups.isEnabledAndIgnoreFCV()
+            ? stableTs
+            : Timestamp::min();
+        LOGV2(22251, "Dropping unknown ident", "ident"_attr = toRemove, "ts"_attr = identDropTs);
+        if (!identDropTs.isNull()) {
+            addDropPendingIdent(identDropTs, std::make_shared<Ident>(toRemove), /*onDrop=*/nullptr);
+        } else {
+            WriteUnitOfWork wuow(opCtx);
+            Status status = _engine->dropIdent(opCtx->recoveryUnit(), toRemove);
+            if (!status.isOK()) {
+                // A concurrent operation, such as a checkpoint could be holding an open data handle
+                // on the ident. Handoff the ident drop to the ident reaper to retry later.
+                addDropPendingIdent(
+                    identDropTs, std::make_shared<Ident>(toRemove), /*onDrop=*/nullptr);
+            }
+            wuow.commit();
         }
-        wuow.commit();
     }
 
     // Scan all collections in the catalog and make sure their ident is known to the storage
diff --git a/src/mongo/db/storage/storage_engine_impl.h b/src/mongo/db/storage/storage_engine_impl.h
index 6c28ce06338..670f88d806c 100644
--- a/src/mongo/db/storage/storage_engine_impl.h
+++ b/src/mongo/db/storage/storage_engine_impl.h
@@ -331,7 +331,7 @@ public:
                                                       CheckpointLock::Mode mode) override;
 
     StatusWith<ReconcileResult> reconcileCatalogAndIdents(
-        OperationContext* opCtx, LastShutdownState lastShutdownState) override;
+        OperationContext* opCtx, Timestamp stableTs, LastShutdownState lastShutdownState) override;
 
     std::string getFilesystemPathForDb(const DatabaseName& dbName) const override;
 
diff --git a/src/mongo/db/storage/storage_engine_mock.h b/src/mongo/db/storage/storage_engine_mock.h
index b80e426f3af..8bcefc5481d 100644
--- a/src/mongo/db/storage/storage_engine_mock.h
+++ b/src/mongo/db/storage/storage_engine_mock.h
@@ -155,7 +155,7 @@ public:
         OldestActiveTransactionTimestampCallback callback) final {}
 
     StatusWith<StorageEngine::ReconcileResult> reconcileCatalogAndIdents(
-        OperationContext* opCtx, LastShutdownState lastShutdownState) final {
+        OperationContext* opCtx, Timestamp stableTs, LastShutdownState lastShutdownState) final {
         return ReconcileResult{};
     }
     Timestamp getAllDurableTimestamp() const final {
diff --git a/src/mongo/db/storage/storage_engine_test_fixture.h b/src/mongo/db/storage/storage_engine_test_fixture.h
index f5537d231e1..064969715ab 100644
--- a/src/mongo/db/storage/storage_engine_test_fixture.h
+++ b/src/mongo/db/storage/storage_engine_test_fixture.h
@@ -116,14 +116,14 @@ public:
 
     StatusWith<StorageEngine::ReconcileResult> reconcile(OperationContext* opCtx) {
         Lock::GlobalLock globalLock{opCtx, MODE_IX};
-        return _storageEngine->reconcileCatalogAndIdents(opCtx,
-                                                         StorageEngine::LastShutdownState::kClean);
+        return _storageEngine->reconcileCatalogAndIdents(
+            opCtx, Timestamp::min(), StorageEngine::LastShutdownState::kClean);
     }
 
     StatusWith<StorageEngine::ReconcileResult> reconcileAfterUncleanShutdown(
         OperationContext* opCtx) {
         return _storageEngine->reconcileCatalogAndIdents(
-            opCtx, StorageEngine::LastShutdownState::kUnclean);
+            opCtx, Timestamp::min(), StorageEngine::LastShutdownState::kUnclean);
     }
 
     std::vector<std::string> getAllKVEngineIdents(OperationContext* opCtx) {