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

10 files changed, 84 insertions, 36 deletions

diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h
index 76905a8fe6f..7c47c82e2ff 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,
-                                    Timestamp readTimestamp) = 0;
+                                    boost::optional<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 735a4b897b0..5843292afd2 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -73,8 +73,11 @@ const OperationContext::Decoration<std::shared_ptr<const CollectionCatalog>> sta
 /**
  * Returns true if the collection is compatible with the read timestamp.
  */
-bool isCollectionCompatible(std::shared_ptr<Collection> coll, Timestamp readTimestamp) {
-    if (!coll) {
+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) {
         return false;
     }
 
@@ -764,18 +767,12 @@ Status CollectionCatalog::reloadViews(OperationContext* opCtx, const DatabaseNam
 
 CollectionPtr CollectionCatalog::openCollection(OperationContext* opCtx,
                                                 const NamespaceString& nss,
-                                                Timestamp readTimestamp) const {
+                                                boost::optional<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);
@@ -788,7 +785,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 (isCollectionCompatible(latestCollection, readTimestamp)) {
+    if (isExistingCollectionCompatible(latestCollection, readTimestamp)) {
         uncommittedCatalogUpdates.openCollection(opCtx, latestCollection);
         return CollectionPtr(latestCollection.get(), CollectionPtr::NoYieldTag{});
     }
@@ -813,7 +810,9 @@ CollectionPtr CollectionCatalog::openCollection(OperationContext* opCtx,
 }
 
 boost::optional<DurableCatalogEntry> CollectionCatalog::_fetchPITCatalogEntry(
-    OperationContext* opCtx, const NamespaceString& nss, const Timestamp& readTimestamp) const {
+    OperationContext* opCtx,
+    const NamespaceString& nss,
+    boost::optional<Timestamp> readTimestamp) const {
     auto [catalogId, result] = lookupCatalogIdByNSS(nss, readTimestamp);
     if (result == CatalogIdLookup::NamespaceExistence::kNotExists) {
         return boost::none;
@@ -837,7 +836,7 @@ boost::optional<DurableCatalogEntry> CollectionCatalog::_fetchPITCatalogEntry(
 std::shared_ptr<Collection> CollectionCatalog::_createCompatibleCollection(
     OperationContext* opCtx,
     const std::shared_ptr<Collection>& latestCollection,
-    const Timestamp& readTimestamp,
+    boost::optional<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> {
@@ -848,7 +847,7 @@ std::shared_ptr<Collection> CollectionCatalog::_createCompatibleCollection(
         return dropPendingIt->second.lock();
     }();
 
-    if (isCollectionCompatible(dropPendingColl, readTimestamp)) {
+    if (isExistingCollectionCompatible(dropPendingColl, readTimestamp)) {
         return dropPendingColl;
     }
 
@@ -885,7 +884,7 @@ std::shared_ptr<Collection> CollectionCatalog::_createCompatibleCollection(
 
 std::shared_ptr<Collection> CollectionCatalog::_createNewPITCollection(
     OperationContext* opCtx,
-    const Timestamp& readTimestamp,
+    boost::optional<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 0b2561f0996..cf2f272c649 100644
--- a/src/mongo/db/catalog/collection_catalog.h
+++ b/src/mongo/db/catalog/collection_catalog.h
@@ -196,15 +196,16 @@ public:
     Status reloadViews(OperationContext* opCtx, const DatabaseName& dbName) const;
 
     /**
-     * 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 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 nullptr when reading from a point-in-time where the collection did not exist.
      */
     CollectionPtr openCollection(OperationContext* opCtx,
                                  const NamespaceString& nss,
-                                 Timestamp readTimestamp) const;
+                                 boost::optional<Timestamp> readTimestamp) const;
 
     /**
      * Returns a shared_ptr to a drop pending index if it's found and not expired.
@@ -643,7 +644,9 @@ private:
      * Searches for a catalog entry at a point-in-time.
      */
     boost::optional<DurableCatalogEntry> _fetchPITCatalogEntry(
-        OperationContext* opCtx, const NamespaceString& nss, const Timestamp& readTimestamp) const;
+        OperationContext* opCtx,
+        const NamespaceString& nss,
+        boost::optional<Timestamp> readTimestamp) const;
 
     /**
      * Tries to create a Collection instance using existing shared collection state. Returns nullptr
@@ -652,7 +655,7 @@ private:
     std::shared_ptr<Collection> _createCompatibleCollection(
         OperationContext* opCtx,
         const std::shared_ptr<Collection>& latestCollection,
-        const Timestamp& readTimestamp,
+        boost::optional<Timestamp> readTimestamp,
         const DurableCatalogEntry& catalogEntry) const;
 
     /**
@@ -660,7 +663,7 @@ private:
      */
     std::shared_ptr<Collection> _createNewPITCollection(
         OperationContext* opCtx,
-        const Timestamp& readTimestamp,
+        boost::optional<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 8710d807f73..70b96c652fe 100644
--- a/src/mongo/db/catalog/collection_catalog_test.cpp
+++ b/src/mongo/db/catalog/collection_catalog_test.cpp
@@ -2388,5 +2388,47 @@ 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 c028252f3b1..34aee68aa0b 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -229,10 +229,14 @@ bool doesMinMaxHaveMixedSchemaData(const BSONObj& min, const BSONObj& max) {
     return false;
 }
 
-bool isIndexCompatible(std::shared_ptr<IndexCatalogEntry> index, Timestamp readTimestamp) {
+bool isIndexCompatible(std::shared_ptr<IndexCatalogEntry> index,
+                       boost::optional<Timestamp> readTimestamp) {
     if (!index) {
         return false;
     }
+    if (!readTimestamp) {
+        return true;
+    }
 
     boost::optional<Timestamp> minVisibleSnapshot = index->getMinimumVisibleSnapshot();
     if (!minVisibleSnapshot) {
@@ -381,7 +385,7 @@ void CollectionImpl::init(OperationContext* opCtx) {
 
 Status CollectionImpl::initFromExisting(OperationContext* opCtx,
                                         const std::shared_ptr<Collection>& collection,
-                                        Timestamp readTimestamp) {
+                                        boost::optional<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 46c2a3bb6be..639ea80d702 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,
-                            Timestamp readTimestamp) final;
+                            boost::optional<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 7ff9e7d00d8..3ac2f7f029b 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,
-                            Timestamp readTimestamp) {
+                            boost::optional<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 71d57817147..5187795cc26 100644
--- a/src/mongo/db/catalog/index_catalog_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_impl.cpp
@@ -185,20 +185,22 @@ std::unique_ptr<IndexCatalog> IndexCatalogImpl::clone() const {
 }
 
 Status IndexCatalogImpl::init(OperationContext* opCtx, Collection* collection) {
-    return _init(opCtx, collection, /*preexistingIndexes=*/{}, /*readTimestamp=*/boost::none);
+    return _init(
+        opCtx, collection, /*preexistingIndexes=*/{}, /*readTimestamp=*/boost::none, false);
 };
 
 Status IndexCatalogImpl::initFromExisting(OperationContext* opCtx,
                                           Collection* collection,
                                           const IndexCatalogEntryContainer& preexistingIndexes,
                                           boost::optional<Timestamp> readTimestamp) {
-    return _init(opCtx, collection, preexistingIndexes, readTimestamp);
+    return _init(opCtx, collection, preexistingIndexes, readTimestamp, true);
 };
 
 Status IndexCatalogImpl::_init(OperationContext* opCtx,
                                Collection* collection,
                                const IndexCatalogEntryContainer& preexistingIndexes,
-                               boost::optional<Timestamp> readTimestamp) {
+                               boost::optional<Timestamp> readTimestamp,
+                               bool fromExisting) {
     vector<string> indexNames;
     collection->getAllIndexes(&indexNames);
     const bool replSetMemberInStandaloneMode =
@@ -215,7 +217,7 @@ Status IndexCatalogImpl::_init(OperationContext* opCtx,
         BSONObj spec = collection->getIndexSpec(indexName).getOwned();
         BSONObj keyPattern = spec.getObjectField("key");
 
-        if (readTimestamp) {
+        if (fromExisting) {
             auto preexistingIt = std::find_if(
                 preexistingIndexes.begin(),
                 preexistingIndexes.end(),
@@ -330,7 +332,7 @@ Status IndexCatalogImpl::_init(OperationContext* opCtx,
         }
     }
 
-    if (!readTimestamp) {
+    if (!fromExisting) {
         // 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 b67f636c8ae..c527d2ec290 100644
--- a/src/mongo/db/catalog/index_catalog_impl.h
+++ b/src/mongo/db/catalog/index_catalog_impl.h
@@ -313,14 +313,12 @@ 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);
+                 boost::optional<Timestamp> readTimestamp,
+                 bool fromExisting);
 
     /**
      * 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 499e6add895..ebe3c95b469 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,
-                            Timestamp readTimestamp) final {
+                            boost::optional<Timestamp> readTimestamp) final {
         unimplementedTasserted();
         return Status(ErrorCodes::UnknownError, "unknown");
     };