SERVER-56877 IndexCatalogEntryImpl::setIndexIsMultikey() uses multikey paths in Collection metadata

This change depends on improvements made in commit 11de948b0c50df7d12de09ae0f01e791fc5d70d7 and
cannot be backported to pre-5.0 branches.

2 files changed, 13 insertions, 44 deletions

diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.cpp b/src/mongo/db/catalog/index_catalog_entry_impl.cpp
index 4189698ea09..30158e9f02c 100644
--- a/src/mongo/db/catalog/index_catalog_entry_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_entry_impl.cpp
@@ -78,12 +78,10 @@ IndexCatalogEntryImpl::IndexCatalogEntryImpl(OperationContext* const opCtx,
 
     {
         stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-        const bool isMultikey = _catalogIsMultikey(opCtx, collection, &_indexMultikeyPathsForWrite);
+        const bool isMultikey = _catalogIsMultikey(opCtx, collection, &_indexMultikeyPathsForRead);
         _isMultikeyForRead.store(isMultikey);
         _isMultikeyForWrite.store(isMultikey);
-        _indexMultikeyPathsForRead = _indexMultikeyPathsForWrite;
-        _numUncommittedCatalogMultikeyUpdates = 0;
-        _indexTracksMultikeyPathsInCatalog = !_indexMultikeyPathsForWrite.empty();
+        _indexTracksMultikeyPathsInCatalog = !_indexMultikeyPathsForRead.empty();
     }
 
     auto nss = DurableCatalog::get(opCtx)->getEntry(_catalogId).nss;
@@ -185,13 +183,15 @@ void IndexCatalogEntryImpl::setMultikey(OperationContext* opCtx,
     }
 
     if (_indexTracksMultikeyPathsInCatalog) {
-        stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-        invariant(multikeyPaths.size() == _indexMultikeyPathsForWrite.size());
+        MultikeyPaths indexMultikeyPathsForWrite;
+        [[maybe_unused]] const bool isMultikeyInCatalog =
+            _catalogIsMultikey(opCtx, collection, &indexMultikeyPathsForWrite);
+        invariant(multikeyPaths.size() == indexMultikeyPathsForWrite.size());
 
         bool newPathIsMultikey = false;
         for (size_t i = 0; i < multikeyPaths.size(); ++i) {
-            if (!std::includes(_indexMultikeyPathsForWrite[i].begin(),
-                               _indexMultikeyPathsForWrite[i].end(),
+            if (!std::includes(indexMultikeyPathsForWrite[i].begin(),
+                               indexMultikeyPathsForWrite[i].end(),
                                multikeyPaths[i].begin(),
                                multikeyPaths[i].end())) {
                 // If 'multikeyPaths' contains a new path component that causes this index to be
@@ -276,12 +276,10 @@ void IndexCatalogEntryImpl::forceSetMultikey(OperationContext* const opCtx,
     {
         stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
         const bool isMultikeyInCatalog =
-            _catalogIsMultikey(opCtx, coll, &_indexMultikeyPathsForWrite);
+            _catalogIsMultikey(opCtx, coll, &_indexMultikeyPathsForRead);
         _isMultikeyForRead.store(isMultikeyInCatalog);
         _isMultikeyForWrite.store(isMultikeyInCatalog);
-        _indexMultikeyPathsForRead = _indexMultikeyPathsForWrite;
-        _numUncommittedCatalogMultikeyUpdates = 0;
-        _indexTracksMultikeyPathsInCatalog = !_indexMultikeyPathsForWrite.empty();
+        _indexTracksMultikeyPathsInCatalog = !_indexMultikeyPathsForRead.empty();
     }
 
     // Since multikey metadata has changed, invalidate the query cache.
@@ -398,13 +396,11 @@ void IndexCatalogEntryImpl::_catalogSetMultikey(OperationContext* opCtx,
     _isMultikeyForRead.store(true);
     if (_indexTracksMultikeyPathsInCatalog) {
         stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-        if (_numUncommittedCatalogMultikeyUpdates == 0) {
-            _indexMultikeyPathsForRead = _indexMultikeyPathsForWrite;
-        }
+        [[maybe_unused]] const bool isMultikeyInCatalog =
+            _catalogIsMultikey(opCtx, collection, &_indexMultikeyPathsForRead);
         for (size_t i = 0; i < multikeyPaths.size(); ++i) {
             _indexMultikeyPathsForRead[i].insert(multikeyPaths[i].begin(), multikeyPaths[i].end());
         }
-        _numUncommittedCatalogMultikeyUpdates++;
     }
     if (indexMetadataHasChanged) {
         LOGV2_DEBUG(4718705,
@@ -420,21 +416,6 @@ void IndexCatalogEntryImpl::_catalogSetMultikey(OperationContext* opCtx,
         // transaction successfully commits. Only after this point may a writer optimize out
         // flipping multikey.
         _isMultikeyForWrite.store(true);
-        if (_indexTracksMultikeyPathsInCatalog) {
-            stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-            _indexMultikeyPathsForWrite = _indexMultikeyPathsForRead;
-            _numUncommittedCatalogMultikeyUpdates--;
-            invariant(_numUncommittedCatalogMultikeyUpdates >= 0,
-                      str::stream() << _ident << descriptor()->indexName());
-        }
-    });
-    opCtx->recoveryUnit()->onRollback([this] {
-        if (_indexTracksMultikeyPathsInCatalog) {
-            stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-            _numUncommittedCatalogMultikeyUpdates--;
-            invariant(_numUncommittedCatalogMultikeyUpdates >= 0,
-                      str::stream() << _ident << ":" << descriptor()->indexName());
-        }
     });
 }
 
diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.h b/src/mongo/db/catalog/index_catalog_entry_impl.h
index fbfbcaf6d4a..6a6f84ff3aa 100644
--- a/src/mongo/db/catalog/index_catalog_entry_impl.h
+++ b/src/mongo/db/catalog/index_catalog_entry_impl.h
@@ -248,8 +248,7 @@ private:
     // this point, future writers do not need to update the catalog.
     mutable AtomicWord<bool> _isMultikeyForWrite;
 
-    // Controls concurrent access to '_indexMultikeyPathsForRead', '_indexMultikeyPathsForWrite',
-    // and '_numUncommittedCatalogMultikeyUpdates'.
+    // Controls concurrent access to '_indexMultikeyPathsForRead'.
     // We acquire this mutex rather than the RESOURCE_METADATA lock as a performance optimization
     // so that it is cheaper to detect whether there is actually any path-level multikey
     // information to update or not.
@@ -264,17 +263,6 @@ private:
     // causes the index to be multikey.
     mutable MultikeyPaths
         _indexMultikeyPathsForRead;  // May include paths not committed to catalog.
-    mutable MultikeyPaths
-        _indexMultikeyPathsForWrite;  // Paths in catalog updated by a transaction commit.
-
-    // If this counter is greater than zero, we have state in '_indexMultikeyPathsForRead' that have
-    // not been successfully updated in the durable catalog. This counter corresponds to the number
-    // of calls _catalogSetMultikey() with open storage transactions.
-    // It is possible for the read paths to contain path info that are not present in the durable
-    // catalog, but all the durable paths must be present in the in-memory state (read paths). This
-    // may lead to less than optimal query plans, but should not result in incorrect plans due to
-    // insufficient multikey coverage.
-    mutable int _numUncommittedCatalogMultikeyUpdates = 0;
 
     // The earliest snapshot that is allowed to read this index.
     boost::optional<Timestamp> _minVisibleSnapshot;