SERVER-68269 Remove minimum visible snapshot for indexes

11 files changed, 0 insertions, 187 deletions

diff --git a/src/mongo/db/catalog/collection_catalog_test.cpp b/src/mongo/db/catalog/collection_catalog_test.cpp
index 227ec0ca5b0..0f1ab9ab043 100644
--- a/src/mongo/db/catalog/collection_catalog_test.cpp
+++ b/src/mongo/db/catalog/collection_catalog_test.cpp
@@ -1372,14 +1372,6 @@ TEST_F(CollectionCatalogTimestampTest, MinimumValidSnapshot) {
     ASSERT_EQ(coll->getMinimumVisibleSnapshot(), createCollectionTs);
     ASSERT_EQ(coll->getMinimumValidSnapshot(), createYIndexTs);
 
-    const IndexDescriptor* desc = coll->getIndexCatalog()->findIndexByName(opCtx.get(), "x_1");
-    const IndexCatalogEntry* entry = coll->getIndexCatalog()->getEntry(desc);
-    ASSERT_EQ(entry->getMinimumVisibleSnapshot(), createXIndexTs);
-
-    desc = coll->getIndexCatalog()->findIndexByName(opCtx.get(), "y_1");
-    entry = coll->getIndexCatalog()->getEntry(desc);
-    ASSERT_EQ(entry->getMinimumVisibleSnapshot(), createYIndexTs);
-
     dropIndex(opCtx.get(), nss, "x_1", dropIndexTs);
     dropIndex(opCtx.get(), nss, "y_1", dropIndexTs);
 
diff --git a/src/mongo/db/catalog/index_build_block.cpp b/src/mongo/db/catalog/index_build_block.cpp
index 584331ef3ea..54dad130d58 100644
--- a/src/mongo/db/catalog/index_build_block.cpp
+++ b/src/mongo/db/catalog/index_build_block.cpp
@@ -183,17 +183,6 @@ Status IndexBuildBlock::init(OperationContext* opCtx, Collection* collection, bo
         indexCatalogEntry->setIndexBuildInterceptor(_indexBuildInterceptor.get());
     }
 
-    if (isBackgroundIndex) {
-        opCtx->recoveryUnit()->onCommit(
-            [entry = indexCatalogEntry, coll = collection](OperationContext*,
-                                                           boost::optional<Timestamp> commitTime) {
-                // This will prevent the unfinished index from being visible on index iterators.
-                if (commitTime) {
-                    entry->setMinimumVisibleSnapshot(commitTime.value());
-                }
-            });
-    }
-
     _completeInit(opCtx, collection);
 
     return Status::OK();
@@ -279,10 +268,6 @@ void IndexBuildBlock::success(OperationContext* opCtx, Collection* collection) {
                   "collectionIdent"_attr = coll->getSharedIdent()->getIdent(),
                   "commitTimestamp"_attr = commitTime);
 
-            if (commitTime) {
-                entry->setMinimumVisibleSnapshot(commitTime.value());
-            }
-
             // Add the index to the TTLCollectionCache upon successfully committing the index build.
             // TTL indexes are not compatible with capped collections.  Note that TTL deletion is
             // supported on capped clustered collections via bounded collection scan, which does not
diff --git a/src/mongo/db/catalog/index_catalog.cpp b/src/mongo/db/catalog/index_catalog.cpp
index 686091ac564..9775d1477e2 100644
--- a/src/mongo/db/catalog/index_catalog.cpp
+++ b/src/mongo/db/catalog/index_catalog.cpp
@@ -64,37 +64,9 @@ ReadyIndexesIterator::ReadyIndexesIterator(OperationContext* const opCtx,
     : _opCtx(opCtx), _iterator(beginIterator), _endIterator(endIterator) {}
 
 const IndexCatalogEntry* ReadyIndexesIterator::_advance() {
-    // (Ignore FCV check): This feature flag doesn't have any upgrade/downgrade concerns.
-    auto pitFeatureEnabled =
-        feature_flags::gPointInTimeCatalogLookups.isEnabledAndIgnoreFCVUnsafe();
     while (_iterator != _endIterator) {
         IndexCatalogEntry* entry = _iterator->get();
         ++_iterator;
-
-        // When the PointInTimeCatalogLookups feature flag is not enabled, it is necessary to check
-        // whether the operation's read timestamp is before or after the most recent index
-        // modification (indicated by the minimum visible snapshot on the IndexCatalogEntry). If the
-        // read timestamp is before the most recent index modification, we must not include this
-        // entry in the iterator.
-        //
-        // When the PointInTimeCatalogLookups feature flag is enabled, the index catalog entry will
-        // be constructed from the durable catalog for reads with a read timestamp older than the
-        // minimum valid snapshot for the collection (which reflects the most recent catalog
-        // modification for that collection, including index modifications), so there's no need to
-        // check the minimum visible snapshot of the entry here.
-        if (!pitFeatureEnabled) {
-            if (auto minSnapshot = entry->getMinimumVisibleSnapshot()) {
-                auto mySnapshot =
-                    _opCtx->recoveryUnit()->getPointInTimeReadTimestamp(_opCtx).get_value_or(
-                        _opCtx->recoveryUnit()->getCatalogConflictingTimestamp());
-
-                if (!mySnapshot.isNull() && mySnapshot < minSnapshot.value()) {
-                    // This index isn't finished in my snapshot.
-                    continue;
-                }
-            }
-        }
-
         return entry;
     }
 
diff --git a/src/mongo/db/catalog/index_catalog_entry.h b/src/mongo/db/catalog/index_catalog_entry.h
index 71bad9953b3..bc357f97fc7 100644
--- a/src/mongo/db/catalog/index_catalog_entry.h
+++ b/src/mongo/db/catalog/index_catalog_entry.h
@@ -185,14 +185,6 @@ public:
      */
     virtual bool shouldValidateDocument() const = 0;
 
-    /**
-     * If return value is not boost::none, reads with majority read concern using an older snapshot
-     * must treat this index as unfinished.
-     */
-    virtual boost::optional<Timestamp> getMinimumVisibleSnapshot() const = 0;
-
-    virtual void setMinimumVisibleSnapshot(Timestamp name) = 0;
-
     virtual const UpdateIndexData& getIndexedPaths() const = 0;
 };
 
diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.cpp b/src/mongo/db/catalog/index_catalog_entry_impl.cpp
index 58c9a7b479d..f4e2b39be91 100644
--- a/src/mongo/db/catalog/index_catalog_entry_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_entry_impl.cpp
@@ -173,12 +173,6 @@ MultikeyPaths IndexCatalogEntryImpl::getMultikeyPaths(OperationContext* opCtx,
 
 // ---
 
-void IndexCatalogEntryImpl::setMinimumVisibleSnapshot(Timestamp newMinimumVisibleSnapshot) {
-    if (!_minVisibleSnapshot || (newMinimumVisibleSnapshot > _minVisibleSnapshot.value())) {
-        _minVisibleSnapshot = newMinimumVisibleSnapshot;
-    }
-}
-
 void IndexCatalogEntryImpl::setIsReady(bool newIsReady) {
     _isReady = newIsReady;
 }
diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.h b/src/mongo/db/catalog/index_catalog_entry_impl.h
index 00df93261dc..bc55e74a0c6 100644
--- a/src/mongo/db/catalog/index_catalog_entry_impl.h
+++ b/src/mongo/db/catalog/index_catalog_entry_impl.h
@@ -175,20 +175,6 @@ public:
 
     bool isReadyInMySnapshot(OperationContext* opCtx) const final;
 
-    /**
-     * If return value is not boost::none, reads with majority read concern using an older snapshot
-     * must treat this index as unfinished.
-     */
-    boost::optional<Timestamp> getMinimumVisibleSnapshot() const final {
-        return _minVisibleSnapshot;
-    }
-
-    /**
-     * Updates the minimum visible snapshot. The 'newMinimumVisibleSnapshot' is ignored if it would
-     * set the minimum visible snapshot backwards in time.
-     */
-    void setMinimumVisibleSnapshot(Timestamp newMinimumVisibleSnapshot) final;
-
     const UpdateIndexData& getIndexedPaths() const final {
         return _indexedPaths;
     }
@@ -242,9 +228,6 @@ private:
     bool _shouldValidateDocument;
     AtomicWord<bool> _isDropped;  // Whether the index drop is committed.
 
-    // The earliest snapshot that is allowed to read this index.
-    boost::optional<Timestamp> _minVisibleSnapshot;
-
     // Offset of this index within the Collection metadata.
     // Used to improve lookups without having to search for the index name
     // accessing the collection metadata.
diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp
index 4be9e44a494..78799813b28 100644
--- a/src/mongo/db/catalog/index_catalog_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_impl.cpp
@@ -297,15 +297,6 @@ void IndexCatalogImpl::init(OperationContext* opCtx,
             IndexCatalogEntry* entry =
                 createIndexEntry(opCtx, collection, std::move(descriptor), flags);
             fassert(17340, entry->isReady(opCtx));
-            // (Ignore FCV check): This feature flag doesn't have any upgrade/downgrade concerns.
-            if (!feature_flags::gPointInTimeCatalogLookups.isEnabledAndIgnoreFCVUnsafe() &&
-                recoveryTs && !entry->descriptor()->isIdIndex()) {
-                // When initializing indexes from disk, we conservatively set the
-                // minimumVisibleSnapshot to non _id indexes to the recovery timestamp. The _id
-                // index is left visible. It's assumed if the collection is visible, it's _id is
-                // valid to be used.
-                entry->setMinimumVisibleSnapshot(recoveryTs.value());
-            }
         }
     }
 
@@ -1685,13 +1676,6 @@ const IndexDescriptor* IndexCatalogImpl::refreshEntry(OperationContext* opCtx,
     // Last rebuild index data for CollectionQueryInfo for this Collection.
     CollectionQueryInfo::get(collection).rebuildIndexData(opCtx, CollectionPtr(collection));
 
-    opCtx->recoveryUnit()->onCommit(
-        [newEntry](OperationContext*, boost::optional<Timestamp> commitTime) {
-            if (commitTime) {
-                newEntry->setMinimumVisibleSnapshot(*commitTime);
-            }
-        });
-
     // Return the new descriptor.
     return newEntry->descriptor();
 }
diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp
index 6adebaa0b64..ef970794156 100644
--- a/src/mongo/db/index/index_access_method.cpp
+++ b/src/mongo/db/index/index_access_method.cpp
@@ -1231,20 +1231,6 @@ Status SortedDataIndexAccessMethod::_indexKeysOrWriteToSideTable(
             *keysInsertedOut += inserted;
         }
     } else {
-        // Ensure that our snapshot is compatible with the index's minimum visibile snapshot.
-        // (Ignore FCV check): This feature flag doesn't have any upgrade/downgrade concerns.
-        if (!feature_flags::gPointInTimeCatalogLookups.isEnabledAndIgnoreFCVUnsafe()) {
-            const auto minVisibleTimestamp = _indexCatalogEntry->getMinimumVisibleSnapshot();
-            const auto readTimestamp =
-                opCtx->recoveryUnit()->getPointInTimeReadTimestamp(opCtx).value_or(
-                    opCtx->recoveryUnit()->getCatalogConflictingTimestamp());
-            if (minVisibleTimestamp && !readTimestamp.isNull() &&
-                readTimestamp < *minVisibleTimestamp) {
-                throwWriteConflictException(
-                    "Unable to read from a snapshot due to pending catalog changes.");
-            }
-        }
-
         int64_t numInserted = 0;
         status = insertKeysAndUpdateMultikeyPaths(
             opCtx,
@@ -1301,20 +1287,6 @@ void SortedDataIndexAccessMethod::_unindexKeysOrWriteToSideTable(
     // We need to disable blind-deletes if 'checkRecordId' is explicitly set 'On'.
     options.dupsAllowed = options.dupsAllowed || checkRecordId == CheckRecordId::On;
 
-    // Ensure that our snapshot is compatible with the index's minimum visibile snapshot.
-    // (Ignore FCV check): This feature flag doesn't have any upgrade/downgrade concerns.
-    if (!feature_flags::gPointInTimeCatalogLookups.isEnabledAndIgnoreFCVUnsafe()) {
-        const auto minVisibleTimestamp = _indexCatalogEntry->getMinimumVisibleSnapshot();
-        const auto readTimestamp =
-            opCtx->recoveryUnit()->getPointInTimeReadTimestamp(opCtx).value_or(
-                opCtx->recoveryUnit()->getCatalogConflictingTimestamp());
-        if (minVisibleTimestamp && !readTimestamp.isNull() &&
-            readTimestamp < *minVisibleTimestamp) {
-            throwWriteConflictException(
-                "Unable to read from a snapshot due to pending catalog changes.");
-        }
-    }
-
     int64_t removed = 0;
     Status status = removeKeys(opCtx, keys, options, &removed);
 
diff --git a/src/mongo/db/query/collection_query_info_test.cpp b/src/mongo/db/query/collection_query_info_test.cpp
index 0bf1bef1093..341262e716e 100644
--- a/src/mongo/db/query/collection_query_info_test.cpp
+++ b/src/mongo/db/query/collection_query_info_test.cpp
@@ -150,14 +150,6 @@ public:
         MONGO_UNIMPLEMENTED;
     }
 
-    boost::optional<Timestamp> getMinimumVisibleSnapshot() const override {
-        MONGO_UNIMPLEMENTED;
-    }
-
-    void setMinimumVisibleSnapshot(Timestamp name) override {
-        MONGO_UNIMPLEMENTED;
-    }
-
     const UpdateIndexData& getIndexedPaths() const override {
         MONGO_UNIMPLEMENTED;
     }
diff --git a/src/mongo/db/query/plan_cache_key_factory.cpp b/src/mongo/db/query/plan_cache_key_factory.cpp
index c5d486cdc1c..c01b3de73b9 100644
--- a/src/mongo/db/query/plan_cache_key_factory.cpp
+++ b/src/mongo/db/query/plan_cache_key_factory.cpp
@@ -107,40 +107,6 @@ PlanCacheKeyInfo makePlanCacheKeyInfo(CanonicalQuery::QueryShapeString&& shapeSt
 }
 
 namespace {
-/**
- * Returns the highest index commit timestamp associated with an index on 'collection' that is
- * visible to this operation.
- */
-boost::optional<Timestamp> computeNewestVisibleIndexTimestamp(OperationContext* opCtx,
-                                                              const CollectionPtr& collection) {
-    auto recoveryUnit = opCtx->recoveryUnit();
-    auto mySnapshot = recoveryUnit->getPointInTimeReadTimestamp(opCtx).get_value_or(
-        recoveryUnit->getCatalogConflictingTimestamp());
-    if (mySnapshot.isNull()) {
-        return boost::none;
-    }
-
-    Timestamp currentNewestVisible = Timestamp::min();
-
-    auto ii = collection->getIndexCatalog()->getIndexIterator(
-        opCtx, IndexCatalog::InclusionPolicy::kReady);
-    while (ii->more()) {
-        const IndexCatalogEntry* ice = ii->next();
-        auto minVisibleSnapshot = ice->getMinimumVisibleSnapshot();
-        if (!minVisibleSnapshot) {
-            continue;
-        }
-
-        if (mySnapshot < *minVisibleSnapshot) {
-            continue;
-        }
-
-        currentNewestVisible = std::max(currentNewestVisible, *minVisibleSnapshot);
-    }
-
-    return currentNewestVisible.isNull() ? boost::optional<Timestamp>{} : currentNewestVisible;
-}
-
 sbe::PlanCacheKeyCollectionState computeCollectionState(OperationContext* opCtx,
                                                         const CollectionPtr& collection,
                                                         bool isSecondaryColl) {
@@ -159,7 +125,6 @@ sbe::PlanCacheKeyCollectionState computeCollectionState(OperationContext* opCtx,
     }
     return {collection->uuid(),
             CollectionQueryInfo::get(collection).getPlanCacheInvalidatorVersion(),
-            plan_cache_detail::computeNewestVisibleIndexTimestamp(opCtx, collection),
             keyShardingEpoch};
 }
 }  // namespace
diff --git a/src/mongo/db/query/sbe_plan_cache.h b/src/mongo/db/query/sbe_plan_cache.h
index 1fe468cd093..215d841a6f9 100644
--- a/src/mongo/db/query/sbe_plan_cache.h
+++ b/src/mongo/db/query/sbe_plan_cache.h
@@ -59,16 +59,12 @@ struct PlanCacheKeyShardingEpoch {
 struct PlanCacheKeyCollectionState {
     bool operator==(const PlanCacheKeyCollectionState& other) const {
         return other.uuid == uuid && other.version == version &&
-            other.newestVisibleIndexTimestamp == newestVisibleIndexTimestamp &&
             other.collectionGeneration == collectionGeneration;
     }
 
     size_t hashCode() const {
         size_t hash = UUID::Hash{}(uuid);
         boost::hash_combine(hash, version);
-        if (newestVisibleIndexTimestamp) {
-            boost::hash_combine(hash, newestVisibleIndexTimestamp->asULL());
-        }
         if (collectionGeneration) {
             collectionGeneration->epoch.hash_combine(hash);
             boost::hash_combine(hash, collectionGeneration->ts.asULL());
@@ -88,20 +84,6 @@ struct PlanCacheKeyCollectionState {
     // all readers seeing this version of the collection have drained.
     size_t version;
 
-    // The '_collectionVersion' is not currently sufficient in order to ensure that the indexes
-    // visible to the reader are consistent with the indexes present in the cache entry. The reason
-    // is that all readers see the latest copy-on-write version of the 'Collection' object, even
-    // though they are allowed to read at an older timestamp, potentially at a time before an index
-    // build completed.
-    //
-    // To solve this problem, we incorporate the timestamp of the newest index visible to the reader
-    // into the plan cache key. This ensures that the set of indexes visible to the reader match
-    // those present in the plan cache entry, preventing a situation where the plan cache entry
-    // reflects a newer version of the index catalog than the one visible to the reader.
-    //
-    // In the future, this could instead be solved with point-in-time catalog lookups.
-    boost::optional<Timestamp> newestVisibleIndexTimestamp;
-
     // Ensures that a cached SBE plan cannot be reused if the collection has since become sharded or
     // changed its shard key. The cached plan may no longer be valid after sharding or shard key
     // refining since the structure of the plan depends on whether the collection is sharded, and if