SERVER-56877 track uncommitted changes to multikey paths with a counter

This change applies the technique in commit 3566db153ea61fb10d3ef11ea917fc7bc93eac4d
for the isMultikey boolean flags to the reader and writer multikey paths.

This commit is designed to support pre-5.0 backports and will be superseded by a
new patch that uses the Collection metadata introduced in 5.0.

(cherry picked from commit 5d8ac7127e207e0ef1aeaa90dcc62cda6896b71e)

5 files changed, 49 insertions, 23 deletions

diff --git a/jstests/noPassthrough/validate_multikey_compound.js b/jstests/noPassthrough/validate_multikey_compound.js
index e354b9af3de..f923b94e562 100644
--- a/jstests/noPassthrough/validate_multikey_compound.js
+++ b/jstests/noPassthrough/validate_multikey_compound.js
@@ -71,9 +71,9 @@ assert(result.indexDetails.c_hashed.valid, tojson(result));
 
 // Check multikey index.
 assert.eq(6, result.keysPerIndex.a_1_b_1, tojson(result));
-assert(!result.indexDetails.a_1_b_1.valid, tojson(result));
+assert(result.indexDetails.a_1_b_1.valid, tojson(result));
 
-assert(!result.valid, tojson(result));
+assert(result.valid, tojson(result));
 
-rst.stopSet(/*signal=*/undefined, /*forRestart=*/undefined, {skipValidation: true});
+rst.stopSet();
 })();
diff --git a/jstests/noPassthrough/validate_multikey_restart.js b/jstests/noPassthrough/validate_multikey_restart.js
index 68a50adf4cf..e57f84c2229 100644
--- a/jstests/noPassthrough/validate_multikey_restart.js
+++ b/jstests/noPassthrough/validate_multikey_restart.js
@@ -58,18 +58,12 @@ assert.eq(1, docs[0]._id, 'unexpected document content in collection: ' + tojson
 
 jsTestLog('Checking multikey query after restart');
 multikeyQueryDocs = testColl.find({a: {$in: [4, 5, 6]}}).toArray();
-assert.eq(3,
+assert.eq(1,
           multikeyQueryDocs.length,
           'too many docs in multikey query result: ' + tojson(multikeyQueryDocs));
 assert.eq(1,
           multikeyQueryDocs[0]._id,
           'unexpected document content in multikey query result: ' + tojson(multikeyQueryDocs));
-assert.eq(1,
-          multikeyQueryDocs[1]._id,
-          'unexpected document content in multikey query result: ' + tojson(multikeyQueryDocs));
-assert.eq(1,
-          multikeyQueryDocs[2]._id,
-          'unexpected document content in multikey query result: ' + tojson(multikeyQueryDocs));
 
 jsTestLog('Validating collection after restart');
 const result = assert.commandWorked(testColl.validate({full: true}));
@@ -88,9 +82,9 @@ assert(result.indexDetails.b_hashed.valid, tojson(result));
 
 // Check multikey index.
 assert.eq(3, result.keysPerIndex.a_1, tojson(result));
-assert(!result.indexDetails.a_1.valid, tojson(result));
+assert(result.indexDetails.a_1.valid, tojson(result));
 
-assert(!result.valid, tojson(result));
+assert(result.valid, tojson(result));
 
-rst.stopSet(/*signal=*/undefined, /*forRestart=*/undefined, {skipValidation: true});
+rst.stopSet();
 })();
diff --git a/jstests/noPassthrough/validate_multikey_stepdown.js b/jstests/noPassthrough/validate_multikey_stepdown.js
index b05ec37fa06..9ca5012a656 100644
--- a/jstests/noPassthrough/validate_multikey_stepdown.js
+++ b/jstests/noPassthrough/validate_multikey_stepdown.js
@@ -121,9 +121,9 @@ assert(result.indexDetails._id_.valid, tojson(result));
 
 // Check geo index.
 assert.lt(1, result.keysPerIndex.geo_2dsphere, tojson(result));
-assert(!result.indexDetails.geo_2dsphere.valid, tojson(result));
+assert(result.indexDetails.geo_2dsphere.valid, tojson(result));
 
-assert(!result.valid, tojson(result));
+assert(result.valid, tojson(result));
 
-rst.stopSet(/*signal=*/undefined, /*forRestart=*/undefined, {skipValidation: true});
+rst.stopSet();
 })();
diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.cpp b/src/mongo/db/catalog/index_catalog_entry_impl.cpp
index d2d570c456b..256954bd154 100644
--- a/src/mongo/db/catalog/index_catalog_entry_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_entry_impl.cpp
@@ -81,10 +81,12 @@ IndexCatalogEntryImpl::IndexCatalogEntryImpl(OperationContext* const opCtx,
 
     {
         stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-        const bool isMultikey = _catalogIsMultikey(opCtx, &_indexMultikeyPathsForRead);
+        const bool isMultikey = _catalogIsMultikey(opCtx, &_indexMultikeyPathsForWrite);
         _isMultikeyForRead.store(isMultikey);
         _isMultikeyForWrite.store(isMultikey);
-        _indexTracksMultikeyPathsInCatalog = !_indexMultikeyPathsForRead.empty();
+        _indexMultikeyPathsForRead = _indexMultikeyPathsForWrite;
+        _numUncommittedCatalogMultikeyUpdates = 0;
+        _indexTracksMultikeyPathsInCatalog = !_indexMultikeyPathsForWrite.empty();
     }
 
     const BSONObj& collation = _descriptor->collation();
@@ -196,12 +198,12 @@ void IndexCatalogEntryImpl::setMultikey(OperationContext* opCtx,
 
     if (_indexTracksMultikeyPathsInCatalog) {
         stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
-        invariant(multikeyPaths.size() == _indexMultikeyPathsForRead.size());
+        invariant(multikeyPaths.size() == _indexMultikeyPathsForWrite.size());
 
         bool newPathIsMultikey = false;
         for (size_t i = 0; i < multikeyPaths.size(); ++i) {
-            if (!std::includes(_indexMultikeyPathsForRead[i].begin(),
-                               _indexMultikeyPathsForRead[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
@@ -371,9 +373,13 @@ void IndexCatalogEntryImpl::_catalogSetMultikey(OperationContext* opCtx,
     _isMultikeyForRead.store(true);
     if (_indexTracksMultikeyPathsInCatalog) {
         stdx::lock_guard<Latch> lk(_indexMultikeyPathsMutex);
+        if (_numUncommittedCatalogMultikeyUpdates == 0) {
+            _indexMultikeyPathsForRead = _indexMultikeyPathsForWrite;
+        }
         for (size_t i = 0; i < multikeyPaths.size(); ++i) {
             _indexMultikeyPathsForRead[i].insert(multikeyPaths[i].begin(), multikeyPaths[i].end());
         }
+        _numUncommittedCatalogMultikeyUpdates++;
     }
     if (indexMetadataHasChanged && _queryInfo) {
         LOGV2_DEBUG(4718705,
@@ -389,6 +395,21 @@ 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 48acd848b45..2ca092a7eae 100644
--- a/src/mongo/db/catalog/index_catalog_entry_impl.h
+++ b/src/mongo/db/catalog/index_catalog_entry_impl.h
@@ -248,7 +248,8 @@ private:
     // this point, future writers do not need to update the catalog.
     AtomicWord<bool> _isMultikeyForWrite;
 
-    // Controls concurrent access to '_indexMultikeyPathsForRead'.
+    // Controls concurrent access to '_indexMultikeyPathsForRead', '_indexMultikeyPathsForWrite',
+    // and '_numUncommittedCatalogMultikeyUpdates'.
     // 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.
@@ -261,7 +262,17 @@ private:
     // in the index key pattern. Each element in the vector is an ordered set of positions (starting
     // at 0) into the corresponding indexed field that represent what prefixes of the indexed field
     // causes the index to be multikey.
-    MultikeyPaths _indexMultikeyPathsForRead;  // May include paths not committed to catalog.
+    MultikeyPaths _indexMultikeyPathsForRead;   // May include paths not committed to catalog.
+    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.
+    int _numUncommittedCatalogMultikeyUpdates = 0;
 
     // KVPrefix used to differentiate between index entries in different logical indexes sharing the
     // same underlying sorted data interface.