SERVER-31988 RollbackViaRefetch makes CollectionImpl validator out of sync with CollectionCatalogEntry

(cherry picked from commit 4053104a4a0336304c8eab82d985941cd5e5e617)

7 files changed, 185 insertions, 2 deletions

diff --git a/jstests/replsets/rollback_collmods.js b/jstests/replsets/rollback_collmods.js
new file mode 100644
index 00000000000..e6db187d6b4
--- /dev/null
+++ b/jstests/replsets/rollback_collmods.js
@@ -0,0 +1,107 @@
+/**
+ * Tests that collMod commands during every stage of rollback are tracked correctly.
+ * This especially targets collection validators that begin partially or fully uninitialized.
+ */
+
+(function() {
+    "use strict";
+
+    load("jstests/replsets/libs/rollback_test.js");
+
+    const testName = "rollback_collmods";
+    const dbName = testName;
+
+    var coll1Name = "NoInitialValidationAtAll";
+    var coll2Name = "NoInitialValidationAction";
+    var coll3Name = "NoInitialValidator";
+    var coll4Name = "NoInitialValidationLevel";
+
+    function printCollectionOptionsForNode(node, time) {
+        let opts = assert.commandWorked(node.getDB(dbName).runCommand({"listCollections": 1}));
+        jsTestLog("Collection options " + time + " on " + node.host + ": " + tojson(opts));
+    }
+
+    function printCollectionOptions(rollbackTest, time) {
+        printCollectionOptionsForNode(rollbackTest.getPrimary(), time);
+        printCollectionOptionsForNode(rollbackTest.getSecondary(), time);
+    }
+
+    // Operations that will be present on both nodes, before the common point.
+    let CommonOps = (node) => {
+        let testDb = node.getDB(dbName);
+        assert.writeOK(testDb[coll1Name].insert({a: 1, b: 1}));
+        assert.writeOK(testDb[coll2Name].insert({a: 2, b: 2}));
+        assert.writeOK(testDb[coll3Name].insert({a: 3, b: 3}));
+        assert.writeOK(testDb[coll4Name].insert({a: 4, b: 4}));
+
+        // Start with no validation action.
+        assert.commandWorked(testDb.runCommand({
+            collMod: coll2Name,
+            validator: {a: 1},
+            validationLevel: "moderate",
+        }));
+
+        // Start with no validator.
+        assert.commandWorked(testDb.runCommand(
+            {collMod: coll3Name, validationLevel: "moderate", validationAction: "warn"}));
+
+        // Start with no validation level.
+        assert.commandWorked(
+            testDb.runCommand({collMod: coll4Name, validator: {a: 1}, validationAction: "warn"}));
+    };
+
+    // Operations that will be performed on the rollback node past the common point.
+    let RollbackOps = (node) => {
+        let testDb = node.getDB(dbName);
+
+        // Set everything on the rollback node.
+        assert.commandWorked(testDb.runCommand({
+            collMod: coll1Name,
+            validator: {a: 1},
+            validationLevel: "moderate",
+            validationAction: "warn"
+        }));
+
+        // Only modify the action, and never modify it again so it needs to be reset to empty.
+        assert.commandWorked(testDb.runCommand({collMod: coll2Name, validationAction: "error"}));
+
+        // Only modify the validator, and never modify it again so it needs to be reset to empty.
+        assert.commandWorked(testDb.runCommand({collMod: coll3Name, validator: {b: 1}}));
+
+        // Only modify the level, and never modify it again so it needs to be reset to empty.
+        assert.commandWorked(testDb.runCommand({
+            collMod: coll4Name,
+            validationLevel: "moderate",
+        }));
+    };
+
+    // Operations that will be performed on the sync source node after rollback.
+    let SteadyStateOps = (node) => {
+        let testDb = node.getDB(dbName);
+
+        assert.commandWorked(testDb.runCommand({collMod: coll2Name, validator: {b: 1}}));
+        assert.commandWorked(testDb.runCommand({collMod: coll3Name, validationAction: "error"}));
+        assert.commandWorked(testDb.runCommand({collMod: coll4Name, validationAction: "error"}));
+    };
+
+    // Set up Rollback Test.
+    let rollbackTest = new RollbackTest(testName);
+    CommonOps(rollbackTest.getPrimary());
+
+    let rollbackNode = rollbackTest.transitionToRollbackOperations();
+    printCollectionOptions(rollbackTest, "before branch");
+    RollbackOps(rollbackNode);
+
+    rollbackTest.transitionToSyncSourceOperations();
+    printCollectionOptions(rollbackTest, "before rollback");
+    // No ops on the sync source.
+
+    // Wait for rollback to finish.
+    rollbackTest.transitionToSteadyStateOperations({waitForRollback: true});
+    printCollectionOptions(rollbackTest, "after rollback");
+
+    SteadyStateOps(rollbackTest.getPrimary());
+    printCollectionOptions(rollbackTest, "at completion");
+
+    rollbackTest.stop();
+})();
diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h
index 5d30401e2b6..3e7eaa560c4 100644
--- a/src/mongo/db/catalog/collection.h
+++ b/src/mongo/db/catalog/collection.h
@@ -314,6 +314,11 @@ public:
         virtual StringData getValidationLevel() const = 0;
         virtual StringData getValidationAction() const = 0;
 
+        virtual Status updateValidator(OperationContext* opCtx,
+                                       BSONObj newValidator,
+                                       StringData newLevel,
+                                       StringData newAction) = 0;
+
         virtual bool isCapped() const = 0;
 
         virtual std::shared_ptr<CappedInsertNotifier> getCappedInsertNotifier() const = 0;
@@ -666,6 +671,13 @@ public:
         return this->_impl().getValidationAction();
     }
 
+    inline Status updateValidator(OperationContext* opCtx,
+                                  BSONObj newValidator,
+                                  StringData newLevel,
+                                  StringData newAction) {
+        return this->_impl().updateValidator(opCtx, newValidator, newLevel, newAction);
+    }
+
     // -----------
 
     //
diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp
index 5d5922c690f..f88d8502cc9 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -997,6 +997,37 @@ Status CollectionImpl::setValidationAction(OperationContext* opCtx, StringData n
     return Status::OK();
 }
 
+Status CollectionImpl::updateValidator(OperationContext* opCtx,
+                                       BSONObj newValidator,
+                                       StringData newLevel,
+                                       StringData newAction) {
+    invariant(opCtx->lockState()->isCollectionLockedForMode(ns().toString(), MODE_X));
+
+    _details->updateValidator(opCtx, newValidator, newLevel, newAction);
+    _validatorDoc = std::move(newValidator);
+
+    auto validatorSW =
+        parseValidator(opCtx, _validatorDoc, MatchExpressionParser::kAllowAllSpecialFeatures);
+    if (!validatorSW.isOK()) {
+        return validatorSW.getStatus();
+    }
+    _validator = std::move(validatorSW.getValue());
+
+    auto levelSW = parseValidationLevel(newLevel);
+    if (!levelSW.isOK()) {
+        return levelSW.getStatus();
+    }
+    _validationLevel = levelSW.getValue();
+
+    auto actionSW = parseValidationAction(newAction);
+    if (!actionSW.isOK()) {
+        return actionSW.getStatus();
+    }
+    _validationAction = actionSW.getValue();
+
+    return Status::OK();
+}
+
 const CollatorInterface* CollectionImpl::getDefaultCollator() const {
     return _collator.get();
 }
diff --git a/src/mongo/db/catalog/collection_impl.h b/src/mongo/db/catalog/collection_impl.h
index cba3445042e..8da45ee0b56 100644
--- a/src/mongo/db/catalog/collection_impl.h
+++ b/src/mongo/db/catalog/collection_impl.h
@@ -305,6 +305,16 @@ public:
     StringData getValidationLevel() const final;
     StringData getValidationAction() const final;
 
+    /**
+     * Sets the validator to exactly what's provided. If newLevel or newAction are empty, this
+     * sets them to the defaults. Any error Status returned by this function should be considered
+     * fatal.
+     */
+    Status updateValidator(OperationContext* opCtx,
+                           BSONObj newValidator,
+                           StringData newLevel,
+                           StringData newAction) final;
+
     // -----------
 
     //
diff --git a/src/mongo/db/catalog/collection_mock.h b/src/mongo/db/catalog/collection_mock.h
index 405a3fde968..e9ce55a3286 100644
--- a/src/mongo/db/catalog/collection_mock.h
+++ b/src/mongo/db/catalog/collection_mock.h
@@ -245,6 +245,13 @@ public:
         std::abort();
     }
 
+    Status updateValidator(OperationContext* opCtx,
+                           BSONObj newValidator,
+                           StringData newLevel,
+                           StringData newAction) {
+        std::abort();
+    }
+
     bool isCapped() const {
         std::abort();
     }
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index 79566d71cd3..5fdbe0ad1b9 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -1180,8 +1180,17 @@ void rollback_internal::syncFixUp(OperationContext* opCtx,
             // Set any document validation options. We update the validator fields without
             // parsing/validation, since we fetched the options object directly from the sync
             // source, and we should set our validation options to match it exactly.
-            cce->updateValidator(
+            auto validatorStatus = collection->updateValidator(
                 opCtx, options.validator, options.validationLevel, options.validationAction);
+            if (!validatorStatus.isOK()) {
+                throw RSFatalException(
+                    str::stream() << "Failed to update validator for " << nss.toString() << " ("
+                                  << uuid
+                                  << ") with "
+                                  << redact(info)
+                                  << ". Got: "
+                                  << validatorStatus.toString());
+            }
 
             wuow.commit();
 
diff --git a/src/mongo/db/repl/rs_rollback_no_uuid.cpp b/src/mongo/db/repl/rs_rollback_no_uuid.cpp
index b26a9908228..5e1409b2470 100644
--- a/src/mongo/db/repl/rs_rollback_no_uuid.cpp
+++ b/src/mongo/db/repl/rs_rollback_no_uuid.cpp
@@ -579,8 +579,15 @@ void syncFixUp(OperationContext* opCtx,
             // Set any document validation options. We update the validator fields without
             // parsing/validation, since we fetched the options object directly from the sync
             // source, and we should set our validation options to match it exactly.
-            cce->updateValidator(
+            auto validatorStatus = collection->updateValidator(
                 opCtx, options.validator, options.validationLevel, options.validationAction);
+            if (!validatorStatus.isOK()) {
+                throw RSFatalException(
+                    str::stream() << "Failed to update validator for " << nss.toString() << " with "
+                                  << redact(info)
+                                  << ". Got: "
+                                  << validatorStatus.toString());
+            }
 
             OptionalCollectionUUID originalLocalUUID = collection->uuid();
             OptionalCollectionUUID remoteUUID = boost::none;