diff options
author | Gregory Wlodarek <gregory.wlodarek@mongodb.com> | 2019-10-01 22:29:40 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-10-01 22:29:40 +0000 |
commit | 32615d54bab1284b28443d493ef5f32a2cdeaf20 (patch) | |
tree | 4543f5e109a47f059c03c15295d8c9c81efecd92 /src/mongo | |
parent | 8be05f0607113b2406d6d6c4fc5c0f998b012812 (diff) | |
download | mongo-32615d54bab1284b28443d493ef5f32a2cdeaf20.tar.gz |
SERVER-42974 Validate should work with KeyString instead of BSON where possible
Diffstat (limited to 'src/mongo')
-rw-r--r-- | src/mongo/db/catalog/collection_validation.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_consistency.cpp | 28 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_consistency.h | 18 | ||||
-rw-r--r-- | src/mongo/db/catalog/throttle_cursor.cpp | 22 | ||||
-rw-r--r-- | src/mongo/db/catalog/throttle_cursor.h | 3 | ||||
-rw-r--r-- | src/mongo/db/catalog/validate_adaptor.cpp | 72 | ||||
-rw-r--r-- | src/mongo/db/catalog/validate_adaptor.h | 2 | ||||
-rw-r--r-- | src/mongo/db/index/index_descriptor.h | 8 | ||||
-rw-r--r-- | src/mongo/db/storage/index_entry_comparison.h | 4 | ||||
-rw-r--r-- | src/mongo/dbtests/validate_tests.cpp | 61 |
10 files changed, 72 insertions, 150 deletions
diff --git a/src/mongo/db/catalog/collection_validation.cpp b/src/mongo/db/catalog/collection_validation.cpp index d2a76a9750d..ae13c779c3f 100644 --- a/src/mongo/db/catalog/collection_validation.cpp +++ b/src/mongo/db/catalog/collection_validation.cpp @@ -129,7 +129,7 @@ void _validateIndexes(OperationContext* opCtx, ValidateResults& curIndexResults = (*indexNsResultsMap)[descriptor->indexName()]; int64_t numTraversedKeys; - indexValidator->traverseIndex(opCtx, descriptor, &numTraversedKeys, &curIndexResults); + indexValidator->traverseIndex(opCtx, index.get(), &numTraversedKeys, &curIndexResults); // If we are performing a full validation, we have information on the number of index keys // validated in _validateIndexesInternalStructure (when we validated the internal structure @@ -207,7 +207,7 @@ void _gatherIndexEntryErrors(OperationContext* opCtx, << descriptor->indexName() << "."; indexValidator->traverseIndex(opCtx, - descriptor, + index.get(), /*numTraversedKeys=*/nullptr, /*ValidateResults=*/nullptr); } diff --git a/src/mongo/db/catalog/index_consistency.cpp b/src/mongo/db/catalog/index_consistency.cpp index 781e544210b..2021f6f1935 100644 --- a/src/mongo/db/catalog/index_consistency.cpp +++ b/src/mongo/db/catalog/index_consistency.cpp @@ -53,8 +53,7 @@ IndexInfo::IndexInfo(const IndexDescriptor* descriptor) : indexName(descriptor->indexName()), keyPattern(descriptor->keyPattern()), indexNameHash(hash(descriptor->indexName())), - ord(Ordering::make(descriptor->keyPattern())), - ks(std::make_unique<KeyString::Builder>(KeyString::Version::kLatestVersion)) {} + ord(Ordering::make(descriptor->keyPattern())) {} IndexConsistency::IndexConsistency(OperationContext* opCtx, CollectionValidation::ValidateState* validateState) @@ -67,11 +66,11 @@ IndexConsistency::IndexConsistency(OperationContext* opCtx, } } -void IndexConsistency::addMultikeyMetadataPath(const KeyString::Builder& ks, IndexInfo* indexInfo) { +void IndexConsistency::addMultikeyMetadataPath(const KeyString::Value& ks, IndexInfo* indexInfo) { indexInfo->hashedMultikeyMetadataPaths.emplace(_hashKeyString(ks, indexInfo->indexNameHash)); } -void IndexConsistency::removeMultikeyMetadataPath(const KeyString::Builder& ks, +void IndexConsistency::removeMultikeyMetadataPath(const KeyString::Value& ks, IndexInfo* indexInfo) { indexInfo->hashedMultikeyMetadataPaths.erase(_hashKeyString(ks, indexInfo->indexNameHash)); } @@ -186,10 +185,9 @@ void IndexConsistency::addIndexEntryErrors(ValidateResultsMap* indexNsResultsMap } void IndexConsistency::addDocKey(OperationContext* opCtx, - const KeyString::Builder& ks, + const KeyString::Value& ks, IndexInfo* indexInfo, - RecordId recordId, - const BSONObj& indexKey) { + RecordId recordId) { const uint32_t hash = _hashKeyString(ks, indexInfo->indexNameHash); if (_firstPhase) { @@ -210,19 +208,20 @@ void IndexConsistency::addDocKey(OperationContext* opCtx, idKey = data["_id"]; } - std::string key = std::string(ks.getBuffer(), ks.getSize()); + auto indexKey = + KeyString::toBsonSafe(ks.getBuffer(), ks.getSize(), indexInfo->ord, ks.getTypeBits()); BSONObj info = _generateInfo(*indexInfo, recordId, indexKey, idKey); // Cannot have duplicate KeyStrings during the document scan phase. + std::string key = std::string(ks.getBuffer(), ks.getSize()); invariant(_missingIndexEntries.count(key) == 0); _missingIndexEntries.insert(std::make_pair(key, info)); } } -void IndexConsistency::addIndexKey(const KeyString::Builder& ks, +void IndexConsistency::addIndexKey(const KeyString::Value& ks, IndexInfo* indexInfo, - RecordId recordId, - const BSONObj& indexKey) { + RecordId recordId) { const uint32_t hash = _hashKeyString(ks, indexInfo->indexNameHash); if (_firstPhase) { @@ -235,10 +234,11 @@ void IndexConsistency::addIndexKey(const KeyString::Builder& ks, // If there is a corresponding document key for the index entry key, we remove the key from // the '_missingIndexEntries' map. However if there was no document key for the index entry // key, we add the key to the '_extraIndexEntries' map. - - std::string key = std::string(ks.getBuffer(), ks.getSize()); + auto indexKey = + KeyString::toBsonSafe(ks.getBuffer(), ks.getSize(), indexInfo->ord, ks.getTypeBits()); BSONObj info = _generateInfo(*indexInfo, recordId, indexKey, boost::none); + std::string key = std::string(ks.getBuffer(), ks.getSize()); if (_missingIndexEntries.count(key) == 0) { // We may have multiple extra index entries for a given KeyString. auto search = _extraIndexEntries.find(key); @@ -287,7 +287,7 @@ BSONObj IndexConsistency::_generateInfo(const IndexInfo& indexInfo, } } -uint32_t IndexConsistency::_hashKeyString(const KeyString::Builder& ks, +uint32_t IndexConsistency::_hashKeyString(const KeyString::Value& ks, uint32_t indexNameHash) const { MurmurHash3_x86_32( ks.getTypeBits().getBuffer(), ks.getTypeBits().getSize(), indexNameHash, &indexNameHash); diff --git a/src/mongo/db/catalog/index_consistency.h b/src/mongo/db/catalog/index_consistency.h index 8ef836793b5..78556234b94 100644 --- a/src/mongo/db/catalog/index_consistency.h +++ b/src/mongo/db/catalog/index_consistency.h @@ -53,8 +53,6 @@ struct IndexInfo { const uint32_t indexNameHash; // More efficient representation of the ordering of the descriptor's key pattern. const Ordering ord; - // For conveniently building KeyStrings in a preallocated buffer. - std::unique_ptr<KeyString::Builder> ks; // The number of index entries belonging to the index. int64_t numKeys = 0; // The number of records that have a key in their document that referenced back to the this @@ -85,10 +83,9 @@ public: * inconsistent hash buckets during the first phase of validation. */ void addDocKey(OperationContext* opCtx, - const KeyString::Builder& ks, + const KeyString::Value& ks, IndexInfo* indexInfo, - RecordId recordId, - const BSONObj& indexKey); + RecordId recordId); /** * During the first phase of validation, given the index entry's KeyString, decrement the @@ -96,10 +93,7 @@ public: * For the second phase of validation, try to match the index entry keys that hashed to * inconsistent hash buckets during the first phase of validation to document keys. */ - void addIndexKey(const KeyString::Builder& ks, - IndexInfo* indexInfo, - RecordId recordId, - const BSONObj& indexKey); + void addIndexKey(const KeyString::Value& ks, IndexInfo* indexInfo, RecordId recordId); /** * To validate $** multikey metadata paths, we first scan the collection and add a hash of all @@ -107,8 +101,8 @@ public: * 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::Builder& ks, IndexInfo* indexInfo); - void removeMultikeyMetadataPath(const KeyString::Builder& ks, IndexInfo* indexInfo); + void addMultikeyMetadataPath(const KeyString::Value& ks, IndexInfo* indexInfo); + void removeMultikeyMetadataPath(const KeyString::Value& ks, IndexInfo* indexInfo); size_t getMultikeyMetadataPathCount(IndexInfo* indexInfo); /** @@ -195,7 +189,7 @@ private: /** * Returns a hashed value from the given KeyString and index namespace. */ - uint32_t _hashKeyString(const KeyString::Builder& ks, uint32_t indexNameHash) const; + uint32_t _hashKeyString(const KeyString::Value& ks, uint32_t indexNameHash) const; }; // IndexConsistency } // namespace mongo diff --git a/src/mongo/db/catalog/throttle_cursor.cpp b/src/mongo/db/catalog/throttle_cursor.cpp index 3f941b2f3b2..a3e0a3cecb1 100644 --- a/src/mongo/db/catalog/throttle_cursor.cpp +++ b/src/mongo/db/catalog/throttle_cursor.cpp @@ -87,6 +87,17 @@ boost::optional<IndexKeyEntry> SortedDataInterfaceThrottleCursor::seek( return entry; } +boost::optional<KeyStringEntry> SortedDataInterfaceThrottleCursor::seekForKeyString( + OperationContext* opCtx, const KeyString::Value& key) { + boost::optional<KeyStringEntry> entry = _cursor->seekForKeyString(key); + if (entry) { + const int64_t dataSize = entry->keyString.getSize() + sizeof(entry->loc.repr()); + _dataThrottle->awaitIfNeeded(opCtx, dataSize); + } + + return entry; +} + boost::optional<IndexKeyEntry> SortedDataInterfaceThrottleCursor::next(OperationContext* opCtx) { boost::optional<IndexKeyEntry> entry = _cursor->next(); if (entry) { @@ -97,6 +108,17 @@ boost::optional<IndexKeyEntry> SortedDataInterfaceThrottleCursor::next(Operation return entry; } +boost::optional<KeyStringEntry> SortedDataInterfaceThrottleCursor::nextKeyString( + OperationContext* opCtx) { + boost::optional<KeyStringEntry> entry = _cursor->nextKeyString(); + if (entry) { + const int64_t dataSize = entry->keyString.getSize() + sizeof(entry->loc.repr()); + _dataThrottle->awaitIfNeeded(opCtx, dataSize); + } + + return entry; +} + void DataThrottle::awaitIfNeeded(OperationContext* opCtx, const int64_t dataSize) { if (_shouldNotThrottle) { return; diff --git a/src/mongo/db/catalog/throttle_cursor.h b/src/mongo/db/catalog/throttle_cursor.h index 1495b4a0544..ba4875fe7ed 100644 --- a/src/mongo/db/catalog/throttle_cursor.h +++ b/src/mongo/db/catalog/throttle_cursor.h @@ -81,8 +81,11 @@ public: DataThrottle* dataThrottle); boost::optional<IndexKeyEntry> seek(OperationContext* opCtx, const KeyString::Value& key); + boost::optional<KeyStringEntry> seekForKeyString(OperationContext* opCtx, + const KeyString::Value& key); boost::optional<IndexKeyEntry> next(OperationContext* opCtx); + boost::optional<KeyStringEntry> nextKeyString(OperationContext* opCtx); void save() { _cursor->save(); diff --git a/src/mongo/db/catalog/validate_adaptor.cpp b/src/mongo/db/catalog/validate_adaptor.cpp index 9fd0f15ed34..7d43c3edc13 100644 --- a/src/mongo/db/catalog/validate_adaptor.cpp +++ b/src/mongo/db/catalog/validate_adaptor.cpp @@ -53,14 +53,6 @@ namespace { const long long kInterruptIntervalNumRecords = 4096; const long long kInterruptIntervalNumBytes = 50 * 1024 * 1024; // 50MB. -KeyString::Builder makeWildCardMultikeyMetadataKeyString(const BSONObj& indexKey) { - const auto multikeyMetadataOrd = Ordering::make(BSON("" << 1 << "" << 1)); - const RecordId multikeyMetadataRecordId(RecordId::ReservedId::kWildcardMultikeyMetadataId); - return {KeyString::Version::kLatestVersion, - indexKey, - multikeyMetadataOrd, - multikeyMetadataRecordId}; -} } // namespace Status ValidateAdaptor::validateRecord(OperationContext* opCtx, @@ -87,13 +79,9 @@ Status ValidateAdaptor::validateRecord(OperationContext* opCtx, return status; } - for (auto& it : _indexConsistency->getIndexInfo()) { - IndexInfo& indexInfo = it.second; - const IndexDescriptor* descriptor = indexCatalog->findIndexByName( - opCtx, indexInfo.indexName, /*includeUnfinishedIndexes=*/false); - invariant(descriptor); - - const IndexAccessMethod* iam = indexCatalog->getEntry(descriptor)->accessMethod(); + for (const auto& index : _validateState->getIndexes()) { + const IndexDescriptor* descriptor = index->descriptor(); + const IndexAccessMethod* iam = index->accessMethod(); if (descriptor->isPartial()) { const IndexCatalogEntry* ice = indexCatalog->getEntry(descriptor); @@ -125,14 +113,10 @@ Status ValidateAdaptor::validateRecord(OperationContext* opCtx, curRecordResults.valid = false; } + IndexInfo& indexInfo = _indexConsistency->getIndexInfo(descriptor->indexName()); for (const auto& keyString : multikeyMetadataKeys) { try { - auto key = KeyString::toBsonSafe(keyString.getBuffer(), - keyString.getSize(), - indexInfo.ord, - keyString.getTypeBits()); - _indexConsistency->addMultikeyMetadataPath( - makeWildCardMultikeyMetadataKeyString(key), &indexInfo); + _indexConsistency->addMultikeyMetadataPath(keyString, &indexInfo); } catch (...) { return exceptionToStatus(); } @@ -140,12 +124,7 @@ Status ValidateAdaptor::validateRecord(OperationContext* opCtx, for (const auto& keyString : documentKeySet) { try { - auto key = KeyString::toBsonSafe(keyString.getBuffer(), - keyString.getSize(), - indexInfo.ord, - keyString.getTypeBits()); - indexInfo.ks->resetToKey(key, indexInfo.ord, recordId); - _indexConsistency->addDocKey(opCtx, *indexInfo.ks, &indexInfo, recordId, key); + _indexConsistency->addDocKey(opCtx, keyString, &indexInfo, recordId); } catch (...) { return exceptionToStatus(); } @@ -155,34 +134,31 @@ Status ValidateAdaptor::validateRecord(OperationContext* opCtx, } void ValidateAdaptor::traverseIndex(OperationContext* opCtx, - const IndexDescriptor* descriptor, + const IndexCatalogEntry* index, int64_t* numTraversedKeys, ValidateResults* results) { + const IndexDescriptor* descriptor = index->descriptor(); auto indexName = descriptor->indexName(); - IndexInfo* indexInfo = &_indexConsistency->getIndexInfo(indexName); + IndexInfo& indexInfo = _indexConsistency->getIndexInfo(indexName); int64_t numKeys = 0; - const auto& key = descriptor->keyPattern(); - const Ordering ord = Ordering::make(key); bool isFirstEntry = true; - // We want to use the latest version of KeyString here. - const KeyString::Version version = KeyString::Version::kLatestVersion; - std::unique_ptr<KeyString::Builder> indexKeyStringBuilder = - std::make_unique<KeyString::Builder>(version); - std::unique_ptr<KeyString::Builder> prevIndexKeyStringBuilder = - std::make_unique<KeyString::Builder>(version); - + const KeyString::Version version = + index->accessMethod()->getSortedDataInterface()->getKeyStringVersion(); KeyString::Builder firstKeyString( - version, BSONObj(), ord, KeyString::Discriminator::kExclusiveBefore); + version, BSONObj(), indexInfo.ord, KeyString::Discriminator::kExclusiveBefore); + + KeyString::Value prevIndexKeyStringValue; // Ensure that this index has an open index cursor. const auto indexCursorIt = _validateState->getIndexCursors().find(indexName); invariant(indexCursorIt != _validateState->getIndexCursors().end()); const std::unique_ptr<SortedDataInterfaceThrottleCursor>& indexCursor = indexCursorIt->second; - for (auto indexEntry = indexCursor->seek(opCtx, firstKeyString.getValueCopy()); indexEntry; - indexEntry = indexCursor->next(opCtx)) { + for (auto indexEntry = indexCursor->seekForKeyString(opCtx, firstKeyString.getValueCopy()); + indexEntry; + indexEntry = indexCursor->nextKeyString(opCtx)) { if (numKeys % kInterruptIntervalNumRecords == 0) { opCtx->checkForInterrupt(); @@ -191,10 +167,9 @@ void ValidateAdaptor::traverseIndex(OperationContext* opCtx, _validateState->yieldLocks(opCtx); } } - indexKeyStringBuilder->resetToKey(indexEntry->key, ord, indexEntry->loc); // Ensure that the index entries are in increasing or decreasing order. - if (!isFirstEntry && *indexKeyStringBuilder < *prevIndexKeyStringBuilder) { + if (!isFirstEntry && indexEntry->keyString < prevIndexKeyStringValue) { if (results && results->valid) { results->errors.push_back( "one or more indexes are not in strictly ascending or descending order"); @@ -209,21 +184,18 @@ void ValidateAdaptor::traverseIndex(OperationContext* opCtx, RecordId::ReservedId::kWildcardMultikeyMetadataId}; if (descriptor->getIndexType() == IndexType::INDEX_WILDCARD && indexEntry->loc == kWildcardMultikeyMetadataRecordId) { - _indexConsistency->removeMultikeyMetadataPath( - makeWildCardMultikeyMetadataKeyString(indexEntry->key), indexInfo); + _indexConsistency->removeMultikeyMetadataPath(indexEntry->keyString, &indexInfo); numKeys++; continue; } - _indexConsistency->addIndexKey( - *indexKeyStringBuilder, indexInfo, indexEntry->loc, indexEntry->key); - + _indexConsistency->addIndexKey(indexEntry->keyString, &indexInfo, indexEntry->loc); numKeys++; isFirstEntry = false; - prevIndexKeyStringBuilder.swap(indexKeyStringBuilder); + prevIndexKeyStringValue = indexEntry->keyString; } - if (results && _indexConsistency->getMultikeyMetadataPathCount(indexInfo) > 0) { + if (results && _indexConsistency->getMultikeyMetadataPathCount(&indexInfo) > 0) { results->errors.push_back(str::stream() << "Index '" << descriptor->indexName() << "' has one or more missing multikey metadata index keys"); diff --git a/src/mongo/db/catalog/validate_adaptor.h b/src/mongo/db/catalog/validate_adaptor.h index 40285b31378..b68a6f37618 100644 --- a/src/mongo/db/catalog/validate_adaptor.h +++ b/src/mongo/db/catalog/validate_adaptor.h @@ -71,7 +71,7 @@ public: * for index consistency. */ void traverseIndex(OperationContext* opCtx, - const IndexDescriptor* descriptor, + const IndexCatalogEntry* index, int64_t* numTraversedKeys, ValidateResults* results); diff --git a/src/mongo/db/index/index_descriptor.h b/src/mongo/db/index/index_descriptor.h index 496ba0a2e58..3c3442fb004 100644 --- a/src/mongo/db/index/index_descriptor.h +++ b/src/mongo/db/index/index_descriptor.h @@ -133,14 +133,6 @@ public: return _projection; } - /** - * Test only command for testing behavior resulting from an incorrect key - * pattern. - */ - void setKeyPatternForTest(BSONObj newKeyPattern) { - _keyPattern = newKeyPattern; - } - // How many fields do we index / are in the key pattern? int getNumFields() const { return _numFields; diff --git a/src/mongo/db/storage/index_entry_comparison.h b/src/mongo/db/storage/index_entry_comparison.h index eefdbf1bb01..4fc93d21caf 100644 --- a/src/mongo/db/storage/index_entry_comparison.h +++ b/src/mongo/db/storage/index_entry_comparison.h @@ -89,8 +89,8 @@ struct KeyStringEntry { invariant(loc == KeyString::decodeRecordIdAtEnd(ks.getBuffer(), ks.getSize())); } - const KeyString::Value keyString; - const RecordId loc; + KeyString::Value keyString; + RecordId loc; }; /** diff --git a/src/mongo/dbtests/validate_tests.cpp b/src/mongo/dbtests/validate_tests.cpp index 5004dcc9444..8d5762da76f 100644 --- a/src/mongo/dbtests/validate_tests.cpp +++ b/src/mongo/dbtests/validate_tests.cpp @@ -911,65 +911,6 @@ public: }; template <bool full, bool background> -class ValidateIndexOrdering : public ValidateBase { -public: - ValidateIndexOrdering() : ValidateBase(full, background) {} - - void run() { - - // Cannot run validate with {background:true} if either - // - the RecordStore cursor does not retrieve documents in RecordId order - // - or the storage engine does not support checkpoints. - if (_background && (!_isInRecordIdOrder || !_engineSupportsCheckpoints)) { - return; - } - - // Create a new collection, insert three records and check it's valid. - lockDb(MODE_X); - OpDebug* const nullOpDebug = nullptr; - Collection* coll; - RecordId id1; - { - WriteUnitOfWork wunit(&_opCtx); - ASSERT_OK(_db->dropCollection(&_opCtx, _nss)); - coll = _db->createCollection(&_opCtx, _nss); - - ASSERT_OK(coll->insertDocument( - &_opCtx, InsertStatement(BSON("_id" << 1 << "a" << 1)), nullOpDebug, true)); - id1 = coll->getCursor(&_opCtx)->next()->id; - ASSERT_OK(coll->insertDocument( - &_opCtx, InsertStatement(BSON("_id" << 2 << "a" << 2)), nullOpDebug, true)); - ASSERT_OK(coll->insertDocument( - &_opCtx, InsertStatement(BSON("_id" << 3 << "b" << 1)), nullOpDebug, true)); - wunit.commit(); - } - - const std::string indexName = "bad_index"; - auto status = dbtests::createIndexFromSpec( - &_opCtx, - coll->ns().ns(), - BSON("name" << indexName << "key" << BSON("a" << 1) << "v" - << static_cast<int>(kIndexVersion) << "background" << false)); - - ASSERT_OK(status); - releaseDb(); - ensureValidateWorked(); - - lockDb(MODE_X); - - // Change the IndexDescriptor's keyPattern to descending so the index ordering - // appears wrong. - IndexCatalog* indexCatalog = coll->getIndexCatalog(); - IndexDescriptor* descriptor = - const_cast<IndexDescriptor*>(indexCatalog->findIndexByName(&_opCtx, indexName)); - descriptor->setKeyPatternForTest(BSON("a" << -1)); - - releaseDb(); - ensureValidateFailed(); - } -}; - -template <bool full, bool background> class ValidateWildCardIndex : public ValidateBase { public: ValidateWildCardIndex() : ValidateBase(full, background) {} @@ -1497,8 +1438,6 @@ public: // Tests for index validation. add<ValidateIndexEntry<false, false>>(); add<ValidateIndexEntry<false, true>>(); - add<ValidateIndexOrdering<false, false>>(); - add<ValidateIndexOrdering<false, true>>(); // Tests that the 'missingIndexEntries' and 'extraIndexEntries' field are populated // correctly. |