From 4ba6b85fcf077e29226404df0ae21ab30d2a80ba Mon Sep 17 00:00:00 2001 From: David Storch Date: Wed, 20 Apr 2022 00:14:42 +0000 Subject: SERVER-65083 Fix SBE plan cache index visibility bug (cherry picked from commit 58219cc81297ed4c3a38045c55588fe16d59a53a) --- src/mongo/db/query/canonical_query.h | 5 ++++ src/mongo/db/query/get_executor.cpp | 2 ++ src/mongo/db/query/get_executor.h | 9 ------- src/mongo/db/query/plan_cache_key_factory.cpp | 38 +++++++++++++++++++++++++++ src/mongo/db/query/sbe_plan_cache.h | 38 ++++++++++++++++++++++++++- 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h index 405d317096b..c06051347d6 100644 --- a/src/mongo/db/query/canonical_query.h +++ b/src/mongo/db/query/canonical_query.h @@ -245,6 +245,11 @@ public: _explain = explain; } + OperationContext* getOpCtx() const { + tassert(6508300, "'CanonicalQuery' does not have an 'ExpressionContext'", _expCtx); + return _expCtx->opCtx; + } + auto& getExpCtx() const { return _expCtx; } diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 2a5fcad2cc8..d09343e18a6 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -277,6 +277,7 @@ void applyIndexFilters(const CollectionPtr& collection, } } +namespace { void fillOutIndexEntries(OperationContext* opCtx, bool apiStrict, const CanonicalQuery* canonicalQuery, @@ -301,6 +302,7 @@ void fillOutIndexEntries(OperationContext* opCtx, indexEntryFromIndexCatalogEntry(opCtx, collection, *ice, canonicalQuery)); } } +} // namespace void fillOutPlannerParams(OperationContext* opCtx, const CollectionPtr& collection, diff --git a/src/mongo/db/query/get_executor.h b/src/mongo/db/query/get_executor.h index 3dd29d878f4..20ca265bcbb 100644 --- a/src/mongo/db/query/get_executor.h +++ b/src/mongo/db/query/get_executor.h @@ -73,15 +73,6 @@ boost::intrusive_ptr makeExpressionContextForGetExecutor( void filterAllowedIndexEntries(const AllowedIndicesFilter& allowedIndicesFilter, std::vector* indexEntries); -/** - * Fills out 'entries' with information about the indexes on 'collection'. - */ -void fillOutIndexEntries(OperationContext* opCtx, - bool apiStrict, - const CanonicalQuery* canonicalQuery, - const CollectionPtr& collection, - std::vector& entries); - /** * Fills out information about secondary collections held by 'collections' in 'plannerParams'. */ diff --git a/src/mongo/db/query/plan_cache_key_factory.cpp b/src/mongo/db/query/plan_cache_key_factory.cpp index 239c777d1be..01d6a8ef934 100644 --- a/src/mongo/db/query/plan_cache_key_factory.cpp +++ b/src/mongo/db/query/plan_cache_key_factory.cpp @@ -94,14 +94,52 @@ PlanCacheKey make(const CanonicalQuery& query, return {makePlanCacheKeyInfo(query, collection)}; } +namespace { +/** + * Returns the highest index commit timestamp associated with an index on 'collection' that is + * visible to this operation. + */ +boost::optional computeNewestVisibleIndexTimestamp(OperationContext* opCtx, + const CollectionPtr& collection) { + auto recoveryUnit = opCtx->recoveryUnit(); + auto mySnapshot = recoveryUnit->getPointInTimeReadTimestamp(opCtx).get_value_or( + recoveryUnit->getCatalogConflictingTimestamp()); + if (mySnapshot.isNull()) { + return boost::none; + } + + Timestamp currentNewestVisible = Timestamp::min(); + + std::unique_ptr ii = + collection->getIndexCatalog()->getIndexIterator(opCtx, /*includeUnfinishedIndexes*/ true); + while (ii->more()) { + const IndexCatalogEntry* ice = ii->next(); + auto minVisibleSnapshot = ice->getMinimumVisibleSnapshot(); + if (!minVisibleSnapshot) { + continue; + } + + if (mySnapshot < *minVisibleSnapshot) { + continue; + } + + currentNewestVisible = std::max(currentNewestVisible, *minVisibleSnapshot); + } + + return currentNewestVisible.isNull() ? boost::optional{} : currentNewestVisible; +} +} // namespace + sbe::PlanCacheKey make(const CanonicalQuery& query, const CollectionPtr& collection, PlanCacheKeyTag) { + OperationContext* opCtx = query.getOpCtx(); auto collectionVersion = CollectionQueryInfo::get(collection).getPlanCacheInvalidatorVersion(); return {makePlanCacheKeyInfo(query, collection), collection->uuid(), collectionVersion, + computeNewestVisibleIndexTimestamp(opCtx, collection), collection.isSharded()}; } } // namespace plan_cache_detail diff --git a/src/mongo/db/query/sbe_plan_cache.h b/src/mongo/db/query/sbe_plan_cache.h index 929fde26d9c..3e16371fcbd 100644 --- a/src/mongo/db/query/sbe_plan_cache.h +++ b/src/mongo/db/query/sbe_plan_cache.h @@ -50,10 +50,12 @@ public: PlanCacheKey(PlanCacheKeyInfo&& info, UUID collectionUuid, size_t collectionVersion, + boost::optional newestVisibleIndexTimestamp, bool isShardedCollection) : _info{std::move(info)}, _collectionUuid{collectionUuid}, _collectionVersion{collectionVersion}, + _newestVisibleIndexTimestamp{newestVisibleIndexTimestamp}, _isShardedCollection{isShardedCollection} {} const UUID& getCollectionUuid() const { @@ -71,7 +73,9 @@ public: bool operator==(const PlanCacheKey& other) const { return other._collectionVersion == _collectionVersion && other._isShardedCollection == _isShardedCollection && - other._collectionUuid == _collectionUuid && other._info == _info; + other._collectionUuid == _collectionUuid && + other._newestVisibleIndexTimestamp == _newestVisibleIndexTimestamp && + other._info == _info; } bool operator!=(const PlanCacheKey& other) const { @@ -86,6 +90,9 @@ public: size_t hash = _info.planCacheKeyHash(); boost::hash_combine(hash, UUID::Hash{}(_collectionUuid)); boost::hash_combine(hash, _collectionVersion); + if (_newestVisibleIndexTimestamp) { + boost::hash_combine(hash, _newestVisibleIndexTimestamp->asULL()); + } boost::hash_combine(hash, _isShardedCollection); return hash; } @@ -95,9 +102,38 @@ public: } private: + // Contains the actual encoding of the query shape as well as the index discriminators. const PlanCacheKeyInfo _info; + const UUID _collectionUuid; + + // There is a special collection versioning scheme associated with the SBE plan cache. Whenever + // an action against a collection is made which should invalidate the plan cache entries for the + // collection -- in particular index builds and drops -- the version number is incremented. + // Readers specify the version number that they are reading at so that they only pick up cache + // entries with the right set of indexes. + // + // We also clean up all cache entries for a particular (collectionUuid, versionNumber) pair when + // all readers seeing this version of the collection have drained. const size_t _collectionVersion; + + // The '_collectionVersion' is not currently sufficient in order to ensure that the indexes + // visible to the reader are consistent with the indexes present in the cache entry. The reason + // is that all readers see the latest copy-on-write version of the 'Collection' object, even + // though they are allowed to read at an older timestamp, potentially at a time before an index + // build completed. + // + // To solve this problem, we incorporate the timestamp of the newest index visible to the reader + // into the plan cache key. This ensures that the set of indexes visible to the reader match + // those present in the plan cache entry, preventing a situation where the plan cache entry + // reflects a newer version of the index catalog than the one visible to the reader. + // + // In the future, this could instead be solved with point-in-time catalog lookups. + const boost::optional _newestVisibleIndexTimestamp; + + // Ensures that a cached SBE plan cannot be reused if the collection has since become sharded. + // The cached plan may no longer be valid after sharding, since orphan filtering is necessary + // for sharded collections. const bool _isShardedCollection; }; -- cgit v1.2.1