SERVER-36444 Skip validation for $** multikey metadata index entries

4 files changed, 15 insertions, 453 deletions

diff --git a/src/mongo/db/catalog/index_consistency.cpp b/src/mongo/db/catalog/index_consistency.cpp
index 474ebf8112e..211f10dd9a8 100644
--- a/src/mongo/db/catalog/index_consistency.cpp
+++ b/src/mongo/db/catalog/index_consistency.cpp
@@ -37,6 +37,7 @@
 #include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/db_raii.h"
 #include "mongo/db/index/index_descriptor.h"
+#include "mongo/db/index_names.h"
 #include "mongo/db/query/query_yield.h"
 #include "mongo/db/server_options.h"
 #include "mongo/db/storage/key_string.h"
@@ -67,7 +68,6 @@ IndexConsistency::IndexConsistency(OperationContext* opCtx,
       _nss(nss),
       _recordStore(recordStore),
       _collLk(std::move(collLk)),
-      _isBackground(background),
       _tracker(opCtx->getServiceContext()->getFastClockSource(),
                internalQueryExecYieldIterations.load(),
                Milliseconds(internalQueryExecYieldPeriodMS.load())) {
@@ -207,88 +207,6 @@ int64_t IndexConsistency::getNumExtraIndexKeys(int indexNumber) const {
     return _indexesInfo.at(indexNumber).numExtraIndexKeys;
 }
 
-void IndexConsistency::applyChange(const IndexDescriptor* descriptor,
-                                   const boost::optional<IndexKeyEntry>& indexEntry,
-                                   ValidationOperation operation) {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-
-    const std::string& indexNs = descriptor->indexNamespace();
-    int indexNumber = getIndexNumber(indexNs);
-    if (indexNumber == -1) {
-        return;
-    }
-
-    // Ignore indexes that weren't ready before we started validation.
-    if (!_indexesInfo.at(indexNumber).isReady) {
-        return;
-    }
-
-    const auto& key = descriptor->keyPattern();
-    const Ordering ord = Ordering::make(key);
-    KeyString::Version version = KeyString::kLatestVersion;
-
-    KeyString ks(version, indexEntry->key, ord, indexEntry->loc);
-
-    if (_stage == ValidationStage::DOCUMENT) {
-        _setYieldAtRecord_inlock(indexEntry->loc);
-        if (_isBeforeLastProcessedRecordId_inlock(indexEntry->loc)) {
-            if (operation == ValidationOperation::INSERT) {
-                if (largeKeyDisallowed() &&
-                    indexEntry->key.objsize() >=
-                        static_cast<int64_t>(KeyString::TypeBits::kMaxKeyBytes)) {
-                    // Index keys >= 1024 bytes are not indexed but are stored in the document key
-                    // set.
-                    _indexesInfo[indexNumber].numRecords++;
-                    _indexesInfo[indexNumber].numLongKeys++;
-                } else {
-                    _addDocKey_inlock(ks, indexNumber);
-                }
-            } else if (operation == ValidationOperation::REMOVE) {
-                if (largeKeyDisallowed() &&
-                    indexEntry->key.objsize() >=
-                        static_cast<int64_t>(KeyString::TypeBits::kMaxKeyBytes)) {
-                    _indexesInfo[indexNumber].numRecords--;
-                    _indexesInfo[indexNumber].numLongKeys--;
-                } else {
-                    _removeDocKey_inlock(ks, indexNumber);
-                }
-            }
-        }
-    } else if (_stage == ValidationStage::INDEX) {
-
-        // Index entries with key sizes >= 1024 bytes are not indexed.
-        if (largeKeyDisallowed() &&
-            indexEntry->key.objsize() >= static_cast<int64_t>(KeyString::TypeBits::kMaxKeyBytes)) {
-            return;
-        }
-
-        if (_isIndexScanning_inlock(indexNumber)) {
-            _setYieldAtIndexEntry_inlock(ks);
-        }
-
-        const bool wasIndexScanStarted =
-            _isIndexFinished_inlock(indexNumber) || _isIndexScanning_inlock(indexNumber);
-        const bool isUpcomingChangeToCurrentIndex =
-            _isIndexScanning_inlock(indexNumber) && !_isBeforeLastProcessedIndexEntry_inlock(ks);
-
-        if (!wasIndexScanStarted || isUpcomingChangeToCurrentIndex) {
-
-            // We haven't started scanning this index namespace yet so everything
-            // happens after the cursor, OR, we are scanning this index namespace,
-            // and an event occured after our cursor
-            if (operation == ValidationOperation::INSERT) {
-                _removeIndexKey_inlock(ks, indexNumber);
-                _indexesInfo.at(indexNumber).numExtraIndexKeys++;
-            } else if (operation == ValidationOperation::REMOVE) {
-                _addIndexKey_inlock(ks, indexNumber);
-                _indexesInfo.at(indexNumber).numExtraIndexKeys--;
-            }
-        }
-    }
-}
-
-
 void IndexConsistency::nextStage() {
 
     stdx::lock_guard<stdx::mutex> lock(_classMutex);
@@ -305,55 +223,6 @@ ValidationStage IndexConsistency::getStage() const {
     return _stage;
 }
 
-void IndexConsistency::setLastProcessedRecordId(RecordId recordId) {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-    if (!recordId.isValid()) {
-        _lastProcessedRecordId = boost::none;
-    } else {
-        _lastProcessedRecordId = recordId;
-    }
-}
-
-void IndexConsistency::setLastProcessedIndexEntry(
-    const IndexDescriptor& descriptor, const boost::optional<IndexKeyEntry>& indexEntry) {
-
-    const auto& key = descriptor.keyPattern();
-    const Ordering ord = Ordering::make(key);
-    KeyString::Version version = KeyString::kLatestVersion;
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-    if (!indexEntry) {
-        _lastProcessedIndexEntry.reset();
-    } else {
-        _lastProcessedIndexEntry.reset(
-            new KeyString(version, indexEntry->key, ord, indexEntry->loc));
-    }
-}
-
-void IndexConsistency::notifyStartIndex(int indexNumber) {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-    if (indexNumber < 0 || indexNumber >= static_cast<int>(_indexesInfo.size())) {
-        return;
-    }
-
-    _lastProcessedIndexEntry.reset(nullptr);
-    _currentIndex = indexNumber;
-}
-
-void IndexConsistency::notifyDoneIndex(int indexNumber) {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-    if (indexNumber < 0 || indexNumber >= static_cast<int>(_indexesInfo.size())) {
-        return;
-    }
-
-    _lastProcessedIndexEntry.reset(nullptr);
-    _currentIndex = -1;
-    _indexesInfo.at(indexNumber).indexScanFinished = true;
-}
-
 int IndexConsistency::getIndexNumber(const std::string& indexNs) {
 
     auto search = _indexNumber.find(indexNs);
@@ -364,47 +233,6 @@ int IndexConsistency::getIndexNumber(const std::string& indexNs) {
     return -1;
 }
 
-bool IndexConsistency::shouldGetNewSnapshot(const RecordId recordId) const {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-    if (!_yieldAtRecordId) {
-        return false;
-    }
-
-    return _yieldAtRecordId <= recordId;
-}
-
-bool IndexConsistency::shouldGetNewSnapshot(const KeyString& keyString) const {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-    if (!_yieldAtIndexEntry) {
-        return false;
-    }
-
-    return *_yieldAtIndexEntry <= keyString;
-}
-
-void IndexConsistency::relockCollectionWithMode(LockMode mode) {
-    // Release the lock and grab the provided lock mode.
-    _collLk.reset();
-    _collLk.reset(new Lock::CollectionLock(_opCtx->lockState(), _nss.toString(), mode));
-    invariant(_opCtx->lockState()->isCollectionLockedForMode(_nss.toString(), mode));
-
-    // Check if the operation was killed.
-    _opCtx->checkForInterrupt();
-
-    // Ensure it is safe to continue.
-    uassertStatusOK(_throwExceptionIfError());
-}
-
-bool IndexConsistency::scanLimitHit() {
-
-    stdx::lock_guard<stdx::mutex> lock(_classMutex);
-
-    // We have to yield every so many scans while doing background validation only.
-    return _isBackground && _tracker.intervalHasElapsed();
-}
-
 void IndexConsistency::_addDocKey_inlock(const KeyString& ks, int indexNumber) {
 
     // Ignore indexes that weren't ready before we started validation.
@@ -453,58 +281,6 @@ void IndexConsistency::_removeIndexKey_inlock(const KeyString& ks, int indexNumb
     _indexesInfo.at(indexNumber).numKeys--;
 }
 
-bool IndexConsistency::_isIndexFinished_inlock(int indexNumber) const {
-
-    return _indexesInfo.at(indexNumber).indexScanFinished;
-}
-
-bool IndexConsistency::_isIndexScanning_inlock(int indexNumber) const {
-
-    return indexNumber == _currentIndex;
-}
-
-void IndexConsistency::_setYieldAtRecord_inlock(const RecordId recordId) {
-
-    if (_isBeforeLastProcessedRecordId_inlock(recordId)) {
-        return;
-    }
-
-    if (!_yieldAtRecordId || recordId <= _yieldAtRecordId) {
-        _yieldAtRecordId = recordId;
-    }
-}
-
-void IndexConsistency::_setYieldAtIndexEntry_inlock(const KeyString& keyString) {
-
-    if (_isBeforeLastProcessedIndexEntry_inlock(keyString)) {
-        return;
-    }
-
-    if (!_yieldAtIndexEntry || keyString <= *_yieldAtIndexEntry) {
-        KeyString::Version version = KeyString::kLatestVersion;
-        _yieldAtIndexEntry.reset(new KeyString(version));
-        _yieldAtIndexEntry->resetFromBuffer(keyString.getBuffer(), keyString.getSize());
-    }
-}
-
-bool IndexConsistency::_isBeforeLastProcessedRecordId_inlock(RecordId recordId) const {
-
-    if (_lastProcessedRecordId && recordId <= _lastProcessedRecordId) {
-        return true;
-    }
-
-    return false;
-}
-
-bool IndexConsistency::_isBeforeLastProcessedIndexEntry_inlock(const KeyString& keyString) const {
-
-    if (_lastProcessedIndexEntry && keyString <= *_lastProcessedIndexEntry) {
-        return true;
-    }
-
-    return false;
-}
-
 uint32_t IndexConsistency::_hashKeyString(const KeyString& ks, int indexNumber) const {
 
     uint32_t indexNsHash = _indexesInfo.at(indexNumber).indexNsHash;
@@ -513,57 +289,4 @@ uint32_t IndexConsistency::_hashKeyString(const KeyString& ks, int indexNumber)
     MurmurHash3_x86_32(ks.getBuffer(), ks.getSize(), indexNsHash, &indexNsHash);
     return indexNsHash % (1U << 22);
 }
-
-Status IndexConsistency::_throwExceptionIfError() {
-
-    Database* database = DatabaseHolder::getDatabaseHolder().get(_opCtx, _nss.db());
-
-    // Ensure the database still exists.
-    if (!database) {
-        return Status(ErrorCodes::NamespaceNotFound,
-                      "The database was dropped during background validation");
-    }
-
-    Collection* collection = database->getCollection(_opCtx, _nss);
-
-    // Ensure the collection still exists.
-    if (!collection) {
-        return Status(ErrorCodes::NamespaceNotFound,
-                      "The collection was dropped during background validation");
-    }
-
-    // Ensure no indexes were removed or added.
-    IndexCatalog* indexCatalog = collection->getIndexCatalog();
-    IndexCatalog::IndexIterator indexIterator = indexCatalog->getIndexIterator(_opCtx, false);
-    int numRelevantIndexes = 0;
-
-    while (indexIterator.more()) {
-        const IndexDescriptor* descriptor = indexIterator.next();
-        int indexNumber = getIndexNumber(descriptor->indexNamespace());
-        if (indexNumber == -1) {
-            // Allow the collection scan to finish to verify that all the records are valid BSON.
-            if (_stage != ValidationStage::DOCUMENT) {
-                // An index was added.
-                return Status(ErrorCodes::IndexModified,
-                              "An index was added during background validation");
-            }
-        } else {
-            // Ignore indexes that weren't ready
-            if (_indexesInfo.at(indexNumber).isReady) {
-                numRelevantIndexes++;
-            }
-        }
-    }
-
-    if (numRelevantIndexes != static_cast<int>(_indexesInfo.size())) {
-        // Allow the collection scan to finish to verify that all the records are valid BSON.
-        if (_stage != ValidationStage::DOCUMENT) {
-            // An index was dropped.
-            return Status(ErrorCodes::IndexModified,
-                          "An index was dropped during background validation");
-        }
-    }
-
-    return Status::OK();
-}
 }  // namespace mongo
diff --git a/src/mongo/db/catalog/index_consistency.h b/src/mongo/db/catalog/index_consistency.h
index 753e12c5ece..9606096317a 100644
--- a/src/mongo/db/catalog/index_consistency.h
+++ b/src/mongo/db/catalog/index_consistency.h
@@ -137,39 +137,6 @@ public:
     int64_t getNumExtraIndexKeys(int indexNumber) const;
 
     /**
-     * This is the entry point for the IndexObserver to apply its observed changes
-     * while it is listening for changes in the IndexAccessMethod.
-     *
-     * This method ensures that during the collection scan stage, inserted, removed and
-     * updated documents are reflected in the index key counts.
-     * It does this by:
-     * 1) Setting the yield point for the collection scan to inform us when we should
-     *    get a new snapshot so we won't scan stale records.
-     * 2) Calling the appropriate `addDocKey` and `removeDocKey` functions if the
-     *    record comes before or equal to our last processed RecordId.
-     *
-     * The IndexObserver will call this method while it is observing changes during
-     * the index scan stage of the collection validation. It ensures we maintain
-     * a pre-image of the indexes since we established the point of validity, which
-     * was determined when the collection scan stage completed.
-     * It does this by:
-     * 1) Setting the yield point for the index scan to inform us when we should get
-     *    a new snapshot so we won't scan stale index entries. The difference between
-     *    this and the collection scan is that it will only set the yield point for the
-     *    index that is currently being scanned, since when we start the next index, we
-     *    will yield before we begin and we would have the latest snapshot.
-     * 2) Calling the appropriate `addIndexKey` and `removeIndexKey` functions for indexes
-     *    that haven't started scanning and are not finished, or they are scanning the
-     *    index and the index changes are after the last processed index entry.
-     * 3) In addition, we maintain the number of external index changes here so that
-     *    after we finish the index scan, we can remove the extra number of operations
-     *    that happened after the point of validity.
-     */
-    void applyChange(const IndexDescriptor* descriptor,
-                     const boost::optional<IndexKeyEntry>& indexEntry,
-                     ValidationOperation operation);
-
-    /**
      * Moves the `_stage` variable to the next corresponding stage in the following order:
      * `DOCUMENT` -> `INDEX`
      * `INDEX` -> `NONE`
@@ -183,69 +150,16 @@ public:
     ValidationStage getStage() const;
 
     /**
-     * Sets `_lastProcessedRecordId` to `recordId`.
-     */
-    void setLastProcessedRecordId(RecordId recordId);
-
-    /**
-     * Sets `_lastProcessedIndexEntry` to the KeyString of `indexEntry`.
-     */
-    void setLastProcessedIndexEntry(const IndexDescriptor& descriptor,
-                                    const boost::optional<IndexKeyEntry>& indexEntry);
-
-    /**
-     * Informs the IndexConsistency instance that the index scan is beginning to scan the index
-     * with namespace `indexNs`. This gives us a chance to clean up after the previous index and
-     * setup for the new index.
-     */
-    void notifyStartIndex(int indexNumber);
-
-    /**
-     * Informs the IndexConsistency instance that the index scan has finished scanning the index
-     * with namespace `indexNs`. This allows us to clean up just like in `notifyStartIndex` and to
-     * set the index to a finished state so that the hooks are prevented from affecting it.
-     */
-    void notifyDoneIndex(int indexNumber);
-
-    /**
      * Returns the index number for the corresponding index namespace's.
      */
     int getIndexNumber(const std::string& indexNs);
 
-    /**
-     * Returns true if a new snapshot should be accquired.
-     * If the `recordId` is equal to or greater than `_yieldAtRecordId` then we must get
-     * a new snapshot otherwise we will use stale data.
-     * Otherwise we do not need a new snapshot and can continue with the collection scan.
-     */
-    bool shouldGetNewSnapshot(const RecordId recordId) const;
-
-    /**
-     * Returns true if a new snapshot should be accquired.
-     * If the `keyString` is equal to or greater than `_yieldAtIndexEntry` then we must get
-     * a new snapshot otherwise we will use stale data.
-     * Otherwise we do not need a new snapshot and can continue with the index scan.
-     */
-    bool shouldGetNewSnapshot(const KeyString& keyString) const;
-
-    /**
-     * Gives up the lock that the collection is currently held in and requests the
-     * the collection again in LockMode `mode`
-     */
-    void relockCollectionWithMode(LockMode mode);
-
-    /**
-     * Returns true if the ElapsedTracker says its time to yield during background validation.
-     */
-    bool scanLimitHit();
-
 private:
     OperationContext* _opCtx;
     Collection* _collection;
     const NamespaceString _nss;
     const RecordStore* _recordStore;
     std::unique_ptr<Lock::CollectionLock> _collLk;
-    const bool _isBackground;
     ElapsedTracker _tracker;
 
     // We map the hashed KeyString values to a bucket which contain the count of how many
@@ -265,24 +179,12 @@ private:
     // A mapping of index numbers to IndexInfo
     std::map<int, IndexInfo> _indexesInfo;
 
-    // RecordId of the last processed document during the collection scan.
-    boost::optional<RecordId> _lastProcessedRecordId = boost::none;
-
-    // KeyString of the last processed index entry during the index scan.
-    std::unique_ptr<KeyString> _lastProcessedIndexEntry = nullptr;
-
     // The current index namespace being scanned in the index scan phase.
     int _currentIndex = -1;
 
     // The stage that the validation is currently on.
     ValidationStage _stage = ValidationStage::DOCUMENT;
 
-    // Contains the RecordId of when we should yield collection scan.
-    boost::optional<RecordId> _yieldAtRecordId = boost::none;
-
-    // Contains the KeyString of when we should yield during the index scan.
-    std::unique_ptr<KeyString> _yieldAtIndexEntry = nullptr;
-
     // Threshold for the number of errors to record before returning "There are too many errors".
     static const int _kErrorThreshold = 100;
 
@@ -317,58 +219,8 @@ private:
     void _removeIndexKey_inlock(const KeyString& ks, int indexNumber);
 
     /**
-     * Returns true if the index for the given `indexNs` has finished being scanned by
-     * the validation, otherwise it returns false.
-     */
-    bool _isIndexFinished_inlock(int indexNumber) const;
-
-    /**
-     * Returns true if this is the current `indexNs` being scanned
-     * by validation, otherwise it returns false.
-     */
-    bool _isIndexScanning_inlock(int indexNumber) const;
-
-    /**
-     * Allows the IndexObserver to set a yield point at `recordId` so that during the collection
-     * scan we must yield before processing the record. This is a preventive measure so the
-     * collection scan doesn't scan stale records.
-     */
-    void _setYieldAtRecord_inlock(const RecordId recordId);
-
-    /**
-     * Allows the IndexObserver to set a yield point at the KeyString of `indexEntry` so that
-     * during the index scan we must yield before processing the index entry.
-     * This is a preventive measure so the index scan doesn't scan stale index entries.
-     */
-    void _setYieldAtIndexEntry_inlock(const KeyString& keyString);
-
-    /**
-     * Returns true if the `recordId` is before or equal to the last processed
-     * RecordId.
-     */
-    bool _isBeforeLastProcessedRecordId_inlock(RecordId recordId) const;
-
-    /**
-     * Returns true if the `keyString` is before or equal to the last processed
-     * index entry.
-     */
-    bool _isBeforeLastProcessedIndexEntry_inlock(const KeyString& keyString) const;
-
-    /**
      * Returns a hashed value from the given KeyString and index namespace.
      */
     uint32_t _hashKeyString(const KeyString& ks, int indexNumbers) const;
-
-    /**
-     * Used alongside `yield()` and `relockCollectionWithMode()` to ensure that after the execution
-     * of them it is safe to continue validating.
-     * Validation can be stopped for a number of reasons including:
-     * 1) The database was dropped.
-     * 2) The collection was dropped.
-     * 3) An index was added or removed in the collection being validated.
-     * 4) The operation was killed.
-     */
-    Status _throwExceptionIfError();
-
 };  // IndexConsistency
 }  // namespace mongo
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 58d92ce2a49..405540b7c47 100644
--- a/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp
+++ b/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp
@@ -159,6 +159,15 @@ 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) {
+            continue;
+        }
+
         _indexConsistency->addIndexKey(*indexKeyString, indexNumber);
 
         numKeys++;
@@ -200,8 +209,8 @@ void RecordStoreValidateAdaptor::traverseRecordStore(RecordStore* recordStore,
             invariant(prevRecordId < record->id);
         }
 
-        // While some storage engines, such as MMAPv1, may use padding, we still require
-        // that they return the unpadded record data.
+        // While some storage engines may use padding, we still require that they return the
+        // unpadded record data.
         if (!status.isOK() || validatedSize != static_cast<size_t>(dataSize)) {
             if (results->valid) {
                 // Only log once.
diff --git a/src/mongo/db/commands/validate.cpp b/src/mongo/db/commands/validate.cpp
index a6ee82bd077..ab89306bdb6 100644
--- a/src/mongo/db/commands/validate.cpp
+++ b/src/mongo/db/commands/validate.cpp
@@ -133,31 +133,6 @@ public:
             uasserted(ErrorCodes::NamespaceNotFound, "ns not found");
         }
 
-        // Omit background validation logic until it is fully implemented and vetted.
-        const bool background = false;
-        /*
-        bool isInRecordIdOrder = collection->getRecordStore()->isInRecordIdOrder();
-        if (isInRecordIdOrder && !full) {
-            background = true;
-        }
-
-        if (cmdObj.hasElement("background")) {
-            background = cmdObj["background"].trueValue();
-        }
-
-        if (!isInRecordIdOrder && background) {
-            uasserted(ErrorCodes::CommandFailed,
-                      "This storage engine does not support the background option, use "
-                      "background:false");
-            return false;
-        }
-
-        if (full && background) {
-            uasserted(ErrorCodes::CommandFailed,
-                      "A full validate cannot run in the background, use full:false");
-        }
-        */
-
         result.append("ns", nss.ns());
 
         // Only one validation per collection can be in progress, the rest wait in order.
@@ -184,6 +159,9 @@ public:
             _validationNotifier.notify_all();
         });
 
+        // TODO SERVER-30357: Add support for background validation.
+        const bool background = false;
+
         ValidateResults results;
         Status status =
             collection->validate(opCtx, level, background, std::move(collLk), &results, &result);