SERVER-65083 Fix SBE plan cache index visibility bug

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 8094a658374..a0d4ddb01bc 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;
 };