diff options
| -rw-r--r-- | src/mongo/db/catalog/collection.h | 8 | ||||
| -rw-r--r-- | src/mongo/db/catalog_raii.cpp | 24 | ||||
| -rw-r--r-- | src/mongo/db/catalog_raii.h | 36 | ||||
| -rw-r--r-- | src/mongo/db/db_raii.cpp | 406 | ||||
| -rw-r--r-- | src/mongo/db/db_raii.h | 167 | ||||
| -rw-r--r-- | src/mongo/db/db_raii_multi_collection_test.cpp | 232 | ||||
| -rw-r--r-- | src/mongo/db/storage/snapshot_helper.cpp | 7 | ||||
| -rw-r--r-- | src/mongo/db/storage/snapshot_helper.h | 5 |
8 files changed, 500 insertions, 385 deletions
diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h index 5ac6f274f3e..6fd99c61da8 100644 --- a/src/mongo/db/catalog/collection.h +++ b/src/mongo/db/catalog/collection.h @@ -876,14 +876,6 @@ public: void yield() const override; void restore() const override; - RestoreFn detachRestoreFn() { - return std::move(_restoreFn); - } - - void attachRestoreFn(RestoreFn newRestoreFn) { - _restoreFn = std::move(newRestoreFn); - } - friend std::ostream& operator<<(std::ostream& os, const CollectionPtr& coll); void setShardKeyPattern(const BSONObj& shardKeyPattern) { diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp index 8645ea5fc40..13635b0a16c 100644 --- a/src/mongo/db/catalog_raii.cpp +++ b/src/mongo/db/catalog_raii.cpp @@ -47,15 +47,6 @@ namespace { MONGO_FAIL_POINT_DEFINE(setAutoGetCollectionWait); /** - * Returns true if 'nss' is a view. False if the namespace or view doesn't exist. - */ -bool isSecondaryNssAView(OperationContext* opCtx, const NamespaceString& nss) { - TenantDatabaseName tenantDbName(boost::none, nss.db()); - auto viewCatalog = DatabaseHolder::get(opCtx)->getViewCatalog(opCtx, tenantDbName); - return viewCatalog && viewCatalog->lookup(opCtx, nss); -} - -/** * Performs some sanity checks on the collection and database. */ void verifyDbAndCollection(OperationContext* opCtx, @@ -229,7 +220,7 @@ AutoGetCollection::AutoGetCollection( invariant(!opCtx->isLockFreeReadsOp()); invariant(secondaryNssOrUUIDs.empty() || modeColl == MODE_IS); - // Get a unique list of 'secondary' database names to pass into AutoGetDbMulti below. + // Get a unique list of 'secondary' database names to pass into AutoGetDb below. std::set<StringData> secondaryDbNames; for (auto& secondaryNssOrUUID : secondaryNssOrUUIDs) { secondaryDbNames.emplace(secondaryNssOrUUID.db()); @@ -287,11 +278,6 @@ AutoGetCollection::AutoGetCollection( secondaryResolvedNss, secondaryColl, databaseHolder->getDb(opCtx, secondaryTenantDbName)); - - // Flag if a secondary namespace is a view. - if (!_secondaryNssIsView && isSecondaryNssAView(opCtx, secondaryResolvedNss)) { - _secondaryNssIsView = true; - } } if (_coll) { @@ -301,8 +287,8 @@ AutoGetCollection::AutoGetCollection( // table are consistent with the read request's shardVersion. // // Note: sharding versioning for an operation has no concept of multiple collections. - auto collDesc = - CollectionShardingState::get(opCtx, _resolvedNss)->getCollectionDescription(opCtx); + auto css = CollectionShardingState::getSharedForLockFreeReads(opCtx, _resolvedNss); + auto collDesc = css->getCollectionDescription(opCtx); if (collDesc.isSharded()) { _coll.setShardKeyPattern(collDesc.getKeyPattern()); } @@ -396,8 +382,8 @@ AutoGetCollectionLockFree::AutoGetCollectionLockFree(OperationContext* opCtx, // operation. The shardVersion will be checked later if the shard filtering metadata is // fetched, ensuring both that the collection description info fetched here and the routing // table are consistent with the read request's shardVersion. - auto collDesc = CollectionShardingState::getSharedForLockFreeReads(opCtx, _collection->ns()) - ->getCollectionDescription(opCtx); + auto css = CollectionShardingState::getSharedForLockFreeReads(opCtx, _collection->ns()); + auto collDesc = css->getCollectionDescription(opCtx); if (collDesc.isSharded()) { _collectionPtr.setShardKeyPattern(collDesc.getKeyPattern()); } diff --git a/src/mongo/db/catalog_raii.h b/src/mongo/db/catalog_raii.h index b1740758d05..c576df1cde4 100644 --- a/src/mongo/db/catalog_raii.h +++ b/src/mongo/db/catalog_raii.h @@ -125,9 +125,9 @@ public: * avoid deadlocks, consistent with other locations in the code wherein we take multiple * collection locks. * - * Invariants if any 'secondaryNssOrUUIDs' represent a view namespace. Only MODE_IS is supported - * when 'secondaryNssOrUUIDs' namespaces are provided. It is safe for 'nsOrUUID' to be - * duplicated in 'secondaryNssOrUUIDs', or 'secondaryNssOrUUIDs' to contain duplicates. + * Only MODE_IS is supported when 'secondaryNssOrUUIDs' namespaces are provided. It is safe for + * 'nsOrUUID' to be duplicated in 'secondaryNssOrUUIDs', or 'secondaryNssOrUUIDs' to contain + * duplicates. */ AutoGetCollection( OperationContext* opCtx, @@ -190,13 +190,6 @@ public: } /** - * Indicates whether any of the 'secondaryNssOrUUIDs' namespaces are views. - */ - bool isAnySecondaryNamespaceAView() const { - return _secondaryNssIsView; - } - - /** * Returns a writable Collection copy that will be returned by current and future calls to this * function as well as getCollection(). Any previous Collection pointers that were returned may * be invalidated. @@ -211,16 +204,6 @@ public: CollectionCatalog::LifetimeMode::kManagedInWriteUnitOfWork); protected: - template <typename AutoGetCollectionType, typename EmplaceAutoGetCollectionFunc> - friend class AutoGetCollectionForReadBase; - - /** - * Allow access to the CollectionPtr as non-const, for friend classes. - */ - CollectionPtr& _getCollectionPtrForModify() { - return _coll; - } - // Ordering matters, the _collLocks should destruct before the _autoGetDb releases the // rstl/global/database locks. boost::optional<AutoGetDb> _autoDb; @@ -229,9 +212,6 @@ protected: CollectionPtr _coll = nullptr; std::shared_ptr<const ViewDefinition> _view; - // Tracks whether any secondary collection namespaces is a view. - bool _secondaryNssIsView = false; - // If the object was instantiated with a UUID, contains the resolved namespace, otherwise it is // the same as the input namespace string NamespaceString _resolvedNss; @@ -316,16 +296,6 @@ public: } private: - template <typename AutoGetCollectionType, typename EmplaceAutoGetCollectionFunc> - friend class AutoGetCollectionForReadBase; - - /** - * Allow access to the CollectionPtr as non-const, for friend classes. - */ - CollectionPtr& _getCollectionPtrForModify() { - return _collectionPtr; - } - // Indicate that we are lock-free on code paths that can run either lock-free or locked for // different kinds of operations. Note: this class member is currently declared first so that it // destructs last, as a safety measure, but not because it is currently depended upon behavior. diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp index 0fe457bd152..b7d4c5b199f 100644 --- a/src/mongo/db/db_raii.cpp +++ b/src/mongo/db/db_raii.cpp @@ -40,6 +40,7 @@ #include "mongo/db/s/collection_sharding_state.h" #include "mongo/db/s/database_sharding_state.h" #include "mongo/db/storage/snapshot_helper.h" +#include "mongo/db/views/view_catalog.h" #include "mongo/logv2/log.h" namespace mongo { @@ -89,6 +90,189 @@ private: }; /** + * Checks that the minimum visible timestamp of 'collection' is compatible with 'readTimestamp'. + * Does nothing if collection does not exist. + * + * Returns OK or SnapshotUnavailable. + */ +Status checkSecondaryCollection(OperationContext* opCtx, + const CollectionPtr& collection, + boost::optional<Timestamp> readTimestamp) { + // Check that the collection exists. + if (!collection) { + return Status::OK(); + } + + // Ensure the readTimestamp is not older than the collection's minimum visible timestamp. + auto minSnapshot = collection->getMinimumVisibleSnapshot(); + if (SnapshotHelper::collectionChangesConflictWithRead(minSnapshot, readTimestamp)) { + // Note: SnapshotHelper::collectionChangesConflictWithRead returns false if either + // minSnapshot or readTimestamp is not set, so it's safe to print them below. + return Status(ErrorCodes::SnapshotUnavailable, + str::stream() + << "Unable to read from a snapshot due to pending collection catalog " + "changes to collection '" + << collection->ns() + << "'; please retry the operation. Snapshot timestamp is " + << readTimestamp->toString() << ". Collection minimum timestamp is " + << minSnapshot->toString()); + } + + return Status::OK(); +} + +/** + * Returns true if 'nss' is a view. False if the view doesn't exist. + */ +bool isSecondaryNssAView(OperationContext* opCtx, const NamespaceString& nss) { + return ViewCatalog::get(opCtx)->lookup(opCtx, nss).get(); +} + +/** + * Returns true if 'nss' is sharded. False otherwise. + */ +bool isSecondaryNssSharded(OperationContext* opCtx, const NamespaceString& nss) { + return CollectionShardingState::getSharedForLockFreeReads(opCtx, nss) + ->getCollectionDescription(opCtx) + .isSharded(); +} + +/** + * Takes a vector of secondary nssOrUUIDs and checks that they are consistently safe to use before + * and after some external operation. Checks the namespaces on construction and then + * isSecondaryStateStillConsistent() can be called to re-check that the namespaces have not changed. + */ +class SecondaryNamespaceStateChecker { +public: + /** + * Uasserts if any namespace has a minimum visible snapshot later than the operation's read + * timestamp. + * + * Resolves the provided NamespaceStringOrUUIDs to NamespaceStrings and stores them, as well as + * whether or not any namespace is a view or sharded, to compare against later in + * isSecondaryStateStillConsistent(). + * + * 'consistencyCheckBypass' can be used to bypass the before and after aspect and instead make a + * single check on construction. The checks will be performed on construction only. + * + * It is safe for secondaryNssOrUUIDs to contain duplicates: namespaces will simply be + * redundantly and benignly re-checked. + */ + SecondaryNamespaceStateChecker(OperationContext* opCtx, + const CollectionCatalog* catalog, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs, + bool consistencyCheckBypass = false) + : _consistencyCheckBypass(consistencyCheckBypass) { + const auto readTimestamp = opCtx->recoveryUnit()->getPointInTimeReadTimestamp(opCtx); + for (const auto& secondaryNssOrUUID : secondaryNssOrUUIDs) { + auto secondaryNss = catalog->resolveNamespaceStringOrUUID(opCtx, secondaryNssOrUUID); + auto collection = catalog->lookupCollectionByNamespace(opCtx, secondaryNss); + + // Check that the secondary collection is safe to use. + uassertStatusOK(checkSecondaryCollection(opCtx, collection, readTimestamp)); + + if (!_consistencyCheckBypass) { + _namespaces.emplace_back(secondaryNssOrUUID, + secondaryNss, + collection ? false + : isSecondaryNssAView(opCtx, secondaryNss), + isSecondaryNssSharded(opCtx, secondaryNss)); + } else { + // isSecondaryStateStillConsistent won't be called, so initialize + // _haveAShardedOrViewSecondaryNss on construction instead. No need to create the + // internal maps, either. + bool view = collection ? false : isSecondaryNssAView(opCtx, secondaryNss); + bool sharded = view ? false : isSecondaryNssSharded(opCtx, secondaryNss); + if ((view || sharded) && !_haveAShardedOrViewSecondaryNss) { + _haveAShardedOrViewSecondaryNss = true; + } + } + } + } + + /** + * Uasserts if any namespace does not exist or has a minimum visible snapshot later than the + * operation's read timestamp. Note: it is possible for the read timestamp to have changed since + * construction. + * + * Returns false if the originally provided 'secondaryNssOrUUIDs' now resolve to different + * NamespaceStrings or are found to now be a view or sharded when they previously where not. + * + * Cannot be called if 'consistencyCheckBypass' was set to true on object construction. + */ + bool isSecondaryStateStillConsistent(OperationContext* opCtx, + const CollectionCatalog* catalog) { + invariant(!_consistencyCheckBypass); + + const auto readTimestamp = opCtx->recoveryUnit()->getPointInTimeReadTimestamp(opCtx); + for (const auto& namespaceIt : _namespaces) { + auto secondaryNss = catalog->resolveNamespaceStringOrUUID(opCtx, namespaceIt.nssOrUUID); + if (secondaryNss != namespaceIt.nss) { + // A secondary collection UUID maps to a different namespace. + return false; + } + + auto collection = catalog->lookupCollectionByNamespace(opCtx, secondaryNss); + uassertStatusOK(checkSecondaryCollection(opCtx, collection, readTimestamp)); + + bool isView = collection ? false : isSecondaryNssAView(opCtx, secondaryNss); + if (isView != namespaceIt.isView || + isSecondaryNssSharded(opCtx, secondaryNss) != namespaceIt.isSharded) { + // A secondary namespace changed to/from sharded or to/from a view. + return false; + } + + if (!_haveAShardedOrViewSecondaryNss && (namespaceIt.isView || namespaceIt.isSharded)) { + _haveAShardedOrViewSecondaryNss = true; + } + } + + _consistencyCheck = true; + return true; + } + + /** + * Returns whether or not any of the secondary namespaces are views or sharded. Can only be + * called after isSecondaryStateStillConsistent() has been called and returned true OR + * 'consistencyCheckBypass' was set to true on construction. + */ + bool isAnySecondaryNamespaceAViewOrSharded() { + invariant(_consistencyCheck || _consistencyCheckBypass); + return _haveAShardedOrViewSecondaryNss; + } + +private: + /** + * Saves a view of a NamespaceStringOrUUID: the resolved NamespaceString, and whether the + * namespace is a view or sharded. + */ + struct Namespace { + Namespace(const NamespaceStringOrUUID& pNssOrUUID, + const NamespaceString& pNss, + bool pIsView, + bool pIsSharded) + : nssOrUUID(pNssOrUUID), nss(pNss), isView(pIsView), isSharded(pIsSharded) {} + + NamespaceStringOrUUID nssOrUUID; + NamespaceString nss; + bool isView; + bool isSharded; + }; + + // Ensures that UUID->Nss, Nss->isSharded and Nss->isView do not change. Duplicate namespaces + // are OK, the namespace will just be checked twice. It is possible that a duplicate UUID can + // match to two different namespaces and pass this class' checks (suppose a lot of concurrent + // renames), but that is also OK because external checks will catch catalog changes. + std::vector<Namespace> _namespaces; + + bool _haveAShardedOrViewSecondaryNss = false; + // Guards access to _haveAShardedOrViewSecondaryNss. + bool _consistencyCheck = false; + // Bypasses the _consistencyCheck guard. + bool _consistencyCheckBypass = false; +}; + +/** * Helper function to acquire a collection and consistent snapshot without holding the RSTL or * collection locks. * @@ -100,17 +284,22 @@ private: * GetCollectionAndEstablishReadSourceFunc. * * ResetFunc is called when we failed to achieve consistency and need to retry. + * + * SetSecondaryState sets any of the secondary state that the AutoGet* needs to know about. */ template <typename GetCollectionAndEstablishReadSourceFunc, typename GetCollectionAfterSnapshotFunc, - typename ResetFunc> + typename ResetFunc, + typename SetSecondaryState> auto acquireCollectionAndConsistentSnapshot( OperationContext* opCtx, bool isLockFreeReadSubOperation, CollectionCatalogStasher& catalogStasher, GetCollectionAndEstablishReadSourceFunc getCollectionAndEstablishReadSource, GetCollectionAfterSnapshotFunc getCollectionAfterSnapshot, - ResetFunc reset) { + ResetFunc reset, + SetSecondaryState setSecondaryState, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}) { // Figure out what type of Collection GetCollectionAndEstablishReadSourceFunc returns. It needs // to behave like a pointer. using CollectionPtrT = decltype(std::declval<GetCollectionAndEstablishReadSourceFunc>()( @@ -126,9 +315,13 @@ auto acquireCollectionAndConsistentSnapshot( long long replTerm = repl::ReplicationCoordinator::get(opCtx)->getTerm(); auto catalog = CollectionCatalog::get(opCtx); + collection = getCollectionAndEstablishReadSource(opCtx, *catalog, isLockFreeReadSubOperation); + SecondaryNamespaceStateChecker secondaryNssStateChecker( + opCtx, catalog.get(), secondaryNssOrUUIDs); + // A lock request does not always find a collection to lock. if (!collection) break; @@ -166,10 +359,12 @@ auto acquireCollectionAndConsistentSnapshot( auto newCatalog = CollectionCatalog::get(opCtx); if (catalog == newCatalog) { auto newCollection = getCollectionAfterSnapshot(opCtx, *catalog); - if (newCollection && catalog == newCatalog && + if (newCollection && collection->getMinimumVisibleSnapshot() == newCollection->getMinimumVisibleSnapshot() && - replTerm == repl::ReplicationCoordinator::get(opCtx)->getTerm()) { + replTerm == repl::ReplicationCoordinator::get(opCtx)->getTerm() && + secondaryNssStateChecker.isSecondaryStateStillConsistent(opCtx, newCatalog.get())) { + setSecondaryState(secondaryNssStateChecker.isAnySecondaryNamespaceAViewOrSharded()); catalogStasher.stash(std::move(catalog)); break; } @@ -186,51 +381,6 @@ auto acquireCollectionAndConsistentSnapshot( return collection; } -/** - * Checks that the 'collection' is not null, that the 'collection' is not sharded and that the - * minimum visible timestamp of 'collection' is compatible with 'readTimestamp', if 'readTimestamp; - * is set. - * - * Returns OK, or either SnapshotUnavailable or NamespaceNotFound. - * Invariants that the collection is not sharded. - */ -Status checkSecondaryCollection(OperationContext* opCtx, - boost::optional<NamespaceString> nss, - const boost::optional<UUID>& uuid, - const std::shared_ptr<const Collection>& collection, - boost::optional<Timestamp> readTimestamp) { - invariant(nss || uuid); - - // Check that the collection exists. - if (!collection) { - return Status(ErrorCodes::NamespaceNotFound, - str::stream() << "Could not find collection '" - << (nss ? nss->toString() : uuid->toString()) << "'"); - } - - // Secondary collections of a query are not allowed to be sharded. - auto collDesc = CollectionShardingState::getSharedForLockFreeReads(opCtx, collection->ns()) - ->getCollectionDescription(opCtx); - invariant(!collDesc.isSharded()); - - // Ensure the readTimestamp is not older than the collection's minimum visible timestamp. - auto minSnapshot = collection->getMinimumVisibleSnapshot(); - if (SnapshotHelper::collectionChangesConflictWithRead(minSnapshot, readTimestamp)) { - // Note: SnapshotHelper::collectionChangesConflictWithRead returns false if either - // minSnapshot or readTimestamp is not set, so it's safe to print them below. - return Status(ErrorCodes::SnapshotUnavailable, - str::stream() - << "Unable to read from a snapshot due to pending collection catalog " - "changes to collection '" - << collection->ns() - << "'; please retry the operation. Snapshot timestamp is " - << readTimestamp->toString() << ". Collection minimum timestamp is " - << minSnapshot->toString()); - } - - return Status::OK(); -} - } // namespace AutoStatsTracker::AutoStatsTracker(OperationContext* opCtx, @@ -462,22 +612,41 @@ EmplaceAutoGetCollectionForRead::EmplaceAutoGetCollectionForRead( OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode, - Date_t deadline) - : _opCtx(opCtx), _nsOrUUID(nsOrUUID), _viewMode(viewMode), _deadline(deadline) { + Date_t deadline, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs) + : _opCtx(opCtx), + _nsOrUUID(nsOrUUID), + _viewMode(viewMode), + _deadline(deadline), + _secondaryNssOrUUIDs(secondaryNssOrUUIDs) { // Multi-document transactions need MODE_IX locks, otherwise MODE_IS. _collectionLockMode = getLockModeForQuery(opCtx, nsOrUUID.nss()); } void EmplaceAutoGetCollectionForRead::emplace(boost::optional<AutoGetCollection>& autoColl) const { - autoColl.emplace(_opCtx, _nsOrUUID, _collectionLockMode, _viewMode, _deadline); + autoColl.emplace( + _opCtx, _nsOrUUID, _collectionLockMode, _viewMode, _deadline, _secondaryNssOrUUIDs); } -AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx, - const NamespaceStringOrUUID& nsOrUUID, - AutoGetCollectionViewMode viewMode, - Date_t deadline) - : AutoGetCollectionForReadBase( - opCtx, EmplaceAutoGetCollectionForRead(opCtx, nsOrUUID, viewMode, deadline)) {} +AutoGetCollectionForRead::AutoGetCollectionForRead( + OperationContext* opCtx, + const NamespaceStringOrUUID& nsOrUUID, + AutoGetCollectionViewMode viewMode, + Date_t deadline, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs) + : AutoGetCollectionForReadBase(opCtx, + EmplaceAutoGetCollectionForRead( + opCtx, nsOrUUID, viewMode, deadline, secondaryNssOrUUIDs)) { + // All relevant locks are held. Check secondary collections and verify they are valid for + // use. + if (getCollection() && !secondaryNssOrUUIDs.empty()) { + auto catalog = CollectionCatalog::get(opCtx); + SecondaryNamespaceStateChecker secondaryNamespaceStateChecker( + opCtx, catalog.get(), secondaryNssOrUUIDs, true /* consistencyCheckBypass */); + _secondaryNssIsAViewOrSharded = + secondaryNamespaceStateChecker.isAnySecondaryNamespaceAViewOrSharded(); + } +} AutoGetCollectionForReadLockFree::EmplaceHelper::EmplaceHelper( OperationContext* opCtx, @@ -541,7 +710,15 @@ void AutoGetCollectionForReadLockFree::EmplaceHelper::emplace( return catalog.lookupCollectionByUUIDForRead(opCtx, uuid); }, /* ResetFunc */ - []() {}); + []() {}, + /* SetSecondaryState */ + [](bool isAnySecondaryNamespaceAViewOrSharded) { + // Not necessary to check for views or sharded secondary collections, which are + // unsupported. If a read is running, changing a namespace to a view would + // require dropping the collection first, which trips other checks. A secondary + // collection becoming sharded during a read is ignored to parallel existing + // behavior for the primary collection. + }); }, _viewMode, _deadline); @@ -551,7 +728,8 @@ AutoGetCollectionForReadLockFree::AutoGetCollectionForReadLockFree( OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode, - Date_t deadline) + Date_t deadline, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs) : _catalogStash(opCtx) { bool isLockFreeReadSubOperation = opCtx->isLockFreeReadsOp(); @@ -581,18 +759,24 @@ AutoGetCollectionForReadLockFree::AutoGetCollectionForReadLockFree( opCtx, _autoGetCollectionForReadBase.get()->uuid()); }, /* ResetFunc */ - [this]() { _autoGetCollectionForReadBase.reset(); }); + [this]() { _autoGetCollectionForReadBase.reset(); }, + /* SetSecondaryState */ + [this](bool isAnySecondaryNamespaceAViewOrSharded) { + _secondaryNssIsAViewOrSharded = isAnySecondaryNamespaceAViewOrSharded; + }, + secondaryNssOrUUIDs); } AutoGetCollectionForReadMaybeLockFree::AutoGetCollectionForReadMaybeLockFree( OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode, - Date_t deadline) { + Date_t deadline, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs) { if (supportsLockFreeRead(opCtx)) { - _autoGetLockFree.emplace(opCtx, nsOrUUID, viewMode, deadline); + _autoGetLockFree.emplace(opCtx, nsOrUUID, viewMode, deadline, secondaryNssOrUUIDs); } else { - _autoGet.emplace(opCtx, nsOrUUID, viewMode, deadline); + _autoGet.emplace(opCtx, nsOrUUID, viewMode, deadline, secondaryNssOrUUIDs); } } @@ -622,12 +806,14 @@ const CollectionPtr& AutoGetCollectionForReadMaybeLockFree::getCollection() cons template <typename AutoGetCollectionForReadType> AutoGetCollectionForReadCommandBase<AutoGetCollectionForReadType>:: - AutoGetCollectionForReadCommandBase(OperationContext* opCtx, - const NamespaceStringOrUUID& nsOrUUID, - AutoGetCollectionViewMode viewMode, - Date_t deadline, - AutoStatsTracker::LogMode logMode) - : _autoCollForRead(opCtx, nsOrUUID, viewMode, deadline), + AutoGetCollectionForReadCommandBase( + OperationContext* opCtx, + const NamespaceStringOrUUID& nsOrUUID, + AutoGetCollectionViewMode viewMode, + Date_t deadline, + AutoStatsTracker::LogMode logMode, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs) + : _autoCollForRead(opCtx, nsOrUUID, viewMode, deadline, secondaryNssOrUUIDs), _statsTracker( opCtx, _autoCollForRead.getNss(), @@ -643,71 +829,6 @@ AutoGetCollectionForReadCommandBase<AutoGetCollectionForReadType>:: } } -void AutoGetCollectionMultiForReadCommandLockFree::_secondaryCollectionsRestoreFn( - OperationContext* opCtx) { - const auto readTimestamp = opCtx->recoveryUnit()->getPointInTimeReadTimestamp(opCtx); - auto catalog = CollectionCatalog::get(opCtx); - - // Fetch secondary collections from the catalog and check they're valid to use. - for (auto& secondaryUUID : _secondaryCollectionUUIDs) { - auto sharedCollPtr = catalog->lookupCollectionByUUIDForRead(opCtx, secondaryUUID); - uassertStatusOK(checkSecondaryCollection( - opCtx, /*nss*/ boost::none, secondaryUUID, sharedCollPtr, readTimestamp)); - } -}; - -AutoGetCollectionMultiForReadCommandLockFree::AutoGetCollectionMultiForReadCommandLockFree( - OperationContext* opCtx, - const NamespaceStringOrUUID& primaryNssOrUUID, - std::vector<NamespaceStringOrUUID>& secondaryNsOrUUIDs, - AutoGetCollectionViewMode viewMode, - Date_t deadline, - AutoStatsTracker::LogMode logMode) - // Set up state regularly for a single collection access. This will handle setting up a - // consistent storage snapshot and a PIT in-memory catalog. We can then use the catalog to fetch - // and verify secondary collection state. - : _autoCollForReadCommandLockFree(opCtx, primaryNssOrUUID, viewMode, deadline, logMode) { - if (!_autoCollForReadCommandLockFree) { - return; - } - - // Fetch secondary collection and verify they're valid for use. - { - auto catalog = CollectionCatalog::get(opCtx); - const auto readTimestamp = opCtx->recoveryUnit()->getPointInTimeReadTimestamp(opCtx); - for (auto& secondaryNssOrUUID : secondaryNsOrUUIDs) { - auto nss = catalog->resolveNamespaceStringOrUUID(opCtx, secondaryNssOrUUID); - auto sharedCollPtr = catalog->lookupCollectionByNamespaceForRead(opCtx, nss); - - // Check that 'sharedCollPtr' exists and is safe to use. - uassertStatusOK(checkSecondaryCollection( - opCtx, nss, /*uuid*/ boost::none, sharedCollPtr, readTimestamp)); - - // Duplicate collection names should not be provided via 'secondaryNsOrUUIDs'. - invariant(std::find(_secondaryCollectionUUIDs.begin(), - _secondaryCollectionUUIDs.end(), - sharedCollPtr->uuid()) == _secondaryCollectionUUIDs.end()); - _secondaryCollectionUUIDs.push_back(sharedCollPtr->uuid()); - } - } - - // Create a new restore from yield function to pass into all of the 'primary' CollectionPtr - // instance. It should encapsulate the logic _autoCollForReadCommandLockFree's CollectionPtr - // currently has and then add logic to check the secondary collections' state. - - // Save the 'primary' CollectionPtr's original restore function so that the new restore function - // can reference it. - _primaryCollectionRestoreFn = - _autoCollForReadCommandLockFree._getCollectionPtrForModify().detachRestoreFn(); - - _autoCollForReadCommandLockFree._getCollectionPtrForModify().attachRestoreFn( - [&](OperationContext* opCtx, UUID collUUID) { - const Collection* primaryCollection = _primaryCollectionRestoreFn(opCtx, collUUID); - _secondaryCollectionsRestoreFn(opCtx); - return primaryCollection; - }); -} - OldClientContext::OldClientContext(OperationContext* opCtx, const std::string& ns, bool doVersion) : _opCtx(opCtx) { const auto dbName = nsToDatabaseSubstring(ns); @@ -744,11 +865,12 @@ AutoGetCollectionForReadCommandMaybeLockFree::AutoGetCollectionForReadCommandMay const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode, Date_t deadline, - AutoStatsTracker::LogMode logMode) { + AutoStatsTracker::LogMode logMode, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs) { if (supportsLockFreeRead(opCtx)) { - _autoGetLockFree.emplace(opCtx, nsOrUUID, viewMode, deadline, logMode); + _autoGetLockFree.emplace(opCtx, nsOrUUID, viewMode, deadline, logMode, secondaryNssOrUUIDs); } else { - _autoGet.emplace(opCtx, nsOrUUID, viewMode, deadline, logMode); + _autoGet.emplace(opCtx, nsOrUUID, viewMode, deadline, logMode, secondaryNssOrUUIDs); } } @@ -794,7 +916,9 @@ AutoReadLockFree::AutoReadLockFree(OperationContext* opCtx, Date_t deadline) /* GetCollectionAfterSnapshotFunc */ [&](OperationContext* opCtx, const CollectionCatalog& catalog) { return &fakeColl; }, /* ResetFunc */ - []() {}); + []() {}, + /* SetSecondaryState */ + [](bool isAnySecondaryNamespaceAViewOrSharded) {}); } AutoGetDbForReadLockFree::AutoGetDbForReadLockFree(OperationContext* opCtx, @@ -826,7 +950,9 @@ AutoGetDbForReadLockFree::AutoGetDbForReadLockFree(OperationContext* opCtx, /* GetCollectionAfterSnapshotFunc */ [&](OperationContext* opCtx, const CollectionCatalog& catalog) { return &fakeColl; }, /* ResetFunc */ - []() {}); + []() {}, + /* SetSecondaryState */ + [](bool isAnySecondaryNamespaceAViewOrSharded) {}); } AutoGetDbForReadMaybeLockFree::AutoGetDbForReadMaybeLockFree(OperationContext* opCtx, diff --git a/src/mongo/db/db_raii.h b/src/mongo/db/db_raii.h index dca7dc62f8c..0bc8a1058f9 100644 --- a/src/mongo/db/db_raii.h +++ b/src/mongo/db/db_raii.h @@ -124,19 +124,6 @@ public: } protected: - // So that AutoGetCollectionForReadLockFree & AutoGetCollectionForReadCommandBase can access a - // non-const CollectionPtr from this class. AutoGetCollectionForRead inherits the access. - friend class AutoGetCollectionForReadLockFree; - template <typename AutoGetCollectionForReadType> - friend class AutoGetCollectionForReadCommandBase; - - /** - * Allow access to the CollectionPtr as non-const, for friend classes. - */ - CollectionPtr& _getCollectionPtrForModify() { - return _autoColl->_getCollectionPtrForModify(); - } - // If this field is set, the reader will not take the ParallelBatchWriterMode lock and conflict // with secondary batch application. This stays in scope with the _autoColl so that locks are // taken and released in the right order. @@ -157,7 +144,8 @@ public: EmplaceAutoGetCollectionForRead(OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode, - Date_t deadline); + Date_t deadline, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs); void emplace(boost::optional<AutoGetCollection>& autoColl) const; @@ -167,6 +155,7 @@ private: AutoGetCollectionViewMode _viewMode; Date_t _deadline; LockMode _collectionLockMode; + const std::vector<NamespaceStringOrUUID> _secondaryNssOrUUIDs; }; /** @@ -178,6 +167,10 @@ private: * some command. This will ensure your reads obey any requested readConcern, but will not update the * status of CurrentOp, or add a Top entry. * + * Any collections specified in 'secondaryNssOrUUIDs' will be checked that their minimum visible + * timestamp supports read concern, throwing a SnapshotUnavailable on error. Additional collection + * and/or database locks will be acquired for 'secondaryNssOrUUIDs' namespaces. + * * NOTE: Must not be used with any locks held, because it needs to block waiting on the committed * snapshot to become available, and can potentially release and reacquire locks. */ @@ -188,17 +181,31 @@ public: OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, - Date_t deadline = Date_t::max()); + Date_t deadline = Date_t::max(), + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}); Database* getDb() const { return _autoColl->getDb(); } + + /** + * Indicates whether any namespace in 'secondaryNssOrUUIDs' is a view or sharded. + * + * The secondary namespaces won't be checked if getCollection() returns nullptr. + */ + bool isAnySecondaryNamespaceAViewOrSharded() const { + return _secondaryNssIsAViewOrSharded; + } + +private: + // Tracks whether any secondary collection namespaces is a view or sharded. + bool _secondaryNssIsAViewOrSharded = false; }; /** * Same as AutoGetCollectionForRead above except does not take collection, database or rstl locks. * Takes the global lock and may take the PBWM, same as AutoGetCollectionForRead. Ensures a - * consistent in-memory and on-disk view of the collection. + * consistent in-memory and on-disk view of the storage catalog. */ class AutoGetCollectionForReadLockFree { public: @@ -206,7 +213,8 @@ public: OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, - Date_t deadline = Date_t::max()); + Date_t deadline = Date_t::max(), + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}); explicit operator bool() const { return static_cast<bool>(getCollection()); @@ -232,19 +240,16 @@ public: return _autoGetCollectionForReadBase->getNss(); } -private: - // So that AutoGetCollectionForReadCommandBase can access a non-const CollectionPtr from this - // class. - template <typename AutoGetCollectionForReadType> - friend class AutoGetCollectionForReadCommandBase; - /** - * Allow access to the CollectionPtr as non-const, for friend classes. + * Indicates whether any namespace in 'secondaryNssOrUUIDs' is a view or sharded. + * + * The secondary namespaces won't be checked if getCollection() returns nullptr. */ - CollectionPtr& _getCollectionPtrForModify() { - return _autoGetCollectionForReadBase->_getCollectionPtrForModify(); + bool isAnySecondaryNamespaceAViewOrSharded() const { + return _secondaryNssIsAViewOrSharded; } +private: /** * Helper for how AutoGetCollectionForReadBase instantiates its owned AutoGetCollectionLockFree. */ @@ -271,6 +276,9 @@ private: bool _isLockFreeReadSubOperation; }; + // Tracks whether any secondary collection namespaces is a view or sharded. + bool _secondaryNssIsAViewOrSharded = false; + // The CollectionCatalogStasher must outlive the LockFreeReadsBlock in the AutoGet* below. // ~LockFreeReadsBlock clears a flag that the ~CollectionCatalogStasher checks. CollectionCatalogStasher _catalogStash; @@ -289,7 +297,8 @@ public: OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, - Date_t deadline = Date_t::max()); + Date_t deadline = Date_t::max(), + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}); /** * Passthrough functions to either _autoGet or _autoGetLockFree. @@ -343,20 +352,18 @@ public: return _autoCollForRead.getNss(); } -protected: - /** - * Allow access to the CollectionPtr as non-const, for friend classes of inheriting classes. - */ - CollectionPtr& _getCollectionPtrForModify() { - return _autoCollForRead._getCollectionPtrForModify(); + bool isAnySecondaryNamespaceAViewOrSharded() const { + return _autoCollForRead.isAnySecondaryNamespaceAViewOrSharded(); } +protected: AutoGetCollectionForReadCommandBase( OperationContext* opCtx, const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, Date_t deadline = Date_t::max(), - AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp); + AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}); AutoGetCollectionForReadType _autoCollForRead; AutoStatsTracker _statsTracker; @@ -374,8 +381,10 @@ public: const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, Date_t deadline = Date_t::max(), - AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp) - : AutoGetCollectionForReadCommandBase(opCtx, nsOrUUID, viewMode, deadline, logMode) {} + AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}) + : AutoGetCollectionForReadCommandBase( + opCtx, nsOrUUID, viewMode, deadline, logMode, secondaryNssOrUUIDs) {} Database* getDb() const { return _autoCollForRead.getDb(); @@ -393,85 +402,10 @@ public: const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, Date_t deadline = Date_t::max(), - AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp) - : AutoGetCollectionForReadCommandBase(opCtx, nsOrUUID, viewMode, deadline, logMode) {} - -private: - // So that AutoGetCollectionMultiForReadCommandLockFree can access a non-const CollectionPtr - // from this class. - friend class AutoGetCollectionMultiForReadCommandLockFree; -}; - -/** - * Creates an AutoGetCollectionForReadCommandLockFree instance for the 'primary' collection and - * verifies 'secondary' collections are safe to use for the read. Secondary collections are checked - * for existence and that their minimum visible timestamp supports read concern, throwing a - * NamespaceNotFound or SnapshotUnavailable on error. Secondaries must be unsharded: this is an - * invariant. - * - * Acquires only the Global IS lock, and sets up consistent in-memory and on-disk state. - */ -class AutoGetCollectionMultiForReadCommandLockFree { -public: - /** - * 'primaryNssOrUUID' specifies the collection that can be sharded and on which sharding checks - * will be made. 'secondaryNsOrUUIDs' specifies the additional collections that are not expected - * to be sharded and on which 'not sharded' checks will be made. - */ - AutoGetCollectionMultiForReadCommandLockFree( - OperationContext* opCtx, - const NamespaceStringOrUUID& primaryNssOrUUID, - std::vector<NamespaceStringOrUUID>& secondaryNsOrUUIDs, - AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, - Date_t deadline = Date_t::max(), - AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp); - - explicit operator bool() const { - return static_cast<bool>(getCollection()); - } - - const Collection* operator->() const { - return getCollection().get(); - } - - const CollectionPtr& operator*() const { - return getCollection(); - } - - const CollectionPtr& getCollection() const { - return _autoCollForReadCommandLockFree.getCollection(); - } - - const ViewDefinition* getView() const { - return _autoCollForReadCommandLockFree.getView(); - } - - /** - * Returns the namespace for the 'primary' collection. - */ - const NamespaceString& getNss() const { - return _autoCollForReadCommandLockFree.getNss(); - } - -private: - /** - * Runs on restore from a query yield. The 'primary' CollectionPtr's restore function is - * extended to run this logic. - * - * Fetches the collections identified by '_secondaryCollectionUUIDs' and verifies they're still - * safe to use. - */ - void _secondaryCollectionsRestoreFn(OperationContext* opCtx); - - // Runs all the usual check on the 'primary' collection. - AutoGetCollectionForReadCommandLockFree _autoCollForReadCommandLockFree; - - // Save a reference to the 'primary' CollectionPtr's original restore lambda so that the new one - // can reference it. - std::function<const Collection*(OperationContext*, UUID)> _primaryCollectionRestoreFn; - - // Used on query yield restore to identify the 'secondary' collections to check. - std::vector<UUID> _secondaryCollectionUUIDs; + AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}) + : AutoGetCollectionForReadCommandBase( + opCtx, nsOrUUID, viewMode, deadline, logMode, secondaryNssOrUUIDs) {} }; /** @@ -486,7 +420,8 @@ public: const NamespaceStringOrUUID& nsOrUUID, AutoGetCollectionViewMode viewMode = AutoGetCollectionViewMode::kViewsForbidden, Date_t deadline = Date_t::max(), - AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp); + AutoStatsTracker::LogMode logMode = AutoStatsTracker::LogMode::kUpdateTopAndCurOp, + const std::vector<NamespaceStringOrUUID>& secondaryNssOrUUIDs = {}); /** * Passthrough function to either _autoGet or _autoGetLockFree. diff --git a/src/mongo/db/db_raii_multi_collection_test.cpp b/src/mongo/db/db_raii_multi_collection_test.cpp index 8ed19af5a71..fa47f44c3df 100644 --- a/src/mongo/db/db_raii_multi_collection_test.cpp +++ b/src/mongo/db/db_raii_multi_collection_test.cpp @@ -65,30 +65,98 @@ public: storageInterface()->createCollection(opCtx, _secondaryNss1, defaultCollectionOptions)); ASSERT_OK( storageInterface()->createCollection(opCtx, _secondaryNss2, defaultCollectionOptions)); + ASSERT_OK(storageInterface()->createCollection( + opCtx, _secondaryNssOtherDbNss, defaultCollectionOptions)); + } + + void createCollectionsExceptOneSecondary(OperationContext* opCtx) { + CollectionOptions defaultCollectionOptions; ASSERT_OK( - storageInterface()->createCollection(opCtx, _otherDBNss, defaultCollectionOptions)); + storageInterface()->createCollection(opCtx, _primaryNss, defaultCollectionOptions)); + ASSERT_OK( + storageInterface()->createCollection(opCtx, _secondaryNss1, defaultCollectionOptions)); + ASSERT_OK(storageInterface()->createCollection( + opCtx, _secondaryNssOtherDbNss, defaultCollectionOptions)); } - const NamespaceString _primaryNss = NamespaceString("primary1.db1"); - const NamespaceString _secondaryNss1 = NamespaceString("secondary1.db1"); - const NamespaceString _secondaryNss2 = NamespaceString("secondary2.db1"); - const NamespaceString _otherDBNss = NamespaceString("secondary2.db2"); + const NamespaceString _primaryNss = NamespaceString("db1.primary1"); + const NamespaceString _secondaryNss1 = NamespaceString("db1.secondary1"); + const NamespaceString _secondaryNss2 = NamespaceString("db1.secondary2"); + const NamespaceString _secondaryNssOtherDbNss = NamespaceString("db2.secondary1"); + + const std::vector<NamespaceStringOrUUID> _secondaryNssOrUUIDVec = { + NamespaceStringOrUUID(_secondaryNss1), NamespaceStringOrUUID(_secondaryNss2)}; + const std::vector<NamespaceStringOrUUID> _secondaryNssOtherDbNssVec = { + NamespaceStringOrUUID(_secondaryNssOtherDbNss)}; - std::vector<NamespaceStringOrUUID> _secondaryNssVec = {NamespaceStringOrUUID(_secondaryNss1), - NamespaceStringOrUUID(_secondaryNss2)}; - std::vector<NamespaceStringOrUUID> _otherDBNssVec = {NamespaceStringOrUUID(_otherDBNss)}; + const std::vector<NamespaceStringOrUUID> _secondaryNssOrUUIDAllVec = { + NamespaceStringOrUUID(_secondaryNss1), + NamespaceStringOrUUID(_secondaryNss2), + NamespaceStringOrUUID(_secondaryNssOtherDbNss)}; + + std::vector<NamespaceString> _secondaryNamespacesAll{ + _secondaryNss1, _secondaryNss2, _secondaryNssOtherDbNss}; const ClientAndCtx _client1 = makeClientWithLocker("client1"); const ClientAndCtx _client2 = makeClientWithLocker("client2"); }; -TEST_F(AutoGetCollectionMultiTest, SingleDB) { +TEST_F(AutoGetCollectionMultiTest, SecondaryNssMinimumVisibleError) { + auto opCtx1 = _client1.second.get(); + + // Create a primary and two secondary collections to lock. _secondaryNss1 will not be used later + // for locking. Instead, the timestamp of _secondaryNss1's creation will be used as the read + // timestamp to ensure that a SnapshotUnavailable error is encountered while verifying the + // _secondaryNss2 collection. + CollectionOptions defaultCollectionOptions; + ASSERT_OK(storageInterface()->createCollection(opCtx1, _primaryNss, defaultCollectionOptions)); + ASSERT_OK( + storageInterface()->createCollection(opCtx1, _secondaryNss1, defaultCollectionOptions)); + ASSERT_OK( + storageInterface()->createCollection(opCtx1, _secondaryNss2, defaultCollectionOptions)); + + // Set the read source earlier than Collection _secondaryNss2' min visible timestamp, but later + // than _primaryNss' min visible timestamp. + opCtx1->recoveryUnit()->setTimestampReadSource(RecoveryUnit::ReadSource::kProvided, [&]() { + AutoGetCollection secondaryCollection1(opCtx1, _secondaryNss1, MODE_IS); + return secondaryCollection1->getMinimumVisibleSnapshot(); + }()); + + // Create the AutoGet* instance on multiple collections with _secondaryNss2 and it should throw. + std::vector<NamespaceStringOrUUID> secondaryNamespaces{NamespaceStringOrUUID(_secondaryNss2)}; + ASSERT_THROWS_CODE(AutoGetCollectionForRead(opCtx1, + _primaryNss, + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + secondaryNamespaces), + AssertionException, + ErrorCodes::SnapshotUnavailable); + + // Now set the read source to Collection _secondaryNss2's min visible timestamp and make sure it + // works, as a sanity check. + opCtx1->recoveryUnit()->setTimestampReadSource(RecoveryUnit::ReadSource::kProvided, [&]() { + AutoGetCollection secondaryCollection1(opCtx1, _secondaryNss2, MODE_IS); + return secondaryCollection1->getMinimumVisibleSnapshot(); + }()); + AutoGetCollectionForRead(opCtx1, + _primaryNss, + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + secondaryNamespaces); +} + +TEST_F(AutoGetCollectionMultiTest, LockFreeMultiCollectionSingleDB) { auto opCtx1 = _client1.second.get(); createCollections(opCtx1); - AutoGetCollectionMultiForReadCommandLockFree autoGet( - opCtx1, NamespaceStringOrUUID(_primaryNss), _secondaryNssVec); + invariant(!opCtx1->lockState()->isCollectionLockedForMode(_primaryNss, MODE_IS)); + + AutoGetCollectionForReadLockFree autoGet(opCtx1, + NamespaceStringOrUUID(_primaryNss), + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + _secondaryNssOrUUIDVec); auto locker = opCtx1->lockState(); locker->dump(); @@ -103,27 +171,59 @@ TEST_F(AutoGetCollectionMultiTest, SingleDB) { ASSERT(coll); ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss1)); ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss2)); +} - coll.yield(); - coll.restore(); +TEST_F(AutoGetCollectionMultiTest, LockedDuplicateNamespaces) { + auto opCtx1 = _client1.second.get(); + + const std::vector<NamespaceStringOrUUID> duplicateNssVector = { + NamespaceStringOrUUID(_primaryNss), + NamespaceStringOrUUID(_primaryNss), + NamespaceStringOrUUID(_secondaryNss1), + NamespaceStringOrUUID(_secondaryNss1)}; + + createCollections(opCtx1); + + invariant(!opCtx1->lockState()->isCollectionLockedForMode(_primaryNss, MODE_IS)); + + AutoGetCollectionForRead autoGet(opCtx1, + NamespaceStringOrUUID(_primaryNss), + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + duplicateNssVector); + + auto locker = opCtx1->lockState(); + locker->dump(); + invariant(locker->isLockHeldForMode(resourceIdGlobal, MODE_IS)); + invariant(locker->isDbLockedForMode(_primaryNss.db(), MODE_IS)); + invariant(locker->isDbLockedForMode(_secondaryNss1.db(), MODE_IS)); + // Set 'shouldConflictWithSecondaryBatchApplication' to true so isCollectionLockedForMode() + // doesn't return true regardless of what locks are held. + opCtx1->lockState()->setShouldConflictWithSecondaryBatchApplication(true); + invariant(locker->isCollectionLockedForMode(_primaryNss, MODE_IS)); + invariant(locker->isCollectionLockedForMode(_secondaryNss1, MODE_IS)); + + const auto& coll = autoGet.getCollection(); ASSERT(coll); ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss1)); - ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss2)); } -TEST_F(AutoGetCollectionMultiTest, MultiDBs) { +TEST_F(AutoGetCollectionMultiTest, LockFreeMultiDBs) { auto opCtx1 = _client1.second.get(); createCollections(opCtx1); - AutoGetCollectionMultiForReadCommandLockFree autoGet( - opCtx1, NamespaceStringOrUUID(_primaryNss), _otherDBNssVec); + AutoGetCollectionForReadLockFree autoGet(opCtx1, + NamespaceStringOrUUID(_primaryNss), + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + _secondaryNssOtherDbNssVec); auto locker = opCtx1->lockState(); locker->dump(); invariant(locker->isLockHeldForMode(resourceIdGlobal, MODE_IS)); invariant(!locker->isDbLockedForMode(_primaryNss.db(), MODE_IS)); - invariant(!locker->isDbLockedForMode(_otherDBNss.db(), MODE_IS)); + invariant(!locker->isDbLockedForMode(_secondaryNssOtherDbNss.db(), MODE_IS)); // Set 'shouldConflictWithSecondaryBatchApplication' to true so isCollectionLockedForMode() // doesn't return true regardless of what locks are held. opCtx1->lockState()->setShouldConflictWithSecondaryBatchApplication(true); @@ -131,87 +231,87 @@ TEST_F(AutoGetCollectionMultiTest, MultiDBs) { const auto& coll = autoGet.getCollection(); ASSERT(coll); - ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _otherDBNss)); - - coll.yield(); - coll.restore(); - ASSERT(coll); - ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _otherDBNss)); + ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, + _secondaryNssOtherDbNss)); } -TEST_F(AutoGetCollectionMultiTest, DropCollection) { +TEST_F(AutoGetCollectionMultiTest, LockedMultiDBs) { auto opCtx1 = _client1.second.get(); createCollections(opCtx1); - AutoGetCollectionMultiForReadCommandLockFree autoGet( - opCtx1, NamespaceStringOrUUID(_primaryNss), _secondaryNssVec); + AutoGetCollectionForRead autoGet(opCtx1, + NamespaceStringOrUUID(_primaryNss), + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + _secondaryNssOtherDbNssVec); auto locker = opCtx1->lockState(); locker->dump(); invariant(locker->isLockHeldForMode(resourceIdGlobal, MODE_IS)); - invariant(!locker->isDbLockedForMode(_primaryNss.db(), MODE_IS)); + invariant(locker->isDbLockedForMode(_primaryNss.db(), MODE_IS)); + invariant(locker->isDbLockedForMode(_secondaryNssOtherDbNss.db(), MODE_IS)); // Set 'shouldConflictWithSecondaryBatchApplication' to true so isCollectionLockedForMode() // doesn't return true regardless of what locks are held. opCtx1->lockState()->setShouldConflictWithSecondaryBatchApplication(true); - invariant(!locker->isCollectionLockedForMode(_primaryNss, MODE_IS)); + invariant(locker->isCollectionLockedForMode(_primaryNss, MODE_IS)); + invariant(locker->isCollectionLockedForMode(_secondaryNssOtherDbNss, MODE_IS)); const auto& coll = autoGet.getCollection(); + ASSERT(coll); + ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, + _secondaryNssOtherDbNss)); +} - // Drop a secondary collection via a different opCtx, so that we can test that yield restore - // throws on failing to verify the secondary collection. - { - auto opCtx2 = _client2.second.get(); - AutoGetCollection secondClientAutoGet(opCtx2, _secondaryNss1, MODE_X); +TEST_F(AutoGetCollectionMultiTest, LockFreeSecondaryNamespaceNotFoundIsOK) { + auto opCtx1 = _client1.second.get(); - // Disable replication so that it is not necessary to set up the infrastructure to timestamp - // catalog writes properly. - repl::UnreplicatedWritesBlock uwb(opCtx2); + createCollectionsExceptOneSecondary(opCtx1); - ASSERT_OK(storageInterface()->dropCollection(opCtx2, _secondaryNss1)); - } + AutoGetCollectionForReadLockFree autoGet(opCtx1, + NamespaceStringOrUUID(_primaryNss), + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + _secondaryNssOrUUIDAllVec); - coll.yield(); - ASSERT_THROWS_CODE(coll.restore(), AssertionException, ErrorCodes::NamespaceNotFound); + invariant(opCtx1->lockState()->isLocked()); + ASSERT(!CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss2)); } -TEST_F(AutoGetCollectionMultiTest, DropAndRecreateCollection) { +TEST_F(AutoGetCollectionMultiTest, LockedSecondaryNamespaceNotFound) { auto opCtx1 = _client1.second.get(); - createCollections(opCtx1); + createCollectionsExceptOneSecondary(opCtx1); - AutoGetCollectionMultiForReadCommandLockFree autoGet( - opCtx1, NamespaceStringOrUUID(_primaryNss), _secondaryNssVec); + AutoGetCollectionForRead autoGet(opCtx1, + NamespaceStringOrUUID(_primaryNss), + AutoGetCollectionViewMode::kViewsForbidden, + Date_t::max(), + _secondaryNssOrUUIDAllVec); auto locker = opCtx1->lockState(); - locker->dump(); - invariant(locker->isLockHeldForMode(resourceIdGlobal, MODE_IS)); - invariant(!locker->isDbLockedForMode(_primaryNss.db(), MODE_IS)); + // Set 'shouldConflictWithSecondaryBatchApplication' to true so isCollectionLockedForMode() // doesn't return true regardless of what locks are held. - opCtx1->lockState()->setShouldConflictWithSecondaryBatchApplication(true); - invariant(!locker->isCollectionLockedForMode(_primaryNss, MODE_IS)); - - const auto& coll = autoGet.getCollection(); + locker->setShouldConflictWithSecondaryBatchApplication(true); - // Drop and recreate a secondary collection via a different opCtx, so that we can test that - // yield restore throws on failing to verify the secondary collection. - { - auto opCtx2 = _client2.second.get(); - AutoGetCollection secondClientAutoGet(opCtx2, _secondaryNss1, MODE_X); - // Disable replication so that it is not necessary to set up the infrastructure to timestamp - // catalog writes properly. - repl::UnreplicatedWritesBlock uwb(opCtx2); + invariant(locker->isLocked()); + invariant(locker->isLockHeldForMode(resourceIdGlobal, MODE_IS)); + invariant(locker->isDbLockedForMode(_primaryNss.db(), MODE_IS)); + invariant(locker->isCollectionLockedForMode(_primaryNss, MODE_IS)); - ASSERT_OK(storageInterface()->dropCollection(_client2.second.get(), _secondaryNss1)); - CollectionOptions defaultCollectionOptions; - ASSERT_OK( - storageInterface()->createCollection(opCtx2, _secondaryNss1, defaultCollectionOptions)); + for (const auto& secondaryNss : _secondaryNamespacesAll) { + invariant(locker->isDbLockedForMode(secondaryNss.db(), MODE_IS)); + invariant(locker->isCollectionLockedForMode(secondaryNss, MODE_IS)); } - coll.yield(); - ASSERT_THROWS_CODE(coll.restore(), AssertionException, ErrorCodes::NamespaceNotFound); + const auto& coll = autoGet.getCollection(); + ASSERT(coll); + ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss1)); + ASSERT(CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, + _secondaryNssOtherDbNss)); + ASSERT(!CollectionCatalog::get(opCtx1)->lookupCollectionByNamespace(opCtx1, _secondaryNss2)); } } // namespace diff --git a/src/mongo/db/storage/snapshot_helper.cpp b/src/mongo/db/storage/snapshot_helper.cpp index db0d1e3c207..1245ff9f2ad 100644 --- a/src/mongo/db/storage/snapshot_helper.cpp +++ b/src/mongo/db/storage/snapshot_helper.cpp @@ -187,8 +187,6 @@ ReadSourceChange shouldChangeReadSource(OperationContext* opCtx, const Namespace bool collectionChangesConflictWithRead(boost::optional<Timestamp> collectionMin, boost::optional<Timestamp> readTimestamp) { - // This is the timestamp of the most recent catalog changes to this collection. If this is - // greater than any point in time read timestamps, we should either wait or return an error. if (!collectionMin) { return false; } @@ -198,7 +196,10 @@ bool collectionChangesConflictWithRead(boost::optional<Timestamp> collectionMin, return false; } - // Return if there are no conflicting catalog changes with the readTimestamp. + // If the last change to the collection was before or at the read timestamp, then the storage + // snapshot will match the collection in-memory state. Return true only if there would be an + // inconsistency: a collection with a newer min timestamp would not match an older storage + // snapshot. return *collectionMin > readTimestamp; } } // namespace SnapshotHelper diff --git a/src/mongo/db/storage/snapshot_helper.h b/src/mongo/db/storage/snapshot_helper.h index cc62cbec889..b87aebff351 100644 --- a/src/mongo/db/storage/snapshot_helper.h +++ b/src/mongo/db/storage/snapshot_helper.h @@ -48,6 +48,11 @@ struct ReadSourceChange { */ ReadSourceChange shouldChangeReadSource(OperationContext* opCtx, const NamespaceString& nss); +/** + * Returns true if 'collectionMin' is not compatible with 'readTimestamp'. They are incompatible + * when the read timestamp is older than the latest in-memory collection state: the storage engine + * view would not match the in-memory collection state. + */ bool collectionChangesConflictWithRead(boost::optional<Timestamp> collectionMin, boost::optional<Timestamp> readTimestamp); } // namespace SnapshotHelper |