diff options
-rw-r--r-- | src/mongo/db/catalog/index_consistency.cpp | 54 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_consistency.h | 31 | ||||
-rw-r--r-- | src/mongo/db/catalog/private/record_store_validate_adaptor.cpp | 32 | ||||
-rw-r--r-- | src/mongo/db/index/index_access_method.h | 8 | ||||
-rw-r--r-- | src/mongo/dbtests/validate_tests.cpp | 232 |
5 files changed, 291 insertions, 66 deletions
diff --git a/src/mongo/db/catalog/index_consistency.cpp b/src/mongo/db/catalog/index_consistency.cpp index 211f10dd9a8..8f499ceb386 100644 --- a/src/mongo/db/catalog/index_consistency.cpp +++ b/src/mongo/db/catalog/index_consistency.cpp @@ -114,34 +114,44 @@ void IndexConsistency::addDocKey(const KeyString& ks, int indexNumber) { _addDocKey_inlock(ks, indexNumber); } -void IndexConsistency::removeDocKey(const KeyString& ks, int indexNumber) { +void IndexConsistency::addIndexKey(const KeyString& ks, int indexNumber) { if (indexNumber < 0 || indexNumber >= static_cast<int>(_indexesInfo.size())) { return; } stdx::lock_guard<stdx::mutex> lock(_classMutex); - _removeDocKey_inlock(ks, indexNumber); + _addIndexKey_inlock(ks, indexNumber); } -void IndexConsistency::addIndexKey(const KeyString& ks, int indexNumber) { - - if (indexNumber < 0 || indexNumber >= static_cast<int>(_indexesInfo.size())) { +void IndexConsistency::addMultikeyMetadataPath(const KeyString& ks, int indexNumber) { + if (indexNumber < 0) { return; } + invariant(static_cast<size_t>(indexNumber) < _indexesInfo.size()); stdx::lock_guard<stdx::mutex> lock(_classMutex); - _addIndexKey_inlock(ks, indexNumber); + _indexesInfo[indexNumber].hashedMultikeyMetadataPaths.emplace(_hashKeyString(ks, indexNumber)); } -void IndexConsistency::removeIndexKey(const KeyString& ks, int indexNumber) { - - if (indexNumber < 0 || indexNumber >= static_cast<int>(_indexesInfo.size())) { +void IndexConsistency::removeMultikeyMetadataPath(const KeyString& ks, int indexNumber) { + if (indexNumber < 0) { return; } + invariant(static_cast<size_t>(indexNumber) < _indexesInfo.size()); stdx::lock_guard<stdx::mutex> lock(_classMutex); - _removeIndexKey_inlock(ks, indexNumber); + _indexesInfo[indexNumber].hashedMultikeyMetadataPaths.erase(_hashKeyString(ks, indexNumber)); +} + +size_t IndexConsistency::getMultikeyMetadataPathCount(int indexNumber) { + if (indexNumber < 0) { + return 0; + } + invariant(static_cast<size_t>(indexNumber) < _indexesInfo.size()); + + stdx::lock_guard<stdx::mutex> lock(_classMutex); + return _indexesInfo[indexNumber].hashedMultikeyMetadataPaths.size(); } void IndexConsistency::addLongIndexKey(int indexNumber) { @@ -245,18 +255,6 @@ void IndexConsistency::_addDocKey_inlock(const KeyString& ks, int indexNumber) { _indexesInfo.at(indexNumber).numRecords++; } -void IndexConsistency::_removeDocKey_inlock(const KeyString& ks, int indexNumber) { - - // Ignore indexes that weren't ready before we started validation. - if (!_indexesInfo.at(indexNumber).isReady) { - return; - } - - const uint32_t hash = _hashKeyString(ks, indexNumber); - _indexKeyCount[hash]--; - _indexesInfo.at(indexNumber).numRecords--; -} - void IndexConsistency::_addIndexKey_inlock(const KeyString& ks, int indexNumber) { // Ignore indexes that weren't ready before we started validation. @@ -269,18 +267,6 @@ void IndexConsistency::_addIndexKey_inlock(const KeyString& ks, int indexNumber) _indexesInfo.at(indexNumber).numKeys++; } -void IndexConsistency::_removeIndexKey_inlock(const KeyString& ks, int indexNumber) { - - // Ignore indexes that weren't ready before we started validation. - if (!_indexesInfo.at(indexNumber).isReady) { - return; - } - - const uint32_t hash = _hashKeyString(ks, indexNumber); - _indexKeyCount[hash]++; - _indexesInfo.at(indexNumber).numKeys--; -} - uint32_t IndexConsistency::_hashKeyString(const KeyString& ks, int indexNumber) const { uint32_t indexNsHash = _indexesInfo.at(indexNumber).indexNsHash; diff --git a/src/mongo/db/catalog/index_consistency.h b/src/mongo/db/catalog/index_consistency.h index 9606096317a..cd7c6efebcb 100644 --- a/src/mongo/db/catalog/index_consistency.h +++ b/src/mongo/db/catalog/index_consistency.h @@ -74,11 +74,13 @@ struct IndexInfo { // The number of long keys that are not indexed for the index. int64_t numLongKeys; // The number of records that have a key in their document that referenced back to the - // this index + // this index. int64_t numRecords; // Keeps track of how many indexes were removed (-1) and added (+1) after the // point of validity was set for this index. int64_t numExtraIndexKeys; + // A hashed set of indexed multikey paths (applies to $** indexes only). + std::set<uint32_t> hashedMultikeyMetadataPaths; }; class IndexConsistency final { @@ -91,13 +93,20 @@ public: const bool background); /** - * Helper functions for `_addDocKey`, `_removeDocKey`, `_addIndexKey`, - * and `_removeIndexKey` for concurrency control. + * Helper functions for `_addDocKey` and `_addIndexKey` for concurrency control. */ void addDocKey(const KeyString& ks, int indexNumber); - void removeDocKey(const KeyString& ks, int indexNumber); void addIndexKey(const KeyString& ks, int indexNumber); - void removeIndexKey(const KeyString& ks, int indexNumber); + + /** + * To validate $** multikey metadata paths, we first scan the collection and add a hash of all + * multikey paths encountered to a set. We then scan the index for multikey metadata path + * entries and remove any path encountered. As we expect the index to contain a super-set of + * the collection paths, a non-empty set represents an invalid index. + */ + void addMultikeyMetadataPath(const KeyString& ks, int indexNumber); + void removeMultikeyMetadataPath(const KeyString& ks, int indexNumber); + size_t getMultikeyMetadataPathCount(int indexNumber); /** * Add one to the `_longKeys` count for the given `indexNs`. @@ -201,24 +210,12 @@ private: void _addDocKey_inlock(const KeyString& ks, int indexNumber); /** - * Given the document's key KeyString, decrement the corresponding `_indexKeyCount` - * by hashing it. - */ - void _removeDocKey_inlock(const KeyString& ks, int indexNumber); - - /** * Given the index entry's KeyString, decrement the corresponding `_indexKeyCount` * by hashing it. */ void _addIndexKey_inlock(const KeyString& ks, int indexNumber); /** - * Given the index entry's KeyString, increment the corresponding `_indexKeyCount` - * by hashing it. - */ - void _removeIndexKey_inlock(const KeyString& ks, int indexNumber); - - /** * Returns a hashed value from the given KeyString and index namespace. */ uint32_t _hashKeyString(const KeyString& ks, int indexNumbers) const; diff --git a/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp b/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp index 405540b7c47..dc173706b4b 100644 --- a/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp +++ b/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp @@ -35,6 +35,7 @@ #include "mongo/bson/bsonobj.h" #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/catalog/index_consistency.h" +#include "mongo/db/index/all_paths_access_method.h" #include "mongo/db/index/index_access_method.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/matcher/expression.h" @@ -52,6 +53,12 @@ bool isLargeKeyDisallowed() { return serverGlobalParams.featureCompatibility.getVersion() == ServerGlobalParams::FeatureCompatibility::Version::kFullyDowngradedTo40; } + +KeyString makeWildCardMultikeyMetadataKeyString(const BSONObj& indexKey) { + const auto multikeyMetadataOrd = Ordering::make(BSON("" << 1 << "" << 1)); + const RecordId multikeyMetadataRecordId(RecordId::ReservedId::kAllPathsMultikeyMetadataId); + return {KeyString::kLatestVersion, indexKey, multikeyMetadataOrd, multikeyMetadataRecordId}; +} } Status RecordStoreValidateAdaptor::validate(const RecordId& recordId, @@ -107,6 +114,11 @@ Status RecordStoreValidateAdaptor::validate(const RecordId& recordId, curRecordResults.valid = false; } + for (const auto& key : multikeyMetadataKeys) { + _indexConsistency->addMultikeyMetadataPath(makeWildCardMultikeyMetadataKeyString(key), + indexNumber); + } + const auto& pattern = descriptor->keyPattern(); const Ordering ord = Ordering::make(pattern); bool largeKeyDisallowed = isLargeKeyDisallowed(); @@ -159,12 +171,13 @@ void RecordStoreValidateAdaptor::traverseIndex(const IndexAccessMethod* iam, results->valid = false; } - - // TODO SERVER-36444: Add validation for $** multikey metadata index keys. const RecordId kAllPathsMultikeyMetadataRecordId{ RecordId::ReservedId::kAllPathsMultikeyMetadataId}; if (descriptor->getIndexType() == IndexType::INDEX_ALLPATHS && indexEntry->loc == kAllPathsMultikeyMetadataRecordId) { + _indexConsistency->removeMultikeyMetadataPath( + makeWildCardMultikeyMetadataKeyString(indexEntry->key), indexNumber); + numKeys++; continue; } @@ -175,6 +188,13 @@ void RecordStoreValidateAdaptor::traverseIndex(const IndexAccessMethod* iam, prevIndexKeyString.swap(indexKeyString); } + if (_indexConsistency->getMultikeyMetadataPathCount(indexNumber) > 0) { + results->errors.push_back( + str::stream() << "Index '" << descriptor->indexName() + << "' has one or more missing multikey metadata index keys"); + results->valid = false; + } + *numTraversedKeys = numKeys; } @@ -182,7 +202,6 @@ void RecordStoreValidateAdaptor::traverseRecordStore(RecordStore* recordStore, ValidateCmdLevel level, ValidateResults* results, BSONObjBuilder* output) { - long long nrecords = 0; long long dataSizeTotal = 0; long long nInvalid = 0; @@ -257,7 +276,12 @@ void RecordStoreValidateAdaptor::validateIndexKeyCount(IndexDescriptor* idx, } } - if (results.valid && !idx->isMultikey(_opCtx) && totalKeys > numRecs) { + // Confirm that the number of index entries is not greater than the number of documents in the + // collection. This check is only valid for indexes that are not multikey (indexed arrays + // produce an index key per array entry) and not $** indexes which can produce index keys for + // multiple paths within a single document. + if (results.valid && !idx->isMultikey(_opCtx) && + idx->getIndexType() != IndexType::INDEX_ALLPATHS && totalKeys > numRecs) { std::string err = str::stream() << "index " << idx->indexName() << " is not multi-key, but has more entries (" << numIndexedKeys << ") than documents in the index (" << numRecs - numLongKeys << ")"; diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h index 9fa286de2ac..404ec76243f 100644 --- a/src/mongo/db/index/index_access_method.h +++ b/src/mongo/db/index/index_access_method.h @@ -338,6 +338,14 @@ public: return {}; } + /** + * For test use only. Provides direct access to the SortedDataInterface, allowing tests to write + * invalid entries that would otherwise not be possible via IndexAccessMethod. + */ + SortedDataInterface* getSortedDataInterface_forTest() { + return _newInterface.get(); + } + protected: /** * Fills 'keys' with the keys that should be generated for 'obj' on this index. diff --git a/src/mongo/dbtests/validate_tests.cpp b/src/mongo/dbtests/validate_tests.cpp index 874a3ca172a..8791185e1ba 100644 --- a/src/mongo/dbtests/validate_tests.cpp +++ b/src/mongo/dbtests/validate_tests.cpp @@ -66,6 +66,9 @@ public: _db(nullptr) { _client.createCollection(_ns); { + _origAllPathsKnob = internalQueryAllowAllPathsIndexes.load(); + internalQueryAllowAllPathsIndexes.store(true); + AutoGetCollection autoGetCollection(&_opCtx, _nss, MODE_X); _isInRecordIdOrder = autoGetCollection.getCollection()->getRecordStore()->isInRecordIdOrder(); @@ -75,6 +78,7 @@ public: ~ValidateBase() { _client.dropCollection(_ns); getGlobalServiceContext()->unsetKillAllOperations(); + internalQueryAllowAllPathsIndexes.store(_origAllPathsKnob); } protected: @@ -136,6 +140,7 @@ protected: unique_ptr<AutoGetDb> _autoDb; Database* _db; bool _isInRecordIdOrder; + bool _origAllPathsKnob{false}; }; template <bool full, bool background> @@ -145,7 +150,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -206,7 +211,7 @@ public: ValidateSecondaryIndexCount() : ValidateBase(full, background) {} void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -280,7 +285,7 @@ public: ValidateSecondaryIndex() : ValidateBase(full, background) {} void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -346,7 +351,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -425,7 +430,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -514,7 +519,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -582,7 +587,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -655,7 +660,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -726,7 +731,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -817,7 +822,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -892,7 +897,7 @@ public: void run() { - // Can't do it in background is the RecordStore is not in RecordId order. + // Can't do it in background if the RecordStore is not in RecordId order. if (_background && !_isInRecordIdOrder) { return; } @@ -942,6 +947,209 @@ public: } }; +template <bool full, bool background> +class ValidateWildCardIndex : public ValidateBase { +public: + ValidateWildCardIndex() : ValidateBase(full, background) {} + + void run() { + // Can't perform background validation if the RecordStore is not in RecordId order. + if (_background && !_isInRecordIdOrder) { + return; + } + + // Create a new collection. + lockDb(MODE_X); + Collection* coll; + { + WriteUnitOfWork wunit(&_opCtx); + ASSERT_OK(_db->dropCollection(&_opCtx, _ns)); + coll = _db->createCollection(&_opCtx, _ns); + wunit.commit(); + } + + // Create a $** index. + const auto indexName = "wildcardIndex"; + const auto indexKey = BSON("$**" << 1); + auto status = dbtests::createIndexFromSpec( + &_opCtx, + coll->ns().ns(), + BSON("name" << indexName << "ns" << coll->ns().ns() << "key" << indexKey << "v" + << static_cast<int>(kIndexVersion) + << "background" + << false)); + ASSERT_OK(status); + + // Insert non-multikey documents. + OpDebug* const nullOpDebug = nullptr; + lockDb(MODE_X); + { + WriteUnitOfWork wunit(&_opCtx); + ASSERT_OK( + coll->insertDocument(&_opCtx, + InsertStatement(BSON("_id" << 1 << "a" << 1 << "b" << 1)), + nullOpDebug, + true)); + ASSERT_OK( + coll->insertDocument(&_opCtx, + InsertStatement(BSON("_id" << 2 << "b" << BSON("0" << 1))), + nullOpDebug, + true)); + wunit.commit(); + } + ASSERT_TRUE(checkValid()); + + // Insert multikey documents. + lockDb(MODE_X); + { + WriteUnitOfWork wunit(&_opCtx); + ASSERT_OK(coll->insertDocument( + &_opCtx, + InsertStatement(BSON("_id" << 3 << "mk_1" << BSON_ARRAY(1 << 2 << 3))), + nullOpDebug, + true)); + ASSERT_OK(coll->insertDocument( + &_opCtx, + InsertStatement(BSON("_id" << 4 << "mk_2" << BSON_ARRAY(BSON("e" << 1)))), + nullOpDebug, + true)); + wunit.commit(); + } + ASSERT_TRUE(checkValid()); + + // Insert additional multikey path metadata index keys. + lockDb(MODE_X); + const RecordId recordId(RecordId::ReservedId::kAllPathsMultikeyMetadataId); + IndexCatalog* indexCatalog = coll->getIndexCatalog(); + IndexDescriptor* descriptor = indexCatalog->findIndexByName(&_opCtx, indexName); + auto sortedDataInterface = + indexCatalog->getIndex(descriptor)->getSortedDataInterface_forTest(); + { + WriteUnitOfWork wunit(&_opCtx); + const BSONObj indexKey = BSON("" << 1 << "" + << "non_existent_path"); + auto insertStatus = + sortedDataInterface->insert(&_opCtx, indexKey, recordId, true /* dupsAllowed */); + ASSERT_OK(insertStatus); + wunit.commit(); + } + + // An index whose set of multikey metadata paths is a superset of collection multikey + // metadata paths is valid. + ASSERT_TRUE(checkValid()); + + // Remove the multikey path metadata index key for a path that exists and is multikey in the + // collection. + lockDb(MODE_X); + { + WriteUnitOfWork wunit(&_opCtx); + const BSONObj indexKey = BSON("" << 1 << "" + << "mk_1"); + sortedDataInterface->unindex(&_opCtx, indexKey, recordId, true /* dupsAllowed */); + wunit.commit(); + } + + // An index that is missing one or more multikey metadata fields that exist in the + // collection is not valid. + ASSERT_FALSE(checkValid()); + + releaseDb(); + } +}; + +template <bool full, bool background> +class ValidateWildCardIndexWithProjection : public ValidateBase { +public: + ValidateWildCardIndexWithProjection() : ValidateBase(full, background) {} + + void run() { + // Can't perform background validation if the RecordStore is not in RecordId order. + if (_background && !_isInRecordIdOrder) { + return; + } + + // Create a new collection. + lockDb(MODE_X); + Collection* coll; + { + WriteUnitOfWork wunit(&_opCtx); + ASSERT_OK(_db->dropCollection(&_opCtx, _ns)); + coll = _db->createCollection(&_opCtx, _ns); + wunit.commit(); + } + + // Create a $** index with a projection on "a". + const auto indexName = "wildcardIndex"; + const auto indexKey = BSON("a.$**" << 1); + auto status = dbtests::createIndexFromSpec( + &_opCtx, + coll->ns().ns(), + BSON("name" << indexName << "ns" << coll->ns().ns() << "key" << indexKey << "v" + << static_cast<int>(kIndexVersion) + << "background" + << false)); + ASSERT_OK(status); + + // Insert documents with indexed and not-indexed paths. + OpDebug* const nullOpDebug = nullptr; + lockDb(MODE_X); + { + WriteUnitOfWork wunit(&_opCtx); + ASSERT_OK( + coll->insertDocument(&_opCtx, + InsertStatement(BSON("_id" << 1 << "a" << 1 << "b" << 1)), + nullOpDebug, + true)); + ASSERT_OK( + coll->insertDocument(&_opCtx, + InsertStatement(BSON("_id" << 2 << "a" << BSON("w" << 1))), + nullOpDebug, + true)); + ASSERT_OK(coll->insertDocument( + &_opCtx, + InsertStatement(BSON("_id" << 3 << "a" << BSON_ARRAY("x" << 1))), + nullOpDebug, + true)); + ASSERT_OK(coll->insertDocument( + &_opCtx, InsertStatement(BSON("_id" << 4 << "b" << 2)), nullOpDebug, true)); + ASSERT_OK( + coll->insertDocument(&_opCtx, + InsertStatement(BSON("_id" << 5 << "b" << BSON("y" << 1))), + nullOpDebug, + true)); + ASSERT_OK(coll->insertDocument( + &_opCtx, + InsertStatement(BSON("_id" << 6 << "b" << BSON_ARRAY("z" << 1))), + nullOpDebug, + true)); + wunit.commit(); + } + ASSERT_TRUE(checkValid()); + + lockDb(MODE_X); + IndexCatalog* indexCatalog = coll->getIndexCatalog(); + IndexDescriptor* descriptor = indexCatalog->findIndexByName(&_opCtx, indexName); + auto sortedDataInterface = + indexCatalog->getIndex(descriptor)->getSortedDataInterface_forTest(); + + // Removing a multikey metadata path for a path included in the projection causes validate + // to fail. + lockDb(MODE_X); + { + WriteUnitOfWork wunit(&_opCtx); + const BSONObj indexKey = BSON("" << 1 << "" + << "a"); + RecordId recordId(RecordId::ReservedId::kAllPathsMultikeyMetadataId); + sortedDataInterface->unindex(&_opCtx, indexKey, recordId, true /* dupsAllowed */); + wunit.commit(); + } + ASSERT_FALSE(checkValid()); + + releaseDb(); + } +}; + + class ValidateTests : public Suite { public: ValidateTests() : Suite("validate_tests") {} @@ -970,6 +1178,8 @@ public: add<ValidatePartialIndex<false, true>>(); add<ValidatePartialIndexOnCollectionWithNonIndexableFields<false, false>>(); add<ValidatePartialIndexOnCollectionWithNonIndexableFields<false, true>>(); + add<ValidateWildCardIndex<false, false>>(); + add<ValidateWildCardIndexWithProjection<false, false>>(); // Tests for index validation. add<ValidateIndexEntry<false, false>>(); |