SERVER-42966 Remove references from helper classes for collection validation to simplify re-locking logic

5 files changed, 62 insertions, 86 deletions

diff --git a/src/mongo/db/catalog/collection_validation.cpp b/src/mongo/db/catalog/collection_validation.cpp
index a9e85c9b289..cd79f1c03d3 100644
--- a/src/mongo/db/catalog/collection_validation.cpp
+++ b/src/mongo/db/catalog/collection_validation.cpp
@@ -34,7 +34,9 @@
 #include "mongo/db/catalog/collection_validation.h"
 
 #include "mongo/db/catalog/collection.h"
+#include "mongo/db/catalog/index_consistency.h"
 #include "mongo/db/catalog/max_validate_mb_per_sec_gen.h"
+#include "mongo/db/index/index_access_method.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/db/storage/durable_catalog.h"
 #include "mongo/db/storage/record_store.h"
@@ -190,7 +192,7 @@ void _validateIndexes(
  */
 void _gatherIndexEntryErrors(
     OperationContext* opCtx,
-    IndexCatalog* indexCatalog,
+    Collection* coll,
     IndexConsistency* indexConsistency,
     ValidateAdaptor* indexValidator,
     const RecordId& firstRecordId,
@@ -212,7 +214,8 @@ void _gatherIndexEntryErrors(
         // We can ignore the status of validate as it was already checked during the first phase.
         size_t validatedSize;
         indexValidator
-            ->validateRecord(record->id, record->data, seekRecordStoreCursor, &validatedSize)
+            ->validateRecord(
+                opCtx, coll, record->id, record->data, seekRecordStoreCursor, &validatedSize)
             .ignore();
     }
 
@@ -221,7 +224,8 @@ void _gatherIndexEntryErrors(
 
     // Iterate through all the indexes in the collection and only record the index entry keys that
     // had inconsistencies during the first phase.
-    std::unique_ptr<IndexCatalog::IndexIterator> it = indexCatalog->getIndexIterator(opCtx, false);
+    std::unique_ptr<IndexCatalog::IndexIterator> it =
+        coll->getIndexCatalog()->getIndexIterator(opCtx, false);
     while (it->more()) {
         opCtx->checkForInterrupt();
 
@@ -246,20 +250,19 @@ void _gatherIndexEntryErrors(
 }
 
 void _validateIndexKeyCount(OperationContext* opCtx,
-                            IndexCatalog* indexCatalog,
-                            RecordStore* recordStore,
+                            Collection* coll,
                             ValidateAdaptor* indexValidator,
                             ValidateResultsMap* indexNsResultsMap) {
 
     const std::unique_ptr<IndexCatalog::IndexIterator> indexIterator =
-        indexCatalog->getIndexIterator(opCtx, false);
+        coll->getIndexCatalog()->getIndexIterator(opCtx, false);
     while (indexIterator->more()) {
         const IndexDescriptor* descriptor = indexIterator->next()->descriptor();
         ValidateResults& curIndexResults = (*indexNsResultsMap)[descriptor->indexName()];
 
         if (curIndexResults.valid) {
             indexValidator->validateIndexKeyCount(
-                descriptor, recordStore->numRecords(opCtx), curIndexResults);
+                descriptor, coll->getRecordStore()->numRecords(opCtx), curIndexResults);
         }
     }
 }
@@ -370,9 +373,8 @@ Status validate(OperationContext* opCtx,
 
     ValidateResultsMap indexNsResultsMap;
     BSONObjBuilder keysPerIndex;  // not using subObjStart to be exception safe.
-    IndexConsistency indexConsistency(opCtx, coll, coll->ns(), background);
-    ValidateAdaptor indexValidator = ValidateAdaptor(
-        opCtx, &indexConsistency, level, coll->getIndexCatalog(), &indexNsResultsMap);
+    IndexConsistency indexConsistency(opCtx, coll);
+    ValidateAdaptor indexValidator = ValidateAdaptor(&indexConsistency, level, &indexNsResultsMap);
 
     try {
         std::map<std::string, int64_t> numIndexKeysPerIndex;
@@ -414,7 +416,8 @@ Status validate(OperationContext* opCtx,
         // In traverseRecordStore(), the index validator keeps track the records in the record
         // store so that _validateIndexes() can confirm that the index entries match the records in
         // the collection.
-        indexValidator.traverseRecordStore(coll->getRecordStore(),
+        indexValidator.traverseRecordStore(opCtx,
+                                           coll,
                                            firstRecordId,
                                            traverseRecordStoreCursor,
                                            seekRecordStoreCursor,
@@ -442,7 +445,7 @@ Status validate(OperationContext* opCtx,
                     << "Index inconsistencies were detected on collection " << coll->ns()
                     << ". Starting the second phase of index validation to gather concise errors.";
                 _gatherIndexEntryErrors(opCtx,
-                                        coll->getIndexCatalog(),
+                                        coll,
                                         &indexConsistency,
                                         &indexValidator,
                                         firstRecordId,
@@ -456,11 +459,7 @@ Status validate(OperationContext* opCtx,
 
         // Validate index key count.
         if (results->valid) {
-            _validateIndexKeyCount(opCtx,
-                                   coll->getIndexCatalog(),
-                                   coll->getRecordStore(),
-                                   &indexValidator,
-                                   &indexNsResultsMap);
+            _validateIndexKeyCount(opCtx, coll, &indexValidator, &indexNsResultsMap);
         }
 
         // Report the validation results for the user to see.
diff --git a/src/mongo/db/catalog/index_consistency.cpp b/src/mongo/db/catalog/index_consistency.cpp
index 6148032137e..4a21ccc736e 100644
--- a/src/mongo/db/catalog/index_consistency.cpp
+++ b/src/mongo/db/catalog/index_consistency.cpp
@@ -32,58 +32,40 @@
 
 #include "mongo/platform/basic.h"
 
-#include "mongo/db/catalog/database_holder.h"
-#include "mongo/db/catalog/index_catalog.h"
 #include "mongo/db/catalog/index_consistency.h"
-#include "mongo/db/concurrency/d_concurrency.h"
+
+#include "mongo/db/catalog/index_catalog.h"
 #include "mongo/db/db_raii.h"
 #include "mongo/db/index/index_descriptor.h"
-#include "mongo/db/index_names.h"
 #include "mongo/db/storage/durable_catalog.h"
-#include "mongo/db/storage/key_string.h"
-#include "mongo/db/storage/record_store.h"
-#include "mongo/db/storage/sorted_data_interface.h"
-#include "mongo/util/elapsed_tracker.h"
 #include "mongo/util/string_map.h"
 
 namespace mongo {
 
 namespace {
 
-// The number of items we can scan before we must yield.
-static const int kScanLimit = 1000;
-static const size_t kNumHashBuckets = 1U << 16;
+const size_t kNumHashBuckets = 1U << 16;
 
 StringSet::hasher hash;
 
 }  // namespace
 
 IndexInfo::IndexInfo(const IndexDescriptor* descriptor)
-    : descriptor(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)) {}
 
-IndexConsistency::IndexConsistency(OperationContext* opCtx,
-                                   Collection* collection,
-                                   NamespaceString nss,
-                                   bool background)
-    : _opCtx(opCtx),
-      _collection(collection),
-      _nss(nss),
-      _tracker(opCtx->getServiceContext()->getFastClockSource(),
-               internalQueryExecYieldIterations.load(),
-               Milliseconds(internalQueryExecYieldPeriodMS.load())),
-      _firstPhase(true) {
+IndexConsistency::IndexConsistency(OperationContext* opCtx, Collection* coll) : _firstPhase(true) {
     _indexKeyCount.resize(kNumHashBuckets);
 
-    IndexCatalog* indexCatalog = _collection->getIndexCatalog();
     std::unique_ptr<IndexCatalog::IndexIterator> indexIterator =
-        indexCatalog->getIndexIterator(_opCtx, false);
+        coll->getIndexCatalog()->getIndexIterator(opCtx, false);
 
     while (indexIterator->more()) {
         const IndexDescriptor* descriptor = indexIterator->next()->descriptor();
-        if (DurableCatalog::get(opCtx)->isIndexReady(opCtx, nss, descriptor->indexName()))
+        if (DurableCatalog::get(opCtx)->isIndexReady(opCtx, coll->ns(), descriptor->indexName()))
             _indexesInfo.emplace(descriptor->indexName(), IndexInfo(descriptor));
     }
 }
@@ -280,8 +262,8 @@ BSONObj IndexConsistency::_generateInfo(const IndexInfo& indexInfo,
                                         RecordId recordId,
                                         const BSONObj& indexKey,
                                         boost::optional<BSONElement> idKey) {
-    const std::string& indexName = indexInfo.descriptor->indexName();
-    const BSONObj& keyPattern = indexInfo.descriptor->keyPattern();
+    const std::string& indexName = indexInfo.indexName;
+    const BSONObj& keyPattern = indexInfo.keyPattern;
 
     // We need to rehydrate the indexKey for improved readability.
     // {"": ObjectId(...)} -> {"_id": ObjectId(...)}
diff --git a/src/mongo/db/catalog/index_consistency.h b/src/mongo/db/catalog/index_consistency.h
index df03c3c129b..8eab32e3b3b 100644
--- a/src/mongo/db/catalog/index_consistency.h
+++ b/src/mongo/db/catalog/index_consistency.h
@@ -29,21 +29,23 @@
 
 #pragma once
 
-#include "mongo/db/concurrency/d_concurrency.h"
-#include "mongo/db/index/index_descriptor.h"
+#include "mongo/bson/simple_bsonobj_comparator.h"
 #include "mongo/db/storage/key_string.h"
 #include "mongo/db/storage/record_store.h"
-#include "mongo/db/storage/sorted_data_interface.h"
-#include "mongo/util/elapsed_tracker.h"
 
 namespace mongo {
 
+class IndexDescriptor;
+
 /**
  * Contains all the index information and stats throughout the validation.
  */
 struct IndexInfo {
     IndexInfo(const IndexDescriptor* descriptor);
-    const IndexDescriptor* const descriptor;
+    // Index name.
+    const std::string indexName;
+    // Contains the indexes key pattern.
+    const BSONObj keyPattern;
     // Contains the pre-computed hash of the index name.
     const uint32_t indexNameHash;
     // More efficient representation of the ordering of the descriptor's key pattern.
@@ -71,10 +73,7 @@ class IndexConsistency final {
     using ValidateResultsMap = std::map<std::string, ValidateResults>;
 
 public:
-    IndexConsistency(OperationContext* opCtx,
-                     Collection* collection,
-                     NamespaceString nss,
-                     bool background);
+    IndexConsistency(OperationContext* opCtx, Collection* coll);
 
     /**
      * During the first phase of validation, given the document's key KeyString, increment the
@@ -140,11 +139,6 @@ public:
 private:
     IndexConsistency() = delete;
 
-    OperationContext* _opCtx;
-    Collection* _collection;
-    const NamespaceString _nss;
-    ElapsedTracker _tracker;
-
     // We map the hashed KeyString values to a bucket that contains the count of how many
     // index keys and document keys we've seen in each bucket. This counter is unsigned to avoid
     // undefined behavior in the (unlikely) case of overflow.
diff --git a/src/mongo/db/catalog/validate_adaptor.cpp b/src/mongo/db/catalog/validate_adaptor.cpp
index f8dc22a48a4..c90828b7ade 100644
--- a/src/mongo/db/catalog/validate_adaptor.cpp
+++ b/src/mongo/db/catalog/validate_adaptor.cpp
@@ -34,6 +34,7 @@
 #include "mongo/db/catalog/validate_adaptor.h"
 
 #include "mongo/bson/bsonobj.h"
+#include "mongo/db/catalog/collection.h"
 #include "mongo/db/catalog/index_catalog.h"
 #include "mongo/db/catalog/index_consistency.h"
 #include "mongo/db/index/index_access_method.h"
@@ -42,7 +43,6 @@
 #include "mongo/db/matcher/expression.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/db/storage/key_string.h"
-#include "mongo/db/storage/record_store.h"
 #include "mongo/rpc/object_check.h"
 #include "mongo/util/log.h"
 
@@ -60,6 +60,8 @@ KeyString::Builder makeWildCardMultikeyMetadataKeyString(const BSONObj& indexKey
 }  // namespace
 
 Status ValidateAdaptor::validateRecord(
+    OperationContext* opCtx,
+    Collection* coll,
     const RecordId& recordId,
     const RecordData& record,
     const std::unique_ptr<SeekableRecordCursor>& seekRecordStoreCursor,
@@ -79,17 +81,21 @@ Status ValidateAdaptor::validateRecord(
         return status;
     }
 
-    if (!_indexCatalog->haveAnyIndexes()) {
+    IndexCatalog* indexCatalog = coll->getIndexCatalog();
+    if (!indexCatalog->haveAnyIndexes()) {
         return status;
     }
 
     for (auto& it : _indexConsistency->getIndexInfo()) {
         IndexInfo& indexInfo = it.second;
-        const IndexDescriptor* descriptor = indexInfo.descriptor;
-        const IndexAccessMethod* iam = _indexCatalog->getEntry(descriptor)->accessMethod();
+        const IndexDescriptor* descriptor = indexCatalog->findIndexByName(
+            opCtx, indexInfo.indexName, /*includeUnfinishedIndexes=*/false);
+        invariant(descriptor);
+
+        const IndexAccessMethod* iam = indexCatalog->getEntry(descriptor)->accessMethod();
 
         if (descriptor->isPartial()) {
-            const IndexCatalogEntry* ice = _indexCatalog->getEntry(descriptor);
+            const IndexCatalogEntry* ice = indexCatalog->getEntry(descriptor);
             if (!ice->getFilterExpression()->matchesBSON(recordBson)) {
                 continue;
             }
@@ -215,7 +221,8 @@ void ValidateAdaptor::traverseIndex(int64_t* numTraversedKeys,
 }
 
 void ValidateAdaptor::traverseRecordStore(
-    RecordStore* recordStore,
+    OperationContext* opCtx,
+    Collection* coll,
     const RecordId& firstRecordId,
     const std::unique_ptr<SeekableRecordCursor>& traverseRecordStoreCursor,
     const std::unique_ptr<SeekableRecordCursor>& seekRecordStoreCursor,
@@ -234,14 +241,14 @@ void ValidateAdaptor::traverseRecordStore(
         ++nrecords;
 
         if (!(nrecords % interruptInterval)) {
-            _opCtx->checkForInterrupt();
+            opCtx->checkForInterrupt();
         }
 
         auto dataSize = record->data.size();
         dataSizeTotal += dataSize;
         size_t validatedSize;
-        Status status =
-            validateRecord(record->id, record->data, seekRecordStoreCursor, &validatedSize);
+        Status status = validateRecord(
+            opCtx, coll, record->id, record->data, seekRecordStoreCursor, &validatedSize);
 
         // Checks to ensure isInRecordIdOrder() is being used properly.
         if (prevRecordId.isValid()) {
@@ -266,7 +273,7 @@ void ValidateAdaptor::traverseRecordStore(
     // Do not update the record store stats if we're in the background as we've validated a
     // checkpoint and it may not have the most up-to-date changes.
     if (results->valid && !background) {
-        recordStore->updateStatsAfterRepair(_opCtx, nrecords, dataSizeTotal);
+        coll->getRecordStore()->updateStatsAfterRepair(opCtx, nrecords, dataSizeTotal);
     }
 
     output->append("nInvalidDocuments", nInvalid);
diff --git a/src/mongo/db/catalog/validate_adaptor.h b/src/mongo/db/catalog/validate_adaptor.h
index 316c8ae42b1..cee9cd8c6c6 100644
--- a/src/mongo/db/catalog/validate_adaptor.h
+++ b/src/mongo/db/catalog/validate_adaptor.h
@@ -29,16 +29,15 @@
 
 #pragma once
 
-#include "mongo/db/catalog/index_catalog.h"
-#include "mongo/db/catalog/index_consistency.h"
-#include "mongo/db/index/index_access_method.h"
-#include "mongo/db/index/index_descriptor.h"
-#include "mongo/db/operation_context.h"
 #include "mongo/db/storage/record_store.h"
+#include "mongo/db/storage/sorted_data_interface.h"
+
 
 namespace mongo {
 
 class IndexConsistency;
+class IndexDescriptor;
+class OperationContext;
 
 enum ValidateCmdLevel : int { kValidateNormal = 0x01, kValidateFull = 0x02 };
 
@@ -54,30 +53,26 @@ using ValidateResultsMap = std::map<std::string, ValidateResults>;
  */
 class ValidateAdaptor {
 public:
-    ValidateAdaptor(OperationContext* opCtx,
-                    IndexConsistency* indexConsistency,
+    ValidateAdaptor(IndexConsistency* indexConsistency,
                     ValidateCmdLevel level,
-                    IndexCatalog* ic,
                     ValidateResultsMap* irm)
 
-        : _opCtx(opCtx),
-          _indexConsistency(indexConsistency),
-          _level(level),
-          _indexCatalog(ic),
-          _indexNsResultsMap(irm) {}
+        : _indexConsistency(indexConsistency), _level(level), _indexNsResultsMap(irm) {}
 
     /**
      * Validates the record data and traverses through its key set to keep track of the
      * index consistency.
      */
     virtual Status validateRecord(
+        OperationContext* opCtx,
+        Collection* coll,
         const RecordId& recordId,
         const RecordData& record,
         const std::unique_ptr<SeekableRecordCursor>& seekRecordStoreCursor,
         size_t* dataSize);
 
     /**
-     * Traverses the index getting index entriess to validate them and keep track of the index keys
+     * Traverses the index getting index entries to validate them and keep track of the index keys
      * for index consistency.
      */
     void traverseIndex(int64_t* numTraversedKeys,
@@ -89,7 +84,8 @@ public:
      * Traverses the record store to retrieve every record and go through its document key
      * set to keep track of the index consistency during a validation.
      */
-    void traverseRecordStore(RecordStore* recordStore,
+    void traverseRecordStore(OperationContext* opCtx,
+                             Collection* coll,
                              const RecordId& firstRecordId,
                              const std::unique_ptr<SeekableRecordCursor>& traverseRecordStoreCursor,
                              const std::unique_ptr<SeekableRecordCursor>& seekRecordStoreCursor,
@@ -105,10 +101,8 @@ public:
                                ValidateResults& results);
 
 private:
-    OperationContext* _opCtx;
     IndexConsistency* _indexConsistency;
     ValidateCmdLevel _level;
-    IndexCatalog* _indexCatalog;
     ValidateResultsMap* _indexNsResultsMap;
 };
 }  // namespace mongo