summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Storch <david.storch@mongodb.com>2022-04-20 00:14:42 +0000
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2022-04-20 15:39:53 +0000
commit4ba6b85fcf077e29226404df0ae21ab30d2a80ba (patch)
tree05f0b0e0af2bba7c0af34f1a58162f6822a19a64
parentadfd03d5eb008910ceabc5bd62aaa50da6aca1ef (diff)
downloadmongo-4ba6b85fcf077e29226404df0ae21ab30d2a80ba.tar.gz
SERVER-65083 Fix SBE plan cache index visibility bug
(cherry picked from commit 58219cc81297ed4c3a38045c55588fe16d59a53a)
-rw-r--r--src/mongo/db/query/canonical_query.h5
-rw-r--r--src/mongo/db/query/get_executor.cpp2
-rw-r--r--src/mongo/db/query/get_executor.h9
-rw-r--r--src/mongo/db/query/plan_cache_key_factory.cpp38
-rw-r--r--src/mongo/db/query/sbe_plan_cache.h38
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
@@ -74,15 +74,6 @@ void filterAllowedIndexEntries(const AllowedIndicesFilter& allowedIndicesFilter,
std::vector<IndexEntry>* 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<IndexEntry>& entries);
-
-/**
* Fills out information about secondary collections held by 'collections' in 'plannerParams'.
*/
std::map<NamespaceString, SecondaryCollectionInfo> fillOutSecondaryCollectionsInformation(
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<Timestamp> 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<IndexCatalog::IndexIterator> 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<Timestamp>{} : currentNewestVisible;
+}
+} // namespace
+
sbe::PlanCacheKey make(const CanonicalQuery& query,
const CollectionPtr& collection,
PlanCacheKeyTag<sbe::PlanCacheKey>) {
+ 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<Timestamp> 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<Timestamp> _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;
};