SERVER-108801 Invalid collMod command with timeseries granularity change creates collection options inconsistency (#42746)

Co-authored-by: Allison Easton <allison.easton@mongodb.com>
GitOrigin-RevId: cb0f3bf556c575a2274b9fc59d573ddcc87ade66

etc/backports_required_for_multiversion_tests.yml

@@ -67,6 +67,8 @@ last-continuous:
       ticket: SERVER-110953
     - test_file: jstests/sharding/migration_blocking_operation/implicit_create_from_upsert_with_paused_migrations.js
       ticket: SERVER-110574
+    - test_file: jstests/core/timeseries/ddl/timeseries_collmod_timeseries_options.js
+      ticket: SERVER-108801
     - test_file: jstests/replsets/log_unprepared_abort_txns.js
       ticket: SERVER-111017
     - test_file: jstests/core/timeseries/geo/timeseries_geonear_measurements.js
@@ -640,6 +642,8 @@ last-lts:
       ticket: SERVER-110953
     - test_file: jstests/sharding/migration_blocking_operation/implicit_create_from_upsert_with_paused_migrations.js
       ticket: SERVER-110574
+    - test_file: jstests/core/timeseries/ddl/timeseries_collmod_timeseries_options.js
+      ticket: SERVER-108801
     - test_file: jstests/replsets/log_unprepared_abort_txns.js
       ticket: SERVER-111017
     - test_file: jstests/core/timeseries/geo/timeseries_geonear_measurements.js

jstests/core/timeseries/ddl/timeseries_collmod_timeseries_options.js

@@ -0,0 +1,63 @@
+/**
+ * Test that timeseries collmod options can only be submitted without other collmod options.
+ *
+ * @tags: [
+ *   # collMod is not retryable
+ *   requires_non_retryable_commands,
+ *   # We need a timeseries collection.
+ *   requires_timeseries,
+ * ]
+ */
+
+const coll = db["coll"];
+const indexField = "a";
+const bucketRoundingSecondsHours = 60 * 60 * 24;
+
+function createTestColl() {
+    coll.drop();
+    assert.commandWorked(
+        db.createCollection(coll.getName(), {timeseries: {timeField: "time", granularity: "seconds"}}),
+    );
+
+    // This test cannot use the index with the key {'time': 1}, since that is the same as the implicitly
+    // created shard key and thus we cannot hide the index. We will use a different index here to avoid
+    // conflicts.
+    assert.commandWorked(coll.createIndex({[indexField]: 1}));
+}
+
+const timeseriesOptions = [
+    {"timeseries": {"granularity": "minutes"}},
+    {
+        "timeseries": {
+            "bucketMaxSpanSeconds": bucketRoundingSecondsHours,
+            "bucketRoundingSeconds": bucketRoundingSecondsHours,
+        },
+    },
+    {"timeseriesBucketsMayHaveMixedSchemaData": true},
+];
+const nonTimeseriesValidOptions = [
+    {"index": {"keyPattern": {[indexField]: 1}, "hidden": true}},
+    {"expireAfterSeconds": 60},
+];
+const nonTimeseriesInvalidOptions = [
+    {"index": {"keyPattern": {[indexField]: 1}, "expireAfterSeconds": 100}},
+    {"validator": {required: ["time"]}},
+    {"validationLevel": "moderate"},
+];
+
+// Test that valid options alone works
+for (const opt of [...timeseriesOptions, ...nonTimeseriesValidOptions]) {
+    createTestColl();
+    assert.commandWorked(db.runCommand({"collMod": coll.getName(), ...opt}));
+}
+
+createTestColl();
+// Test that valid timeseries options combined with other options always return InvalidOptions error
+for (const timeseriesOpt of timeseriesOptions) {
+    for (const nonTimeseriesOpt of [...nonTimeseriesInvalidOptions, ...nonTimeseriesValidOptions]) {
+        assert.commandFailedWithCode(
+            db.runCommand({"collMod": coll.getName(), ...timeseriesOpt, ...nonTimeseriesOpt}),
+            ErrorCodes.InvalidOptions,
+        );
+    }
+}

src/mongo/db/global_catalog/ddl/collmod_coordinator.cpp

@@ -90,11 +90,6 @@ MONGO_FAIL_POINT_DEFINE(collModBeforeConfigServerUpdate);
 namespace {
 
 
-bool hasTimeseriesParams(const CollModRequest& request) {
-    return (request.getTimeseries().has_value() && !request.getTimeseries()->toBSON().isEmpty()) ||
-        request.getTimeseriesBucketsMayHaveMixedSchemaData().has_value();
-}
-
 template <typename CommandType>
 std::vector<AsyncRequestsSender::Response> sendAuthenticatedCommandWithOsiToShards(
     OperationContext* opCtx,
@@ -352,7 +347,7 @@ ExecutorFuture<void> CollModCoordinator::_runImpl(
                 _saveShardingInfoOnCoordinatorIfNecessary(opCtx);
 
                 if (_collInfo->isTracked && _collInfo->timeSeriesOptions &&
-                    hasTimeseriesParams(_request)) {
+                    hasTimeseriesOptions(_request)) {
                     ShardsvrParticipantBlock blockCRUDOperationsRequest(_collInfo->nsForTargeting);
                     blockCRUDOperationsRequest.setBlockType(
                         CriticalSectionBlockTypeEnum::kReadsAndWrites);
@@ -376,7 +371,7 @@ ExecutorFuture<void> CollModCoordinator::_runImpl(
                 _saveShardingInfoOnCoordinatorIfNecessary(opCtx);
 
                 if (_collInfo->isTracked && _collInfo->timeSeriesOptions &&
-                    hasTimeseriesParams(_request)) {
+                    hasTimeseriesOptions(_request)) {
                     ConfigsvrCollMod request(_collInfo->nsForTargeting, _request);
                     generic_argument_util::setMajorityWriteConcern(request);
 
@@ -440,7 +435,7 @@ ExecutorFuture<void> CollModCoordinator::_runImpl(
 
                         ShardsvrCollModParticipant request(originalNss(), _request);
                         bool needsUnblock =
-                            _collInfo->timeSeriesOptions && hasTimeseriesParams(_request);
+                            _collInfo->timeSeriesOptions && hasTimeseriesOptions(_request);
                         request.setNeedsUnblock(needsUnblock);
 
                         std::vector<AsyncRequestsSender::Response> responses;

src/mongo/db/local_catalog/coll_mod.cpp

@@ -1092,6 +1092,11 @@ Status _collModInternal(OperationContext* opCtx,
 
 }  // namespace
 
+bool hasTimeseriesOptions(const CollModRequest& request) {
+    return (request.getTimeseries().has_value() && !request.getTimeseries()->toBSON().isEmpty()) ||
+        request.getTimeseriesBucketsMayHaveMixedSchemaData().has_value();
+}
+
 void staticValidateCollMod(OperationContext* opCtx,
                            const NamespaceString& nss,
                            const CollModRequest& request) {
@@ -1103,6 +1108,27 @@ void staticValidateCollMod(OperationContext* opCtx,
             "collMod on a time-series collection's underlying buckets collection is not "
             "supported.",
             !nss.isTimeseriesBucketsCollection());
+
+    if (hasTimeseriesOptions(request)) {
+        auto containsNotTimeseriesOptions = false;
+        for (const auto& field : request.toBSON()) {
+            if (field.fieldName() != CollModRequest::kTimeseriesFieldName &&
+                field.fieldName() !=
+                    CollModRequest::kTimeseriesBucketsMayHaveMixedSchemaDataFieldName) {
+                containsNotTimeseriesOptions = true;
+                break;
+            }
+        }
+
+        // Prevents catalog inconsistency (SERVER-108801) in sharded collMod.
+        // The timeseries option changes are applied to the global catalog before
+        // forwarding all options to shards. If subsequent options fail on a shard, the global
+        // and shard-local catalogs can become inconsistent.
+        uassert(ErrorCodes::InvalidOptions,
+                "Cannot combine timeseries option with other modification options in a single "
+                "collMod command. Send separate collMod commands for timeseries and other options.",
+                !containsNotTimeseriesOptions);
+    }
 }
 
 bool isCollModIndexUniqueConversion(const CollModRequest& request) {

src/mongo/db/local_catalog/coll_mod.h

@@ -77,6 +77,11 @@ Status processCollModCommand(OperationContext* opCtx,
                              CollectionAcquisition* acquisition,
                              BSONObjBuilder* result);
 
+/**
+ * Returns true if the given collmod @request contains options related to timeseries collections
+ */
+bool hasTimeseriesOptions(const CollModRequest& request);
+
 /**
  * Performs static validation of CollMod request.
  *