From bdb1299412486fab396214f5745d1768a56a449f Mon Sep 17 00:00:00 2001 From: Benety Goh Date: Wed, 2 Jun 2021 18:59:45 -0400 Subject: 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. --- src/mongo/db/catalog/index_catalog_entry_impl.cpp | 43 +++++++---------------- src/mongo/db/catalog/index_catalog_entry_impl.h | 14 +------- 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 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 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 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 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 lk(_indexMultikeyPathsMutex); - _indexMultikeyPathsForWrite = _indexMultikeyPathsForRead; - _numUncommittedCatalogMultikeyUpdates--; - invariant(_numUncommittedCatalogMultikeyUpdates >= 0, - str::stream() << _ident << descriptor()->indexName()); - } - }); - opCtx->recoveryUnit()->onRollback([this] { - if (_indexTracksMultikeyPathsInCatalog) { - stdx::lock_guard 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 _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 _minVisibleSnapshot; -- cgit v1.2.1