SERVER-49340 Add repair mode to validate for startup --repair

11 files changed, 119 insertions, 29 deletions

diff --git a/src/mongo/db/catalog/collection_validation.cpp b/src/mongo/db/catalog/collection_validation.cpp
index 2adcaad50bf..78be0a16a85 100644
--- a/src/mongo/db/catalog/collection_validation.cpp
+++ b/src/mongo/db/catalog/collection_validation.cpp
@@ -423,14 +423,19 @@ void _validateCatalogEntry(OperationContext* opCtx,
 Status validate(OperationContext* opCtx,
                 const NamespaceString& nss,
                 ValidateMode mode,
+                RepairMode repairMode,
                 ValidateResults* results,
                 BSONObjBuilder* output,
                 bool turnOnExtraLoggingForTest) {
     invariant(!opCtx->lockState()->isLocked() || storageGlobalParams.repair);
 
+    if (repairMode == RepairMode::kRepair) {
+        invariant(storageGlobalParams.repair);
+    }
+
     // This is deliberately outside of the try-catch block, so that any errors thrown in the
     // constructor fail the cmd, as opposed to returning OK with valid:false.
-    ValidateState validateState(opCtx, nss, mode, turnOnExtraLoggingForTest);
+    ValidateState validateState(opCtx, nss, mode, repairMode, turnOnExtraLoggingForTest);
 
     const auto replCoord = repl::ReplicationCoordinator::get(opCtx);
     // Check whether we are allowed to read from this node after acquiring our locks. If we are
diff --git a/src/mongo/db/catalog/collection_validation.h b/src/mongo/db/catalog/collection_validation.h
index bd0e68a9dd3..22e7885f949 100644
--- a/src/mongo/db/catalog/collection_validation.h
+++ b/src/mongo/db/catalog/collection_validation.h
@@ -63,6 +63,17 @@ enum class ValidateMode {
 };
 
 /**
+ * RepairMode indicates whether validate should repair the data inconsistencies it detects. When set
+ * to kRepair, if any repairs are made, the 'repaired' flag in ValidateResults will be set to true.
+ * If all errors are fixed, then 'valid' will also be set to true. kRepair is incompatible with the
+ * ValidateModes kBackground and kForegroundFullEnforceFastCount.
+ */
+enum class RepairMode {
+    kNone,
+    kRepair,
+};
+
+/**
  * Expects the caller to hold no locks.
  *
  * Background validation does not support any type of full validation above.
@@ -76,6 +87,7 @@ enum class ValidateMode {
 Status validate(OperationContext* opCtx,
                 const NamespaceString& nss,
                 ValidateMode mode,
+                RepairMode repairMode,
                 ValidateResults* results,
                 BSONObjBuilder* output,
                 bool turnOnExtraLoggingForTest = false);
diff --git a/src/mongo/db/catalog/collection_validation_test.cpp b/src/mongo/db/catalog/collection_validation_test.cpp
index b3ed9f5185f..a9ac30cb35d 100644
--- a/src/mongo/db/catalog/collection_validation_test.cpp
+++ b/src/mongo/db/catalog/collection_validation_test.cpp
@@ -91,18 +91,21 @@ public:
  * Calls validate on collection kNss with both kValidateFull and kValidateNormal validation levels
  * and verifies the results.
  */
-void foregroundValidate(OperationContext* opCtx,
-                        bool valid,
-                        int numRecords,
-                        int numInvalidDocuments,
-                        int numErrors,
-                        std::initializer_list<CollectionValidation::ValidateMode> modes = {
-                            CollectionValidation::ValidateMode::kForeground,
-                            CollectionValidation::ValidateMode::kForegroundFull}) {
+void foregroundValidate(
+    OperationContext* opCtx,
+    bool valid,
+    int numRecords,
+    int numInvalidDocuments,
+    int numErrors,
+    std::initializer_list<CollectionValidation::ValidateMode> modes =
+        {CollectionValidation::ValidateMode::kForeground,
+         CollectionValidation::ValidateMode::kForegroundFull},
+    CollectionValidation::RepairMode repairMode = CollectionValidation::RepairMode::kNone) {
     for (auto mode : modes) {
         ValidateResults validateResults;
         BSONObjBuilder output;
-        ASSERT_OK(CollectionValidation::validate(opCtx, kNss, mode, &validateResults, &output));
+        ASSERT_OK(CollectionValidation::validate(
+            opCtx, kNss, mode, repairMode, &validateResults, &output));
         ASSERT_EQ(validateResults.valid, valid);
         ASSERT_EQ(validateResults.errors.size(), static_cast<long unsigned int>(numErrors));
 
@@ -134,8 +137,12 @@ void backgroundValidate(OperationContext* opCtx,
 
     ValidateResults validateResults;
     BSONObjBuilder output;
-    ASSERT_OK(CollectionValidation::validate(
-        opCtx, kNss, CollectionValidation::ValidateMode::kBackground, &validateResults, &output));
+    ASSERT_OK(CollectionValidation::validate(opCtx,
+                                             kNss,
+                                             CollectionValidation::ValidateMode::kBackground,
+                                             CollectionValidation::RepairMode::kNone,
+                                             &validateResults,
+                                             &output));
     BSONObj obj = output.obj();
 
     ASSERT_EQ(validateResults.valid, valid);
diff --git a/src/mongo/db/catalog/validate_state.cpp b/src/mongo/db/catalog/validate_state.cpp
index a85352f9c49..a449652e8c2 100644
--- a/src/mongo/db/catalog/validate_state.cpp
+++ b/src/mongo/db/catalog/validate_state.cpp
@@ -54,9 +54,11 @@ namespace CollectionValidation {
 ValidateState::ValidateState(OperationContext* opCtx,
                              const NamespaceString& nss,
                              ValidateMode mode,
+                             RepairMode repairMode,
                              bool turnOnExtraLoggingForTest)
     : _nss(nss),
       _mode(mode),
+      _repairMode(repairMode),
       _dataThrottle(opCtx),
       _extraLoggingForTest(turnOnExtraLoggingForTest) {
 
@@ -86,6 +88,13 @@ ValidateState::ValidateState(OperationContext* opCtx,
                   str::stream() << "Collection '" << _nss << "' does not exist to validate.");
     }
 
+    // RepairMode is incompatible with the ValidateModes kBackground and
+    // kForegroundFullEnforceFastCount.
+    if (shouldRunRepair()) {
+        invariant(!isBackground());
+        invariant(!shouldEnforceFastCount());
+    }
+
     _uuid = _collection->uuid();
     _catalogGeneration = opCtx->getServiceContext()->getCatalogGeneration();
 }
diff --git a/src/mongo/db/catalog/validate_state.h b/src/mongo/db/catalog/validate_state.h
index 92e2e4f0672..396f3393399 100644
--- a/src/mongo/db/catalog/validate_state.h
+++ b/src/mongo/db/catalog/validate_state.h
@@ -62,6 +62,7 @@ public:
     ValidateState(OperationContext* opCtx,
                   const NamespaceString& nss,
                   ValidateMode mode,
+                  RepairMode repairMode,
                   bool turnOnExtraLoggingForTest = false);
 
     const NamespaceString& nss() const {
@@ -85,6 +86,10 @@ public:
         return isFullValidation() || _mode == ValidateMode::kForegroundFullIndexOnly;
     }
 
+    bool shouldRunRepair() const {
+        return _repairMode == RepairMode::kRepair;
+    }
+
     const UUID uuid() const {
         invariant(_uuid);
         return *_uuid;
@@ -191,6 +196,7 @@ private:
 
     NamespaceString _nss;
     ValidateMode _mode;
+    RepairMode _repairMode;
     OptionalCollectionUUID _uuid;
 
     boost::optional<Lock::GlobalLock> _globalLock;
diff --git a/src/mongo/db/catalog/validate_state_test.cpp b/src/mongo/db/catalog/validate_state_test.cpp
index 5b4a99a4b86..9171284b4e5 100644
--- a/src/mongo/db/catalog/validate_state_test.cpp
+++ b/src/mongo/db/catalog/validate_state_test.cpp
@@ -116,15 +116,21 @@ void dropIndex(OperationContext* opCtx, const NamespaceString& nss, const std::s
 TEST_F(ValidateStateTest, NonExistentCollectionShouldThrowNamespaceNotFoundError) {
     auto opCtx = operationContext();
 
-    ASSERT_THROWS_CODE(CollectionValidation::ValidateState(
-                           opCtx, kNss, CollectionValidation::ValidateMode::kForeground),
-                       AssertionException,
-                       ErrorCodes::NamespaceNotFound);
-
-    ASSERT_THROWS_CODE(CollectionValidation::ValidateState(
-                           opCtx, kNss, CollectionValidation::ValidateMode::kBackground),
-                       AssertionException,
-                       ErrorCodes::NamespaceNotFound);
+    ASSERT_THROWS_CODE(
+        CollectionValidation::ValidateState(opCtx,
+                                            kNss,
+                                            CollectionValidation::ValidateMode::kForeground,
+                                            CollectionValidation::RepairMode::kNone),
+        AssertionException,
+        ErrorCodes::NamespaceNotFound);
+
+    ASSERT_THROWS_CODE(
+        CollectionValidation::ValidateState(opCtx,
+                                            kNss,
+                                            CollectionValidation::ValidateMode::kBackground,
+                                            CollectionValidation::RepairMode::kNone),
+        AssertionException,
+        ErrorCodes::NamespaceNotFound);
 }
 
 TEST_F(ValidateStateTest, UncheckpointedCollectionShouldBeAbleToInitializeCursors) {
@@ -138,7 +144,10 @@ TEST_F(ValidateStateTest, UncheckpointedCollectionShouldBeAbleToInitializeCursor
 
     createCollectionAndPopulateIt(opCtx, kNss);
     CollectionValidation::ValidateState validateState(
-        opCtx, kNss, CollectionValidation::ValidateMode::kBackground);
+        opCtx,
+        kNss,
+        CollectionValidation::ValidateMode::kBackground,
+        CollectionValidation::RepairMode::kNone);
     // Assert that cursors are able to created on the new collection.
     validateState.initializeCursors(opCtx);
     // There should only be a first record id if cursors were initialized successfully.
@@ -172,7 +181,10 @@ TEST_F(ValidateStateTest, OpenCursorsOnAllIndexes) {
     {
         // Open the cursors.
         CollectionValidation::ValidateState validateState(
-            opCtx, kNss, CollectionValidation::ValidateMode::kForeground);
+            opCtx,
+            kNss,
+            CollectionValidation::ValidateMode::kForeground,
+            CollectionValidation::RepairMode::kNone);
         validateState.initializeCursors(opCtx);
 
         // Make sure all of the indexes were found and cursors opened against them. Including the
@@ -187,7 +199,10 @@ TEST_F(ValidateStateTest, OpenCursorsOnAllIndexes) {
 
     // Check that foreground validation behaves just the same with checkpoint'ed data.
     CollectionValidation::ValidateState validateState(
-        opCtx, kNss, CollectionValidation::ValidateMode::kForeground);
+        opCtx,
+        kNss,
+        CollectionValidation::ValidateMode::kForeground,
+        CollectionValidation::RepairMode::kNone);
     validateState.initializeCursors(opCtx);
     ASSERT_EQ(validateState.getIndexes().size(), 5);
 }
@@ -222,7 +237,10 @@ TEST_F(ValidateStateTest, OpenCursorsOnAllIndexesWithBackground) {
 
     // Open the cursors.
     CollectionValidation::ValidateState validateState(
-        opCtx, kNss, CollectionValidation::ValidateMode::kBackground);
+        opCtx,
+        kNss,
+        CollectionValidation::ValidateMode::kBackground,
+        CollectionValidation::RepairMode::kNone);
     validateState.initializeCursors(opCtx);
 
     // We should be able to open a cursor on each index.
@@ -267,7 +285,10 @@ TEST_F(ValidateStateTest, CursorsAreNotOpenedAgainstCheckpointedIndexesThatWereL
     // (Note the _id index was create with collection creation, so we have 3 indexes.)
     {
         CollectionValidation::ValidateState validateState(
-            opCtx, kNss, CollectionValidation::ValidateMode::kBackground);
+            opCtx,
+            kNss,
+            CollectionValidation::ValidateMode::kBackground,
+            CollectionValidation::RepairMode::kNone);
         validateState.initializeCursors(opCtx);
         ASSERT_EQ(validateState.getIndexes().size(), 3);
     }
@@ -277,7 +298,10 @@ TEST_F(ValidateStateTest, CursorsAreNotOpenedAgainstCheckpointedIndexesThatWereL
     opCtx->recoveryUnit()->waitUntilUnjournaledWritesDurable(opCtx, /*stableCheckpoint*/ false);
 
     CollectionValidation::ValidateState validateState(
-        opCtx, kNss, CollectionValidation::ValidateMode::kBackground);
+        opCtx,
+        kNss,
+        CollectionValidation::ValidateMode::kBackground,
+        CollectionValidation::RepairMode::kNone);
     validateState.initializeCursors(opCtx);
     ASSERT_EQ(validateState.getIndexes().size(), 3);
 }
diff --git a/src/mongo/db/commands/validate.cpp b/src/mongo/db/commands/validate.cpp
index 318cbfa421b..ab46736a080 100644
--- a/src/mongo/db/commands/validate.cpp
+++ b/src/mongo/db/commands/validate.cpp
@@ -189,8 +189,13 @@ public:
                 return CollectionValidation::ValidateMode::kForegroundFull;
             return CollectionValidation::ValidateMode::kForeground;
         }();
+
+        // External users cannot run validate with repair as there is no way yet for users to invoke
+        // it. It is only to be used by startup repair.
+        auto repairMode = CollectionValidation::RepairMode::kNone;
         ValidateResults validateResults;
-        Status status = CollectionValidation::validate(opCtx, nss, mode, &validateResults, &result);
+        Status status =
+            CollectionValidation::validate(opCtx, nss, mode, repairMode, &validateResults, &result);
         if (!status.isOK()) {
             return CommandHelpers::appendCommandStatusNoThrow(result, status);
         }
diff --git a/src/mongo/db/repair_database.cpp b/src/mongo/db/repair_database.cpp
index 789eed1dc90..171f1efd2ad 100644
--- a/src/mongo/db/repair_database.cpp
+++ b/src/mongo/db/repair_database.cpp
@@ -168,6 +168,7 @@ Status repairCollections(OperationContext* opCtx,
             opCtx,
             nss,
             CollectionValidation::ValidateMode::kForegroundFullIndexOnly,
+            CollectionValidation::RepairMode::kRepair,
             &validateResults,
             &output);
         if (!status.isOK()) {
@@ -176,6 +177,17 @@ Status repairCollections(OperationContext* opCtx,
 
         LOGV2(21028, "Collection validation", "results"_attr = output.done());
 
+        if (validateResults.repaired) {
+            if (validateResults.valid) {
+                LOGV2(4934000, "Validate successfully repaired all data", "collection"_attr = nss);
+            } else {
+                LOGV2(4934001, "Validate was unable to repair all data", "collection"_attr = nss);
+            }
+        } else {
+            LOGV2(4934002, "Validate did not make any repairs", "collection"_attr = nss);
+        }
+
+        // If not valid, whether repair ran or not, indexes will need to be rebuilt.
         if (!validateResults.valid) {
             status = rebuildIndexesForNamespace(opCtx, nss, engine);
             if (!status.isOK()) {
diff --git a/src/mongo/db/repl/idempotency_test_fixture.cpp b/src/mongo/db/repl/idempotency_test_fixture.cpp
index 07aa9d39d3c..a8218696795 100644
--- a/src/mongo/db/repl/idempotency_test_fixture.cpp
+++ b/src/mongo/db/repl/idempotency_test_fixture.cpp
@@ -406,6 +406,7 @@ CollectionState IdempotencyTest::validate(const NamespaceString& nss) {
             CollectionValidation::validate(_opCtx.get(),
                                            nss,
                                            CollectionValidation::ValidateMode::kForegroundFull,
+                                           CollectionValidation::RepairMode::kNone,
                                            &validateResults,
                                            &bob));
         ASSERT_TRUE(validateResults.valid);
diff --git a/src/mongo/db/storage/record_store.h b/src/mongo/db/storage/record_store.h
index 692355b701f..fcd1b9843d3 100644
--- a/src/mongo/db/storage/record_store.h
+++ b/src/mongo/db/storage/record_store.h
@@ -570,6 +570,7 @@ protected:
 
 struct ValidateResults {
     bool valid = true;
+    bool repaired = false;
     boost::optional<Timestamp> readTimestamp = boost::none;
     std::vector<std::string> errors;
     std::vector<std::string> warnings;
diff --git a/src/mongo/dbtests/validate_tests.cpp b/src/mongo/dbtests/validate_tests.cpp
index 0fee3f366fb..99970f39139 100644
--- a/src/mongo/dbtests/validate_tests.cpp
+++ b/src/mongo/dbtests/validate_tests.cpp
@@ -102,11 +102,12 @@ protected:
             return _full ? CollectionValidation::ValidateMode::kForegroundFull
                          : CollectionValidation::ValidateMode::kForeground;
         }();
+        auto repairMode = CollectionValidation::RepairMode::kNone;
         ValidateResults results;
         BSONObjBuilder output;
 
         ASSERT_OK(CollectionValidation::validate(
-            &_opCtx, _nss, mode, &results, &output, kTurnOnExtraLoggingForTest));
+            &_opCtx, _nss, mode, repairMode, &results, &output, kTurnOnExtraLoggingForTest));
 
         //  Check if errors are reported if and only if valid is set to false.
         ASSERT_EQ(results.valid, results.errors.empty());
@@ -1203,6 +1204,7 @@ public:
                 CollectionValidation::validate(&_opCtx,
                                                _nss,
                                                CollectionValidation::ValidateMode::kForegroundFull,
+                                               CollectionValidation::RepairMode::kNone,
                                                &results,
                                                &output,
                                                kTurnOnExtraLoggingForTest));
@@ -1321,6 +1323,7 @@ public:
                 CollectionValidation::validate(&_opCtx,
                                                _nss,
                                                CollectionValidation::ValidateMode::kForegroundFull,
+                                               CollectionValidation::RepairMode::kNone,
                                                &results,
                                                &output,
                                                kTurnOnExtraLoggingForTest));
@@ -1412,6 +1415,7 @@ public:
                 CollectionValidation::validate(&_opCtx,
                                                _nss,
                                                CollectionValidation::ValidateMode::kForegroundFull,
+                                               CollectionValidation::RepairMode::kNone,
                                                &results,
                                                &output,
                                                kTurnOnExtraLoggingForTest));
@@ -1787,6 +1791,7 @@ public:
                 CollectionValidation::validate(&_opCtx,
                                                _nss,
                                                CollectionValidation::ValidateMode::kForeground,
+                                               CollectionValidation::RepairMode::kNone,
                                                &results,
                                                &output,
                                                kTurnOnExtraLoggingForTest));
@@ -1806,6 +1811,9 @@ public:
 
             dumpOnErrorGuard.dismiss();
         }
+
+        // TODO SERVER-49341: Call validate with repair true, expect corrupt BSON records are
+        // removed and results to be valid
     }
 };