Skip to content

CASSANDRA-19195 Added guardrail to enforce minimum CMS size#4950

Open
nvharikrishna wants to merge 2 commits into
apache:trunkfrom
nvharikrishna:19195-cms-guardrail-trunk
Open

CASSANDRA-19195 Added guardrail to enforce minimum CMS size#4950
nvharikrishna wants to merge 2 commits into
apache:trunkfrom
nvharikrishna:19195-cms-guardrail-trunk

Conversation

@nvharikrishna

Copy link
Copy Markdown
Contributor

Adds the minimum_cms_size_fail_threshold guardrail that fails an operator-requested CMS reconfiguration (nodetool cms reconfigure / JMX) when the resulting CMS size (aggregate replication factor across DCs) would fall below the threshold.

  • Config-driven, default -1 (disabled); valid values are -1 or >= 3
  • Enforced only on operator-initiated reconfiguration, not the automatic reconfiguration on topology changes
  • Skipped for clusters smaller than the threshold
  • Fail-only (no warning threshold)

patch by nvharikrishna; reviewed by <reviewer> for CASSANDRA-19195

The CASSANDRA-19195

private void guardMinimumCmsSize(ReplicationParams params)
{
ClusterMetadata metadata = ClusterMetadata.current();
int minCmsSize = DatabaseDescriptor.getGuardrailsConfig().getMinimumCmsSizeFailThreshold();

@smiklosovic smiklosovic Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting usage. I would use MinThreshold API here and called Guardrails.instance.minimumCmsSize.failValue|warnValue instead. We would then introduce CMSSizeThreshold guardrail class extending MinThreshold with overridden methods.

Or at least getting int minCmsSize from Guardrails.instance + appropriate getter if above is overkill.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! I looked into the CMSSizeThreshold extends MinThreshold approach. This validation needs two different values: cmsSize - the actual value being guarded, and totalNodes - a precondition deciding whether the check should apply at all (skipping the enforcement when the cluster is smaller than the required CMS size). totalNodes is a live value derived from ClusterMetadata.current(). The Threshold API has no way to pass it in (enabled()/guard() only take ClientState), so a CMSSizeThreshold subclass would have to reach into the ClusterMetadata.current() singleton from inside guard() just to get it. That would invert our package dependencies — db.guardrails currently imports nothing from tcm, and this would make a guardrail depend on cluster metadata. I feel there isn't much to reuse from the parent.

I opted for the second suggestion; it’s simpler. Now, both the threshold lookup and enforcement pass through Guardrails. I already have a getter for it and have used it here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nvharikrishna you could do it the same way as in ClientDriverVersionGuardrail which introduces a custom guard method accepting additional values so there you would pass values from CMS you would obtain outside of that guardrail. You would not need to depend on CMS inside guardrail.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nvharikrishna check this https://github.com/apache/cassandra/compare/trunk...smiklosovic:cassandra:CASSANDRA-19195-cms-guardrail-trunk?expand=1

What I like about that approach is that it encapsulates that logic and all guardrail's construction etc. It is just easier for an eye.

Adds the minimum_cms_size_fail_threshold guardrail that fails an operator-requested CMS reconfiguration (nodetool cms reconfigure / JMX) when the resulting CMS size (aggregate replication factor across DCs) would fall below the threshold.

- Config-driven, default -1 (disabled); valid values are -1 or >= 3
- Enforced only on operator-initiated reconfiguration, not the automatic reconfiguration on topology changes
- Skipped for clusters smaller than the threshold
- Fail-only (no warning threshold)

 patch by nvharikrishna; reviewed by <reviewer> for CASSANDRA-19195
@nvharikrishna
nvharikrishna force-pushed the 19195-cms-guardrail-trunk branch from 422e2c1 to e1e5c53 Compare July 22, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants