SERVER-67597 Handle new time-series bucketing parameters on downgrade

2 files changed, 60 insertions, 49 deletions

diff --git a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
index d73102619d2..dc2e1ffb7c0 100644
--- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
+++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
@@ -637,7 +637,8 @@ private:
                         dbName,
                         MODE_S,
                         [&](const CollectionPtr& collection) {
-                            invariant(collection->getTimeseriesOptions());
+                            auto tsOptions = collection->getTimeseriesOptions();
+                            invariant(tsOptions);
 
                             auto indexCatalog = collection->getIndexCatalog();
                             auto indexIt = indexCatalog->getIndexIterator(
@@ -645,10 +646,10 @@ private:
                                 IndexCatalog::InclusionPolicy::kReady |
                                     IndexCatalog::InclusionPolicy::kUnfinished);
 
+                            // Check and fail to downgrade if the time-series collection has a
+                            // partial, TTL index.
                             while (indexIt->more()) {
                                 auto indexEntry = indexIt->next();
-                                // Fail to downgrade if the time-series collection has a partial,
-                                // TTL index.
                                 if (indexEntry->descriptor()->isPartial()) {
                                     // TODO (SERVER-67659): Remove partial, TTL index check once
                                     // FCV 7.0 becomes last-lts.
@@ -668,6 +669,20 @@ private:
                                             IndexDescriptor::kExpireAfterSecondsFieldName));
                                 }
                             }
+
+                            // Check the time-series options for a default granularity. Fail the
+                            // downgrade if the bucketing parameters are custom values.
+                            uassert(
+                                ErrorCodes::CannotDowngrade,
+                                str::stream()
+                                    << "Cannot downgrade the cluster when there are time-series "
+                                       "collections with custom bucketing parameters. In order to "
+                                       "downgrade, the time-series collection(s) must be updated "
+                                       "with a granularity of 'seconds', 'minutes' or 'hours'. "
+                                       "First detected incompatible collection: '"
+                                    << collection->ns().getTimeseriesViewNamespace() << "'",
+                                tsOptions->getGranularity().has_value());
+
                             return true;
                         },
                         [&](const CollectionPtr& collection) {
diff --git a/src/mongo/db/timeseries/timeseries_options.cpp b/src/mongo/db/timeseries/timeseries_options.cpp
index ba296b6ecc1..3b02088e459 100644
--- a/src/mongo/db/timeseries/timeseries_options.cpp
+++ b/src/mongo/db/timeseries/timeseries_options.cpp
@@ -128,6 +128,8 @@ Status isTimeseriesGranularityValidAndUnchanged(const TimeseriesOptions& current
         return Status{ErrorCodes::InvalidOptions, "No timeseries parameters were given"};
     }
 
+    // Check if we are trying to set the bucketing parameters to the existing values. If so, we do
+    // not need to update any existing options.
     if (currentOptions.getBucketMaxSpanSeconds() == targetMaxSpanSeconds) {
         if (!allowSecondsParameters ||
             currentOptions.getBucketRoundingSeconds() == targetRoundingSeconds) {
@@ -135,6 +137,14 @@ Status isTimeseriesGranularityValidAndUnchanged(const TimeseriesOptions& current
                 *shouldUpdateOptions = false;
             return Status::OK();
         }
+
+        if (allowSecondsParameters && currentGranularity &&
+            (targetRoundingSeconds ==
+             timeseries::getBucketRoundingSecondsFromGranularity(*currentGranularity))) {
+            if (shouldUpdateOptions)
+                *shouldUpdateOptions = false;
+            return Status::OK();
+        }
     }
 
     if (targetOptions.getBucketMaxSpanSeconds() != targetOptions.getBucketRoundingSeconds()) {
@@ -179,36 +189,29 @@ StatusWith<std::pair<TimeseriesOptions, bool>> applyTimeseriesOptionsModificatio
     TimeseriesOptions newOptions = currentOptions;
     bool shouldUpdateOptions = true;
     bool updated = false;
-    bool allowSecondsParameters = feature_flags::gTimeseriesScalabilityImprovements.isEnabled(
-        serverGlobalParams.featureCompatibility);
     auto targetGranularity = mod.getGranularity();
 
-    auto isValidTranistion =
+    auto isValidTransition =
         isTimeseriesGranularityValidAndUnchanged(currentOptions, mod, &shouldUpdateOptions);
-    if (!isValidTranistion.isOK()) {
-        return isValidTranistion;
+    if (!isValidTransition.isOK()) {
+        return isValidTransition;
     }
 
     if (shouldUpdateOptions) {
         int32_t targetMaxSpanSeconds = 0;
-        int32_t targetRoundingSeconds = 0;
+        boost::optional<int32_t> targetRoundingSeconds = boost::none;
 
         if (targetGranularity) {
             targetMaxSpanSeconds = getMaxSpanSecondsFromGranularity(targetGranularity.get());
-            targetRoundingSeconds =
-                getBucketRoundingSecondsFromGranularity(targetGranularity.get());
             newOptions.setGranularity(targetGranularity.get());
         } else {
             targetMaxSpanSeconds = mod.getBucketMaxSpanSeconds().get();
-            targetRoundingSeconds = mod.getBucketRoundingSeconds().get();
+            targetRoundingSeconds = mod.getBucketRoundingSeconds();
             newOptions.setGranularity(boost::none);
         }
 
         newOptions.setBucketMaxSpanSeconds(targetMaxSpanSeconds);
-
-        if (allowSecondsParameters)
-            newOptions.setBucketRoundingSeconds(targetRoundingSeconds);
-
+        newOptions.setBucketRoundingSeconds(targetRoundingSeconds);
         updated = true;
     }
 
@@ -321,34 +324,30 @@ Status validateAndSetBucketingParameters(TimeseriesOptions& timeseriesOptions) {
     auto roundingSecondsFromGranularity = getBucketRoundingSecondsFromGranularity(
         granularity.get_value_or(BucketGranularityEnum::Seconds));
 
-    if (allowSecondsParameters) {
-        if (granularity) {
-            if (maxSpanSeconds && maxSpanSeconds != maxSpanSecondsFromGranularity) {
-                return Status{
-                    ErrorCodes::InvalidOptions,
-                    fmt::format("Timeseries 'bucketMaxSpanSeconds' is not configurable to a value "
-                                "other than the default of {} for the provided granularity",
-                                maxSpanSecondsFromGranularity)};
-            }
-
-            if (roundingSeconds && roundingSeconds != roundingSecondsFromGranularity) {
-                return Status{
-                    ErrorCodes::InvalidOptions,
-                    fmt::format("Timeseries 'bucketRoundingSeconds' is not configurable to a value "
-                                "other than the default of {} for the provided granularity",
-                                roundingSecondsFromGranularity)};
-            }
-
-            if (!maxSpanSeconds)
-                timeseriesOptions.setBucketMaxSpanSeconds(maxSpanSecondsFromGranularity);
-
-            if (!roundingSeconds)
-                timeseriesOptions.setBucketRoundingSeconds(roundingSecondsFromGranularity);
+    if (granularity) {
+        if (maxSpanSeconds && maxSpanSeconds != maxSpanSecondsFromGranularity) {
+            return Status{
+                ErrorCodes::InvalidOptions,
+                fmt::format("Timeseries 'bucketMaxSpanSeconds' is not configurable to a value "
+                            "other than the default of {} for the provided granularity",
+                            maxSpanSecondsFromGranularity)};
+        }
 
-            return Status::OK();
+        if (roundingSeconds && roundingSeconds != roundingSecondsFromGranularity) {
+            return Status{
+                ErrorCodes::InvalidOptions,
+                fmt::format("Timeseries 'bucketRoundingSeconds' is not configurable to a value "
+                            "other than the default of {} for the provided granularity",
+                            roundingSecondsFromGranularity)};
         }
 
-        if (!maxSpanAndRoundingSecondsSpecified && (maxSpanSeconds || roundingSeconds)) {
+        if (!maxSpanSeconds)
+            timeseriesOptions.setBucketMaxSpanSeconds(maxSpanSecondsFromGranularity);
+
+        // If the granularity is specified, do not set the 'bucketRoundingSeconds' field.
+        timeseriesOptions.setBucketRoundingSeconds(boost::none);
+    } else if (allowSecondsParameters && (maxSpanSeconds || roundingSeconds)) {
+        if (!maxSpanAndRoundingSecondsSpecified) {
             return Status{
                 ErrorCodes::InvalidOptions,
                 "Timeseries 'bucketMaxSpanSeconds' and 'bucketRoundingSeconds' need to be "
@@ -360,12 +359,6 @@ Status validateAndSetBucketingParameters(TimeseriesOptions& timeseriesOptions) {
                           "Timeseries 'bucketRoundingSeconds' needs to be equal to "
                           "'bucketMaxSpanSeconds'"};
         }
-
-        if (!maxSpanSeconds) {
-            timeseriesOptions.setBucketMaxSpanSeconds(maxSpanSecondsFromGranularity);
-            timeseriesOptions.setBucketRoundingSeconds(roundingSecondsFromGranularity);
-            timeseriesOptions.setGranularity(BucketGranularityEnum::Seconds);
-        }
     } else {
         if (maxSpanSeconds && maxSpanSecondsFromGranularity != maxSpanSeconds) {
             return Status{
@@ -374,14 +367,17 @@ Status validateAndSetBucketingParameters(TimeseriesOptions& timeseriesOptions) {
                             "other than the default of {} for the provided granularity",
                             maxSpanSecondsFromGranularity)};
         }
-        if (!granularity)
+        if (!granularity) {
             timeseriesOptions.setGranularity(BucketGranularityEnum::Seconds);
+        }
 
-        if (!maxSpanSeconds)
+        if (!maxSpanSeconds) {
             timeseriesOptions.setBucketMaxSpanSeconds(maxSpanSecondsFromGranularity);
+        }
     }
 
     return Status::OK();
 }
+
 }  // namespace timeseries
 }  // namespace mongo