SERVER-72677 Surface index validation errors arising from WT::verify

7 files changed, 60 insertions, 24 deletions

diff --git a/jstests/disk/wt_validate_table_logging.js b/jstests/disk/wt_validate_table_logging.js
index 1dcebc31144..571491a3be2 100644
--- a/jstests/disk/wt_validate_table_logging.js
+++ b/jstests/disk/wt_validate_table_logging.js
@@ -55,8 +55,7 @@ const primary = replTest.getPrimary();
 // Run validate as a replica set, which will expect the tables to not be logged.
 let res = assert.commandWorked(primary.getDB(dbName).runCommand({validate: collName}));
 assert(!res.valid);
-// TODO (SERVER-72677): The validate results should report three errors.
-assert.eq(res.errors.length, 1);
+assert.eq(res.errors.length, 3);
 checkLog.containsJson(primary, 6898101, {uri: collUri(primary), expected: false});
 checkLog.containsJson(
     primary, 6898101, {index: '_id_', uri: indexUri(primary, '_id_'), expected: false});
@@ -75,8 +74,7 @@ conn = MongoRunner.runMongod(nodeOptions);
 // Run validate as a standalone, which will expect the tables to be logged.
 res = assert.commandWorked(conn.getDB(dbName).runCommand({validate: collName}));
 assert(!res.valid);
-// TODO (SERVER-72677): The validate results should report three errors.
-assert.eq(res.errors.length, 1);
+assert.eq(res.errors.length, 3);
 checkLog.containsJson(conn, 6898101, {uri: collUri(conn), expected: true});
 checkLog.containsJson(conn, 6898101, {index: '_id_', uri: indexUri(conn, '_id_'), expected: true});
 checkLog.containsJson(
diff --git a/jstests/noPassthrough/background_validation_checkpoint_existence.js b/jstests/noPassthrough/background_validation_checkpoint_existence.js
index 8b5a1cd8ea0..438278b9aea 100644
--- a/jstests/noPassthrough/background_validation_checkpoint_existence.js
+++ b/jstests/noPassthrough/background_validation_checkpoint_existence.js
@@ -32,30 +32,30 @@ for (let i = 0; i < 5; i++) {
 
 // The collection has not been checkpointed yet, so there is nothing to validate.
 let res = assert.commandWorked(db.runCommand({validate: collName, background: true}));
-assert.eq(true, res.valid);
-assert.eq(false, res.hasOwnProperty("nrecords"));
-assert.eq(false, res.hasOwnProperty("nIndexes"));
+assert.eq(true, res.valid, res);
+assert.eq(false, res.hasOwnProperty("nrecords"), res);
+assert.eq(false, res.hasOwnProperty("nIndexes"), res);
 
 forceCheckpoint();
 
 res = assert.commandWorked(db.runCommand({validate: collName, background: true}));
-assert.eq(true, res.valid);
-assert.eq(true, res.hasOwnProperty("nrecords"));
-assert.eq(true, res.hasOwnProperty("nIndexes"));
+assert.eq(true, res.valid, res);
+assert.eq(true, res.hasOwnProperty("nrecords"), res);
+assert.eq(true, res.hasOwnProperty("nIndexes"), res);
 
 assert.commandWorked(coll.createIndex({x: 1}));
 
 // Shouldn't validate the newly created index here as it wasn't checkpointed yet.
 res = assert.commandWorked(db.runCommand({validate: collName, background: true}));
-assert.eq(true, res.valid);
-assert.eq(1, res.nIndexes);
+assert.eq(true, res.valid, res);
+assert.eq(1, res.nIndexes, res);
 
 forceCheckpoint();
 
 // Validating after the checkpoint should validate the newly created index.
 res = assert.commandWorked(db.runCommand({validate: collName, background: true}));
-assert.eq(true, res.valid);
-assert.eq(2, res.nIndexes);
+assert.eq(true, res.valid, res);
+assert.eq(2, res.nIndexes, res);
 
 MongoRunner.stopMongod(conn);
 }());
\ No newline at end of file
diff --git a/src/mongo/db/catalog/collection_validation.cpp b/src/mongo/db/catalog/collection_validation.cpp
index 238b246ba84..952f1c75b54 100644
--- a/src/mongo/db/catalog/collection_validation.cpp
+++ b/src/mongo/db/catalog/collection_validation.cpp
@@ -258,19 +258,20 @@ void _reportValidationResults(OperationContext* opCtx,
 
     // Report detailed index validation results gathered when using {full: true} for validated
     // indexes.
-    for (const auto& index : validateState->getIndexes()) {
-        const std::string indexName = index->descriptor()->indexName();
-        auto& indexResultsMap = results->indexResultsMap;
-        if (indexResultsMap.find(indexName) == indexResultsMap.end()) {
-            continue;
-        }
-
-        auto& vr = indexResultsMap.at(indexName);
-
+    int nIndexes = results->indexResultsMap.size();
+    for (const auto& [indexName, vr] : results->indexResultsMap) {
         if (!vr.valid) {
             results->valid = false;
         }
 
+        if (validateState->getSkippedIndexes().contains(indexName)) {
+            // Index internal state was checked and cleared, so it was reported in indexResultsMap,
+            // but we did not verify the index contents against the collection, so we should exclude
+            // it from this report.
+            --nIndexes;
+            continue;
+        }
+
         BSONObjBuilder bob(indexDetails.subobjStart(indexName));
         bob.appendBool("valid", vr.valid);
 
@@ -289,7 +290,7 @@ void _reportValidationResults(OperationContext* opCtx,
         results->errors.insert(results->errors.end(), vr.errors.begin(), vr.errors.end());
     }
 
-    output->append("nIndexes", static_cast<int>(validateState->getIndexes().size()));
+    output->append("nIndexes", nIndexes);
     output->append("keysPerIndex", keysPerIndex.done());
     output->append("indexDetails", indexDetails.done());
 }
diff --git a/src/mongo/db/catalog/collection_validation_test.cpp b/src/mongo/db/catalog/collection_validation_test.cpp
index a8a91339ae6..b2284fdee82 100644
--- a/src/mongo/db/catalog/collection_validation_test.cpp
+++ b/src/mongo/db/catalog/collection_validation_test.cpp
@@ -673,6 +673,20 @@ TEST_F(CollectionValidationTest, ValidateEnforceFastCount) {
                        {CollectionValidation::ValidateMode::kForegroundFullEnforceFastCount});
 }
 
+TEST_F(CollectionValidationDiskTest, ValidateIndexDetailResultsSurfaceVerifyErrors) {
+    FailPointEnableBlock fp{"WTValidateIndexStructuralDamage"};
+    auto opCtx = operationContext();
+    insertDataRange(opCtx, 0, 5);  // initialize collection
+    foregroundValidate(
+        kNss,
+        opCtx,
+        /*valid*/ false,
+        /*numRecords*/ std::numeric_limits<int32_t>::min(),           // uninitialized
+        /*numInvalidDocuments*/ std::numeric_limits<int32_t>::min(),  // uninitialized
+        /*numErrors*/ 1,
+        {CollectionValidation::ValidateMode::kForegroundFull});
+}
+
 /**
  * Waits for a parallel running collection validation operation to start and then hang at a
  * failpoint.
diff --git a/src/mongo/db/catalog/validate_state.cpp b/src/mongo/db/catalog/validate_state.cpp
index 304f664005b..ba5f45a1838 100644
--- a/src/mongo/db/catalog/validate_state.cpp
+++ b/src/mongo/db/catalog/validate_state.cpp
@@ -286,6 +286,7 @@ void ValidateState::initializeCursors(OperationContext* opCtx) {
                   "checkpoint.",
                   "desc_indexName"_attr = desc->indexName(),
                   "nss"_attr = _nss);
+            _skippedIndexes.emplace(desc->indexName());
             continue;
         }
 
@@ -303,6 +304,7 @@ void ValidateState::initializeCursors(OperationContext* opCtx) {
                   "yet in a checkpoint.",
                   "desc_indexName"_attr = desc->indexName(),
                   "nss"_attr = _nss);
+            _skippedIndexes.emplace(desc->indexName());
             continue;
         }
 
@@ -323,6 +325,7 @@ void ValidateState::initializeCursors(OperationContext* opCtx) {
                     "consistent in the checkpoint.",
                     "desc_indexName"_attr = desc->indexName(),
                     "nss"_attr = _nss);
+                _skippedIndexes.emplace(desc->indexName());
                 continue;
             }
         } else {
@@ -331,6 +334,7 @@ void ValidateState::initializeCursors(OperationContext* opCtx) {
                 LOGV2(6325100,
                       "[Debugging] skipping index {index_name} because it isn't SortedData",
                       "index_name"_attr = desc->indexName());
+                _skippedIndexes.emplace(desc->indexName());
                 continue;
             }
 
@@ -349,6 +353,7 @@ void ValidateState::initializeCursors(OperationContext* opCtx) {
                     "consistent in the checkpoint.",
                     "desc_indexName"_attr = desc->indexName(),
                     "nss"_attr = _nss);
+                _skippedIndexes.emplace(desc->indexName());
                 continue;
             }
         }
diff --git a/src/mongo/db/catalog/validate_state.h b/src/mongo/db/catalog/validate_state.h
index a5902ca52ff..fa99e652567 100644
--- a/src/mongo/db/catalog/validate_state.h
+++ b/src/mongo/db/catalog/validate_state.h
@@ -145,6 +145,10 @@ public:
         return _indexes;
     }
 
+    const StringSet& getSkippedIndexes() const {
+        return _skippedIndexes;
+    }
+
     /**
      * Map of index names to index cursors.
      */
@@ -263,6 +267,10 @@ private:
     std::unique_ptr<SeekableRecordThrottleCursor> _seekRecordStoreCursor;
     StringMap<std::unique_ptr<ColumnStore::Cursor>> _columnStoreIndexCursors;
 
+    // Stores the set of indexes that will not be validated for some reason, e.g. they are not
+    // ready.
+    StringSet _skippedIndexes;
+
     RecordId _firstRecordId;
 
     DataThrottle _dataThrottle;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_index_util.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_index_util.cpp
index 76071a9ced7..77d4b3fd4a3 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_index_util.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_index_util.cpp
@@ -37,6 +37,7 @@
 namespace mongo {
 
 MONGO_FAIL_POINT_DEFINE(WTCompactIndexEBUSY);
+MONGO_FAIL_POINT_DEFINE(WTValidateIndexStructuralDamage);
 
 bool WiredTigerIndexUtil::appendCustomStats(OperationContext* opCtx,
                                             BSONObjBuilder* output,
@@ -125,6 +126,15 @@ void WiredTigerIndexUtil::validateStructure(OperationContext* opCtx,
         return;
     }
 
+    if (WTValidateIndexStructuralDamage.shouldFail()) {
+        std::string msg = str::stream() << "verify() returned an error. "
+                                        << "This indicates structural damage. "
+                                        << "Not examining individual index entries.";
+        results.errors.push_back(msg);
+        results.valid = false;
+        return;
+    }
+
     auto err = WiredTigerUtil::verifyTable(opCtx, uri, &results.errors);
     if (err == EBUSY) {
         std::string msg = str::stream()