From 239160cda02c1b13d0b2afbb14467dae84e907ec Mon Sep 17 00:00:00 2001 From: Stefan Miklosovic Date: Thu, 23 Jul 2026 10:46:45 +0200 Subject: [PATCH] refactor: encapsulate CMS size guardrail in CMSSizeGuardrail Move the guardrail construction and the reconfigure skip logic into a dedicated CMSSizeGuardrail class, per review (aba3e2a944). Keeps the cmsSize >= totalNodes clamp rather than reintroducing the totalNodes < minCmsSize skip, so the guardrail is not silently bypassed when the configured threshold exceeds the current cluster size. The clamp needs no threshold value, so the guard() overload takes only (totalNodes, cmsSize) and CMSOperations no longer reads the threshold. --- .../db/guardrails/CMSSizeGuardrail.java | 59 +++++++++++++++++++ .../cassandra/db/guardrails/Guardrails.java | 12 +--- .../apache/cassandra/tcm/CMSOperations.java | 10 +--- 3 files changed, 61 insertions(+), 20 deletions(-) create mode 100644 src/java/org/apache/cassandra/db/guardrails/CMSSizeGuardrail.java diff --git a/src/java/org/apache/cassandra/db/guardrails/CMSSizeGuardrail.java b/src/java/org/apache/cassandra/db/guardrails/CMSSizeGuardrail.java new file mode 100644 index 0000000000..e1fb4c0b9a --- /dev/null +++ b/src/java/org/apache/cassandra/db/guardrails/CMSSizeGuardrail.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.guardrails; + +import java.util.function.ToLongFunction; + +import org.apache.cassandra.service.ClientState; + +import static java.lang.String.format; + +public class CMSSizeGuardrail extends MinThreshold +{ + /** + * Creates a new minimum threshold guardrail. + * + * @param failThreshold a {@link ClientState}-based provider of the value above which the operation should be aborted. + */ + public CMSSizeGuardrail(ToLongFunction failThreshold) + { + super("minimum_cms_size", + null, + state -> -1, + failThreshold, + (isWarning, what, value, threshold) -> + format("The CMS size of %s is below the failure threshold of %s. " + + "Reconfigure CMS so its total size (sum of replication factors across all datacenters) is at least %s.", + value, threshold, threshold)); + this.throwOnNullClientState = true; + } + + public void guard(int totalNodes, int cmsSize) + { + // A cluster can't host more CMS replicas than it has nodes. If the request already uses every node there's + // nothing more to enforce (e.g. during bootstrap), and requiring more would brick reconfiguration. Otherwise + // the request must meet the configured minimum, even when the threshold exceeds the current cluster size, so + // an operator can't sidestep the floor by setting a high threshold. An operator who genuinely wants a smaller + // CMS should disable the guardrail. When it is disabled, guard() below is a no-op. + if (cmsSize >= totalNodes) + return; + + guard(cmsSize, "CMS", false, null); + } +} diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index aa0018d827..cc44b5add8 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -618,17 +618,7 @@ public final class Guardrails implements GuardrailsMBean /** * Guardrail on the minimum CMS size (aggregate replication factor across DCs). */ - public static final MinThreshold minimumCmsSize = - (MinThreshold) new MinThreshold("minimum_cms_size", - null, - state -> -1, - state -> CONFIG_PROVIDER.getOrCreate(state).getMinimumCmsSizeFailThreshold(), - (isWarning, what, value, threshold) -> - format("The CMS size of %s is below the failure threshold of %s. " + - "Reconfigure CMS so its total size (sum of replication factors across all datacenters) is at least %s.", - value, threshold, threshold)) - // CMS reconfiguration is a system operation with no ClientState; make it abort rather than just log. - .throwOnNullClientState(true); + public static final CMSSizeGuardrail minimumCmsSize = new CMSSizeGuardrail(state -> CONFIG_PROVIDER.getOrCreate(state).getMinimumCmsSizeFailThreshold()); /** * Guardrail on the maximum replication factor. diff --git a/src/java/org/apache/cassandra/tcm/CMSOperations.java b/src/java/org/apache/cassandra/tcm/CMSOperations.java index 089ca93c7f..615f1e9034 100644 --- a/src/java/org/apache/cassandra/tcm/CMSOperations.java +++ b/src/java/org/apache/cassandra/tcm/CMSOperations.java @@ -188,15 +188,7 @@ public class CMSOperations implements CMSOperationsMBean int cmsSize = params.options.values().stream() .mapToInt(Integer::parseInt) .sum(); - // A cluster can't host more CMS replicas than it has nodes. If the request already uses every node there's - // nothing more to enforce (e.g. during bootstrap), and requiring more would brick reconfiguration. Otherwise - // the request must meet the configured minimum, even when the threshold exceeds the current cluster size, so - // an operator can't sidestep the floor by setting a high threshold. An operator who genuinely wants a smaller - // CMS should disable the guardrail. When it is disabled, guard() below is a no-op. - if (cmsSize >= totalNodes) - return; - - Guardrails.minimumCmsSize.guard(cmsSize, "CMS", false, null); + Guardrails.minimumCmsSize.guard(totalNodes, cmsSize); } @Override