Revert "SERVER-70424: Provide way to create collection instance from untimestamped storage snapshot"

This reverts commit a29be1cf33df71c62ebbcc55ec1c1d870c46e297.

10 files changed, 36 insertions, 84 deletions

diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h
index 7c47c82e2ff..76905a8fe6f 100644
--- a/src/mongo/db/catalog/collection.h
+++ b/src/mongo/db/catalog/collection.h
@@ -230,7 +230,7 @@ public:
      */
     virtual Status initFromExisting(OperationContext* opCtx,
                                     const std::shared_ptr<Collection>& collection,
-                                    boost::optional<Timestamp> readTimestamp) = 0;
+                                    Timestamp readTimestamp) = 0;
 
     virtual bool isCommitted() const {
         return true;
diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index 5843292afd2..735a4b897b0 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -73,11 +73,8 @@ const OperationContext::Decoration<std::shared_ptr<const CollectionCatalog>> sta
 /**
  * Returns true if the collection is compatible with the read timestamp.
  */
-bool isExistingCollectionCompatible(std::shared_ptr<Collection> coll,
-                                    boost::optional<Timestamp> readTimestamp) {
-    // If no read timestamp is provided, no existing collection is compatible since we must
-    // instantiate a new collection instance.
-    if (!coll || !readTimestamp) {
+bool isCollectionCompatible(std::shared_ptr<Collection> coll, Timestamp readTimestamp) {
+    if (!coll) {
         return false;
     }
 
@@ -767,12 +764,18 @@ Status CollectionCatalog::reloadViews(OperationContext* opCtx, const DatabaseNam
 
 CollectionPtr CollectionCatalog::openCollection(OperationContext* opCtx,
                                                 const NamespaceString& nss,
-                                                boost::optional<Timestamp> readTimestamp) const {
+                                                Timestamp readTimestamp) const {
     if (!feature_flags::gPointInTimeCatalogLookups.isEnabledAndIgnoreFCV()) {
         return CollectionPtr();
     }
 
+    // Check if the storage transaction already has this collection instantiated.
     auto& uncommittedCatalogUpdates = UncommittedCatalogUpdates::get(opCtx);
+    auto lookupResult = uncommittedCatalogUpdates.lookupCollection(opCtx, nss);
+    if (lookupResult.found && !lookupResult.newColl) {
+        invariant(isCollectionCompatible(lookupResult.collection, readTimestamp));
+        return CollectionPtr(lookupResult.collection.get(), CollectionPtr::NoYieldTag{});
+    }
 
     // Try to find a catalog entry matching 'readTimestamp'.
     auto catalogEntry = _fetchPITCatalogEntry(opCtx, nss, readTimestamp);
@@ -785,7 +788,7 @@ CollectionPtr CollectionCatalog::openCollection(OperationContext* opCtx,
     auto latestCollection = _lookupCollectionByUUID(*catalogEntry->metadata->options.uuid);
 
     // Return the in-memory Collection instance if it is compatible with the read timestamp.
-    if (isExistingCollectionCompatible(latestCollection, readTimestamp)) {
+    if (isCollectionCompatible(latestCollection, readTimestamp)) {
         uncommittedCatalogUpdates.openCollection(opCtx, latestCollection);
         return CollectionPtr(latestCollection.get(), CollectionPtr::NoYieldTag{});
     }
@@ -810,9 +813,7 @@ CollectionPtr CollectionCatalog::openCollection(OperationContext* opCtx,
 }
 
 boost::optional<DurableCatalogEntry> CollectionCatalog::_fetchPITCatalogEntry(
-    OperationContext* opCtx,
-    const NamespaceString& nss,
-    boost::optional<Timestamp> readTimestamp) const {
+    OperationContext* opCtx, const NamespaceString& nss, const Timestamp& readTimestamp) const {
     auto [catalogId, result] = lookupCatalogIdByNSS(nss, readTimestamp);
     if (result == CatalogIdLookup::NamespaceExistence::kNotExists) {
         return boost::none;
@@ -836,7 +837,7 @@ boost::optional<DurableCatalogEntry> CollectionCatalog::_fetchPITCatalogEntry(
 std::shared_ptr<Collection> CollectionCatalog::_createCompatibleCollection(
     OperationContext* opCtx,
     const std::shared_ptr<Collection>& latestCollection,
-    boost::optional<Timestamp> readTimestamp,
+    const Timestamp& readTimestamp,
     const DurableCatalogEntry& catalogEntry) const {
     // Check if the collection is drop pending, not expired, and compatible with the read timestamp.
     std::shared_ptr<Collection> dropPendingColl = [&]() -> std::shared_ptr<Collection> {
@@ -847,7 +848,7 @@ std::shared_ptr<Collection> CollectionCatalog::_createCompatibleCollection(
         return dropPendingIt->second.lock();
     }();
 
-    if (isExistingCollectionCompatible(dropPendingColl, readTimestamp)) {
+    if (isCollectionCompatible(dropPendingColl, readTimestamp)) {
         return dropPendingColl;
     }
 
@@ -884,7 +885,7 @@ std::shared_ptr<Collection> CollectionCatalog::_createCompatibleCollection(
 
 std::shared_ptr<Collection> CollectionCatalog::_createNewPITCollection(
     OperationContext* opCtx,
-    boost::optional<Timestamp> readTimestamp,
+    const Timestamp& readTimestamp,
     const DurableCatalogEntry& catalogEntry) const {
     // The ident is expired, but it still may not have been dropped by the reaper. Try to mark it as
     // in use.
diff --git a/src/mongo/db/catalog/collection_catalog.h b/src/mongo/db/catalog/collection_catalog.h
index cf2f272c649..0b2561f0996 100644
--- a/src/mongo/db/catalog/collection_catalog.h
+++ b/src/mongo/db/catalog/collection_catalog.h
@@ -196,16 +196,15 @@ public:
     Status reloadViews(OperationContext* opCtx, const DatabaseName& dbName) const;
 
     /**
-     * Returns the collection pointer representative of 'nss' at the provided read timestamp. If no
-     * timestamp is provided, returns instance of the latest collection. The returned collection
-     * instance is only valid while the storage snapshot is open and becomes invalidated when the
-     * snapshot is closed.
+     * Returns the collection pointer representative of 'nss' at the provided read timestamp. The
+     * returned collection instance is only valid while the storage snapshot is open and becomes
+     * invalidated when the snapshot is closed.
      *
      * Returns nullptr when reading from a point-in-time where the collection did not exist.
      */
     CollectionPtr openCollection(OperationContext* opCtx,
                                  const NamespaceString& nss,
-                                 boost::optional<Timestamp> readTimestamp) const;
+                                 Timestamp readTimestamp) const;
 
     /**
      * Returns a shared_ptr to a drop pending index if it's found and not expired.
@@ -644,9 +643,7 @@ private:
      * Searches for a catalog entry at a point-in-time.
      */
     boost::optional<DurableCatalogEntry> _fetchPITCatalogEntry(
-        OperationContext* opCtx,
-        const NamespaceString& nss,
-        boost::optional<Timestamp> readTimestamp) const;
+        OperationContext* opCtx, const NamespaceString& nss, const Timestamp& readTimestamp) const;
 
     /**
      * Tries to create a Collection instance using existing shared collection state. Returns nullptr
@@ -655,7 +652,7 @@ private:
     std::shared_ptr<Collection> _createCompatibleCollection(
         OperationContext* opCtx,
         const std::shared_ptr<Collection>& latestCollection,
-        boost::optional<Timestamp> readTimestamp,
+        const Timestamp& readTimestamp,
         const DurableCatalogEntry& catalogEntry) const;
 
     /**
@@ -663,7 +660,7 @@ private:
      */
     std::shared_ptr<Collection> _createNewPITCollection(
         OperationContext* opCtx,
-        boost::optional<Timestamp> readTimestamp,
+        const Timestamp& readTimestamp,
         const DurableCatalogEntry& catalogEntry) const;
 
     /**
diff --git a/src/mongo/db/catalog/collection_catalog_test.cpp b/src/mongo/db/catalog/collection_catalog_test.cpp
index 70b96c652fe..8710d807f73 100644
--- a/src/mongo/db/catalog/collection_catalog_test.cpp
+++ b/src/mongo/db/catalog/collection_catalog_test.cpp
@@ -2388,47 +2388,5 @@ DEATH_TEST_F(CollectionCatalogTimestampTest, OpenCollectionInWriteUnitOfWork, "i
         CollectionCatalog::get(opCtx.get())->openCollection(opCtx.get(), nss, readTimestamp);
 }
 
-TEST_F(CollectionCatalogTimestampTest, OpenCollectionNoTimestamp) {
-    RAIIServerParameterControllerForTest featureFlagController(
-        "featureFlagPointInTimeCatalogLookups", true);
-
-    const NamespaceString nss("a.b");
-    const Timestamp createCollectionTs = Timestamp(10, 10);
-    const Timestamp createIndexTs = Timestamp(20, 20);
-    const Timestamp readTimestamp = Timestamp(30, 30);
-
-    createCollection(opCtx.get(), nss, createCollectionTs);
-
-    // Fetch a collection instance after creation but before creating an index.
-    auto preIndexColl =
-        CollectionCatalog::get(opCtx.get())->lookupCollectionByNamespace(opCtx.get(), nss);
-    ASSERT(preIndexColl);
-    ASSERT_EQ(0, preIndexColl->getIndexCatalog()->numIndexesTotal());
-
-    createIndex(opCtx.get(),
-                nss,
-                BSON("v" << 2 << "name"
-                         << "x_1"
-                         << "key" << BSON("x" << 1)),
-                createIndexTs);
-    OneOffRead oor(opCtx.get(), readTimestamp);
-    Lock::GlobalLock globalLock(opCtx.get(), MODE_IS);
-
-    // Open an instance of the latest collection by passing no timestamp.
-    auto coll = CollectionCatalog::get(opCtx.get())->openCollection(opCtx.get(), nss, boost::none);
-    ASSERT(coll);
-    ASSERT_EQ(1, coll->getIndexCatalog()->numIndexesTotal());
-
-    // Verify the CollectionCatalog returns the latest collection.
-    auto currentColl =
-        CollectionCatalog::get(opCtx.get())->lookupCollectionByNamespace(opCtx.get(), nss);
-    ASSERT(currentColl);
-    ASSERT_EQ(1, currentColl->getIndexCatalog()->numIndexesTotal());
-    ASSERT_EQ(coll, currentColl);
-    ASSERT_NE(coll, preIndexColl);
-
-    // Ensure the idents are shared between the up-to-date instances.
-    ASSERT_EQ(coll->getSharedIdent(), currentColl->getSharedIdent());
-}
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp
index 34aee68aa0b..c028252f3b1 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -229,14 +229,10 @@ bool doesMinMaxHaveMixedSchemaData(const BSONObj& min, const BSONObj& max) {
     return false;
 }
 
-bool isIndexCompatible(std::shared_ptr<IndexCatalogEntry> index,
-                       boost::optional<Timestamp> readTimestamp) {
+bool isIndexCompatible(std::shared_ptr<IndexCatalogEntry> index, Timestamp readTimestamp) {
     if (!index) {
         return false;
     }
-    if (!readTimestamp) {
-        return true;
-    }
 
     boost::optional<Timestamp> minVisibleSnapshot = index->getMinimumVisibleSnapshot();
     if (!minVisibleSnapshot) {
@@ -385,7 +381,7 @@ void CollectionImpl::init(OperationContext* opCtx) {
 
 Status CollectionImpl::initFromExisting(OperationContext* opCtx,
                                         const std::shared_ptr<Collection>& collection,
-                                        boost::optional<Timestamp> readTimestamp) {
+                                        Timestamp readTimestamp) {
     LOGV2_DEBUG(
         6825402, 1, "Initializing collection using shared state", logAttrs(collection->ns()));
 
diff --git a/src/mongo/db/catalog/collection_impl.h b/src/mongo/db/catalog/collection_impl.h
index 639ea80d702..46c2a3bb6be 100644
--- a/src/mongo/db/catalog/collection_impl.h
+++ b/src/mongo/db/catalog/collection_impl.h
@@ -82,7 +82,7 @@ public:
     void init(OperationContext* opCtx) final;
     Status initFromExisting(OperationContext* opCtx,
                             const std::shared_ptr<Collection>& collection,
-                            boost::optional<Timestamp> readTimestamp) final;
+                            Timestamp readTimestamp) final;
     bool isInitialized() const final;
     bool isCommitted() const final;
     void setCommitted(bool val) final;
diff --git a/src/mongo/db/catalog/collection_mock.h b/src/mongo/db/catalog/collection_mock.h
index 3ac2f7f029b..7ff9e7d00d8 100644
--- a/src/mongo/db/catalog/collection_mock.h
+++ b/src/mongo/db/catalog/collection_mock.h
@@ -68,7 +68,7 @@ public:
 
     Status initFromExisting(OperationContext* opCtx,
                             const std::shared_ptr<Collection>& collection,
-                            boost::optional<Timestamp> readTimestamp) {
+                            Timestamp readTimestamp) {
         MONGO_UNREACHABLE;
     }
 
diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp
index 5187795cc26..71d57817147 100644
--- a/src/mongo/db/catalog/index_catalog_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_impl.cpp
@@ -185,22 +185,20 @@ std::unique_ptr<IndexCatalog> IndexCatalogImpl::clone() const {
 }
 
 Status IndexCatalogImpl::init(OperationContext* opCtx, Collection* collection) {
-    return _init(
-        opCtx, collection, /*preexistingIndexes=*/{}, /*readTimestamp=*/boost::none, false);
+    return _init(opCtx, collection, /*preexistingIndexes=*/{}, /*readTimestamp=*/boost::none);
 };
 
 Status IndexCatalogImpl::initFromExisting(OperationContext* opCtx,
                                           Collection* collection,
                                           const IndexCatalogEntryContainer& preexistingIndexes,
                                           boost::optional<Timestamp> readTimestamp) {
-    return _init(opCtx, collection, preexistingIndexes, readTimestamp, true);
+    return _init(opCtx, collection, preexistingIndexes, readTimestamp);
 };
 
 Status IndexCatalogImpl::_init(OperationContext* opCtx,
                                Collection* collection,
                                const IndexCatalogEntryContainer& preexistingIndexes,
-                               boost::optional<Timestamp> readTimestamp,
-                               bool fromExisting) {
+                               boost::optional<Timestamp> readTimestamp) {
     vector<string> indexNames;
     collection->getAllIndexes(&indexNames);
     const bool replSetMemberInStandaloneMode =
@@ -217,7 +215,7 @@ Status IndexCatalogImpl::_init(OperationContext* opCtx,
         BSONObj spec = collection->getIndexSpec(indexName).getOwned();
         BSONObj keyPattern = spec.getObjectField("key");
 
-        if (fromExisting) {
+        if (readTimestamp) {
             auto preexistingIt = std::find_if(
                 preexistingIndexes.begin(),
                 preexistingIndexes.end(),
@@ -332,7 +330,7 @@ Status IndexCatalogImpl::_init(OperationContext* opCtx,
         }
     }
 
-    if (!fromExisting) {
+    if (!readTimestamp) {
         // Only do this when we're not initializing an earlier collection from the shared state of
         // an existing collection.
         CollectionQueryInfo::get(collection).init(opCtx, collection);
diff --git a/src/mongo/db/catalog/index_catalog_impl.h b/src/mongo/db/catalog/index_catalog_impl.h
index c527d2ec290..b67f636c8ae 100644
--- a/src/mongo/db/catalog/index_catalog_impl.h
+++ b/src/mongo/db/catalog/index_catalog_impl.h
@@ -313,12 +313,14 @@ private:
 
     /**
      * Helper for init() and initFromExisting().
+     *
+     * Passing boost::none for 'preexistingIndexes' indicates that the IndexCatalog is not being
+     * initialized at an earlier point-in-time.
      */
     Status _init(OperationContext* opCtx,
                  Collection* collection,
                  const IndexCatalogEntryContainer& preexistingIndexes,
-                 boost::optional<Timestamp> readTimestamp,
-                 bool fromExisting);
+                 boost::optional<Timestamp> readTimestamp);
 
     /**
      * In addition to IndexNames::findPluginName, validates that it is a known index type.
diff --git a/src/mongo/db/catalog/virtual_collection_impl.h b/src/mongo/db/catalog/virtual_collection_impl.h
index ebe3c95b469..499e6add895 100644
--- a/src/mongo/db/catalog/virtual_collection_impl.h
+++ b/src/mongo/db/catalog/virtual_collection_impl.h
@@ -68,7 +68,7 @@ public:
 
     Status initFromExisting(OperationContext* opCtx,
                             const std::shared_ptr<Collection>& collection,
-                            boost::optional<Timestamp> readTimestamp) final {
+                            Timestamp readTimestamp) final {
         unimplementedTasserted();
         return Status(ErrorCodes::UnknownError, "unknown");
     };