SERVER-82164 Use throttling in extraKeys dbcheck

moustafamaher · Evergreen Agent · commit 5e9f50eef1f8 · 2023-11-22T00:25:59.000Z
diff --git a/jstests/replsets/dbcheck.js b/jstests/replsets/dbcheck.js
@@ -302,29 +302,53 @@ function testDbCheckParameters() {
     }
     {
         // Validate maxDbCheckMBperSec.
-        clearHealthLog(replSet);
         const coll = db.getSiblingDB("maxDbCheckMBperSec").maxDbCheckMBperSec;
+        assert.commandWorked(db.getSiblingDB("maxDbCheckMBperSec").runCommand({
+            createIndexes: coll.getName(),
+            indexes: [{key: {a: 1}, name: 'a_1'}],
+        }));
 
         // Insert nDocs, each slightly larger than the maxDbCheckMBperSec value (1MB), which is the
         // default value, while maxBatchTimeMillis is defaulted to 1 second. Consequently, we will
         // have only 1MB per batch.
         const nDocs = 5;
-        coll.insertMany([...Array(nDocs).keys()].map(x => ({a: 'a'.repeat(1024 * 1024 * 2)})),
+        const chars = ['a', 'b', 'c', 'd', 'e'];
+        coll.insertMany([...Array(nDocs).keys()].map(x => ({a: chars[x].repeat(1024 * 1024 * 2)})),
                         {ordered: false});
+        [{}, {validateMode: "dataConsistency"}, {
+            validateMode: "dataConsistencyAndMissingIndexKeysCheck"
+        }].forEach(parameters => {
+            clearHealthLog(replSet);
+            runDbCheck(replSet, db.getSiblingDB("maxDbCheckMBperSec"), coll.getName(), parameters);
+
+            // DbCheck logs (nDocs + 1) batches to account for each batch hitting the time deadline
+            // after processing only one document. Then, DbCheck will run an additional empty batch
+            // at the end to confirm that there are no more documents.
+            let query = {"operation": "dbCheckBatch"};
+            checkHealthLog(healthlog, query, nDocs + 1);
+
+            query = {"operation": "dbCheckBatch", "data.count": 1};
+            checkHealthLog(healthlog, query, nDocs);
+
+            query = {"operation": "dbCheckBatch", "data.count": 0};
+            checkHealthLog(healthlog, query, 1);
+        });
 
-        runDbCheck(replSet, db.getSiblingDB("maxDbCheckMBperSec"), coll.getName(), {});
+        clearHealthLog(replSet);
+        runDbCheck(replSet, db.getSiblingDB("maxDbCheckMBperSec"), coll.getName(), {
+            validateMode: "extraIndexKeysCheck",
+            secondaryIndex: "a_1",
+        });
 
-        // DbCheck logs (nDocs + 1) batches to account for each batch hitting the time deadline
-        // after processing only one document. Then, DbCheck will run an additional empty batch at
-        // the end to confirm that there are no more documents.
+        // DbCheck logs (nDocs) batches to account for each batch hitting the time deadline after
+        // processing only one document.
+        // Extra index check's implementation is different from 'dataConsistency' as it doesn't need
+        // to run an additional empty batch at the end to confirm that there are no more documents.
         let query = {"operation": "dbCheckBatch"};
-        checkHealthLog(healthlog, query, nDocs + 1);
+        checkHealthLog(healthlog, query, nDocs);
 
         query = {"operation": "dbCheckBatch", "data.count": 1};
         checkHealthLog(healthlog, query, nDocs);
-
-        query = {"operation": "dbCheckBatch", "data.count": 0};
-        checkHealthLog(healthlog, query, 1);
     }
 }
 
diff --git a/src/mongo/db/catalog/throttle_cursor.h b/src/mongo/db/catalog/throttle_cursor.h
@@ -138,6 +138,10 @@ class SortedDataInterfaceThrottleCursor {
         return _cursor->isRecordIdAtEndOfKeyString();
     }
 
+    void setEndPosition(const BSONObj& key, bool inclusive) {
+        _cursor->setEndPosition(key, inclusive);
+    }
+
 private:
     std::unique_ptr<SortedDataInterface::Cursor> _cursor;
     DataThrottle* _dataThrottle;
diff --git a/src/mongo/db/commands/dbcheck_command.cpp b/src/mongo/db/commands/dbcheck_command.cpp
@@ -674,20 +674,9 @@ void DbChecker::_extraIndexKeysCheck(OperationContext* opCtx) {
 
     int64_t totalBytesSeen = 0;
     int64_t totalKeysSeen = 0;
-    using Clock = stdx::chrono::system_clock;
-    using TimePoint = stdx::chrono::time_point<Clock>;
-    TimePoint lastStart = Clock::now();
-    int64_t docsInCurrentInterval = 0;
-
     do {
-        using namespace std::literals::chrono_literals;
-
-        if (Clock::now() - lastStart > 1s) {
-            lastStart = Clock::now();
-            docsInCurrentInterval = 0;
-        }
-
         DbCheckExtraIndexKeysBatchStats batchStats = {0};
+        batchStats.deadline = Date_t::now() + Milliseconds(_info.maxBatchTimeMillis);
 
         // 1. Get batch bounds (stored in batchStats) and run reverse lookup if
         // skipLookupForExtraKeys is not set.
@@ -759,21 +748,11 @@ void DbChecker::_extraIndexKeysCheck(OperationContext* opCtx) {
         // 5. Check if we've exceeded any limits.
         _batchesProcessed++;
         totalBytesSeen += batchStats.nBytes;
-        totalKeysSeen += batchStats.nDocs;
-        docsInCurrentInterval += batchStats.nDocs;
+        totalKeysSeen += batchStats.nKeys;
 
-        bool tooManyDocs = totalKeysSeen >= _info.maxCount;
+        bool tooManyKeys = totalKeysSeen >= _info.maxCount;
         bool tooManyBytes = totalBytesSeen >= _info.maxSize;
-        reachedEnd = batchStats.finishedIndexCheck || tooManyDocs || tooManyBytes;
-
-        if (docsInCurrentInterval > _info.maxRate && _info.maxRate > 0) {
-            // If an extremely low max rate has been set (substantially smaller than the
-            // batch size) we might want to sleep for multiple seconds between batches.
-            int64_t timesExceeded = docsInCurrentInterval / _info.maxRate;
-
-            stdx::this_thread::sleep_for(timesExceeded * 1s - (Clock::now() - lastStart));
-        }
-
+        reachedEnd = batchStats.finishedIndexCheck || tooManyKeys || tooManyBytes;
     } while (!reachedEnd);
 
     // TODO SERVER-79846: Add testing for progress meter
@@ -882,9 +861,8 @@ Status DbChecker::_hashExtraIndexKeysCheck(OperationContext* opCtx,
         return e.toStatus();
     }
 
-    const auto batchDeadline = Date_t::now() + Milliseconds(_info.maxBatchTimeMillis);
-    Status status = hasher->hashForExtraIndexKeysCheck(
-        opCtx, collection.get(), batchFirst, batchLast, batchDeadline);
+    Status status =
+        hasher->hashForExtraIndexKeysCheck(opCtx, collection.get(), batchFirst, batchLast);
     if (!status.isOK()) {
         return status;
     }
@@ -1063,8 +1041,8 @@ Status DbChecker::_getCatalogSnapshotAndRunReverseLookup(
     const auto ordering = iam->getSortedDataInterface()->getOrdering();
 
 
-    std::unique_ptr<SortedDataInterface::Cursor> indexCursor =
-        iam->newCursor(opCtx, true /* forward */);
+    auto indexCursor =
+        std::make_unique<SortedDataInterfaceThrottleCursor>(opCtx, iam, &_info.dataThrottle);
 
 
     // Set the index cursor's end position based on the inputted end parameter for when to stop
@@ -1085,7 +1063,7 @@ Status DbChecker::_getCatalogSnapshotAndRunReverseLookup(
         logAttrs(_info.nss),
         "uuid"_attr = _info.uuid);
 
-    auto currIndexKey = indexCursor->seekForKeyString(lookupStart);
+    auto currIndexKey = indexCursor->seekForKeyString(opCtx, lookupStart);
 
     // Note that if we can't find lookupStart (e.g. it was deleted in between snapshots),
     // seekForKeyString will automatically return the next adjacent keystring in the storage
@@ -1148,9 +1126,9 @@ Status DbChecker::_getCatalogSnapshotAndRunReverseLookup(
         numBytes += keyString.getSize();
         numKeys++;
         batchStats.nBytes += keyString.getSize();
-        batchStats.nDocs++;
+        batchStats.nKeys++;
 
-        currIndexKey = indexCursor->nextKeyString();
+        currIndexKey = indexCursor->nextKeyString(opCtx);
 
         // Set nextLookupStart.
         if (currIndexKey) {
@@ -1162,8 +1140,7 @@ Status DbChecker::_getCatalogSnapshotAndRunReverseLookup(
         // snapshot/batch, so skip this check.
         if (!(currIndexKey && (keyString == currIndexKey.get().keyString))) {
             // Check if we should finish this batch.
-            if (batchStats.nBytes >= _info.maxBytesPerBatch ||
-                batchStats.nDocs >= _info.maxDocsPerBatch) {
+            if (batchStats.nKeys >= _info.maxDocsPerBatch) {
                 batchStats.finishedIndexBatch = true;
                 break;
             }
@@ -1172,6 +1149,11 @@ Status DbChecker::_getCatalogSnapshotAndRunReverseLookup(
                 break;
             }
         }
+
+        if (Date_t::now() > batchStats.deadline) {
+            batchStats.finishedIndexBatch = true;
+            break;
+        }
     }
 
 
@@ -1211,9 +1193,12 @@ void DbChecker::_reverseLookup(OperationContext* opCtx,
         }
         MONGO_UNREACHABLE;
     }();
-    RecordData record;
-    bool res = collection->getRecordStore()->findRecord(opCtx, recordId, &record);
-    if (!res) {
+
+    auto seekRecordStoreCursor = std::make_unique<SeekableRecordThrottleCursor>(
+        opCtx, collection->getRecordStore(), &_info.dataThrottle);
+
+    auto record = seekRecordStoreCursor->seekExact(opCtx, recordId);
+    if (!record) {
         LOGV2_DEBUG(7844802,
                     3,
                     "reverse lookup failed to find record data",
@@ -1247,7 +1232,7 @@ void DbChecker::_reverseLookup(OperationContext* opCtx,
     }
 
     // Found record in record store.
-    auto recordBson = record.toBson();
+    auto recordBson = record->data.toBson();
 
     // Generate the set of keys for the record data and check that it includes the
     // index key.
diff --git a/src/mongo/db/commands/dbcheck_command.h b/src/mongo/db/commands/dbcheck_command.h
@@ -79,7 +79,7 @@ struct DbCheckCollectionBatchStats {
  * For organizing the results of batches for extra index keys check.
  */
 struct DbCheckExtraIndexKeysBatchStats {
-    int64_t nDocs;
+    int64_t nKeys;
     int64_t nBytes;
     key_string::Value firstIndexKey;
     key_string::Value lastIndexKey;
@@ -89,6 +89,7 @@ struct DbCheckExtraIndexKeysBatchStats {
     std::string md5;
     repl::OpTime time;
     boost::optional<Timestamp> readTimestamp;
+    Date_t deadline;
 };
 
 /**
diff --git a/src/mongo/db/repl/dbcheck.cpp b/src/mongo/db/repl/dbcheck.cpp
@@ -429,8 +429,7 @@ void maybeAppend(md5_state_t* state, const boost::optional<UUID>& uuid) {
 Status DbCheckHasher::hashForExtraIndexKeysCheck(OperationContext* opCtx,
                                                  const Collection* collection,
                                                  const key_string::Value& first,
-                                                 const key_string::Value& last,
-                                                 Date_t deadline) {
+                                                 const key_string::Value& last) {
     // hashForExtraIndexKeysCheck must only be called if the hasher was created with indexName.
     invariant(_indexName);
     StringData indexName = _indexName.get();
@@ -442,17 +441,17 @@ Status DbCheckHasher::hashForExtraIndexKeysCheck(OperationContext* opCtx,
     auto iam = indexCatalogEntry->accessMethod()->asSortedData();
     const auto ordering = iam->getSortedDataInterface()->getOrdering();
 
-    std::unique_ptr<SortedDataInterface::Cursor> indexCursor =
-        iam->newCursor(opCtx, true /* forward */);
+    auto indexCursor =
+        std::make_unique<SortedDataInterfaceThrottleCursor>(opCtx, iam, _dataThrottle);
     auto firstBson =
         key_string::toBsonSafe(first.getBuffer(), first.getSize(), ordering, first.getTypeBits());
     auto lastBson =
         key_string::toBsonSafe(last.getBuffer(), last.getSize(), ordering, last.getTypeBits());
     indexCursor->setEndPosition(lastBson, true /*inclusive*/);
 
     // Iterate through index table.
-    for (auto currEntry = indexCursor->seekForKeyString(first); currEntry;
-         currEntry = indexCursor->nextKeyString()) {
+    for (auto currEntry = indexCursor->seekForKeyString(opCtx, first); currEntry;
+         currEntry = indexCursor->nextKeyString(opCtx)) {
         const auto keyString = currEntry->keyString;
         auto keyStringBson = key_string::toBsonSafe(
             keyString.getBuffer(), keyString.getSize(), ordering, keyString.getTypeBits());
diff --git a/src/mongo/db/repl/dbcheck.h b/src/mongo/db/repl/dbcheck.h
@@ -198,13 +198,12 @@ class DbCheckHasher {
                                   Date_t deadline = Date_t::max());
 
     /**
-     * Hash index keys up to the deadline.
+     * Hash index keys between first and last inclusive.
      */
     Status hashForExtraIndexKeysCheck(OperationContext* opCtx,
                                       const Collection* collection,
                                       const key_string::Value& first,
-                                      const key_string::Value& last,
-                                      Date_t deadline = Date_t::max());
+                                      const key_string::Value& last);
 
     /**
      * Checks if a document has missing index keys by finding the index keys that should be
