diff options
author | Will Buerger <will.buerger@mongodb.com> | 2022-11-03 13:29:32 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-11-03 14:03:07 +0000 |
commit | c73e2cf8985997ceabbe7882597ce655a11c4487 (patch) | |
tree | 4023a3ada4eea85b91864d3817f64b2ebb8434e1 | |
parent | e6f5ea4856da2da7cc0fe92c39c102e1dc92c9c0 (diff) | |
download | mongo-c73e2cf8985997ceabbe7882597ce655a11c4487.tar.gz |
Revert "SERVER-70424: Provide way to create collection instance from untimestamped storage snapshot"
This reverts commit a29be1cf33df71c62ebbcc55ec1c1d870c46e297.
-rw-r--r-- | src/mongo/db/catalog/collection.h | 2 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_catalog.cpp | 27 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_catalog.h | 17 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_catalog_test.cpp | 42 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_impl.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_impl.h | 2 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_mock.h | 2 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_catalog_impl.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_catalog_impl.h | 6 | ||||
-rw-r--r-- | src/mongo/db/catalog/virtual_collection_impl.h | 2 |
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"); }; |