You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "isaacreath (via GitHub)" <gi...@apache.org> on 2026/01/06 17:47:25 UTC
[PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
isaacreath opened a new pull request, #4547:
URL: https://github.com/apache/cassandra/pull/4547
This patch adds a new configuration, `data_disk_usage_stop_writes_for_keyspace_on_fail`, which ensures that if any node which replicates a given keyspace is full, all writes to that keyspace are blocked. The goal of this configuration is to provide simpler semantics to users of this guardrail, instead of having to deal with managing write failures to a subset of replicas in the cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683920936
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
aha yes, it is basically taking it from `local_data_disk_usage` and when `local_data_disk_usage` is disabled (by -1 thresholds) then it will effectively skip `replica_disk_usage` (will report it as NOT_AVAILABLE) or similar, interesting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2682426503
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
I do not think this is a good naming convention. We have _flags_ (ending on "enabled"), _thresholds, ending on _threshold_, and values, ending on "_ignored, _disallowed, _warned".
Your implementation does not adhere to anything like that.
If you do not want to have two levels - _warn and _fail and you want to have just "fail" one, then `data_disk_usage_stop_writes_on_any_full_node_enabled: false` seems to be better.
I do not understand `...for_keyspace_on_fail` - that is - there is no _keyspace_ we would set, right? Your guardrail is "keyspace agnostic", any keyspace would fill the node, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#issuecomment-3750173634
As said I do not see how
`data_disk_usage_failure_scope: <replica | keyspace>` would be congruent with the naming of other guardrails / predicates we have.
`data_disk_usage_keyspace_wide_protection_enabled` is more suitable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687697320
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -131,6 +206,80 @@ private boolean computeHasStuffedOrFullNode()
return false;
}
+ /**
+ * Update the set of full nodes by datacenter based on the disk usage state for the given endpoint.
+ * If the node is FULL, add it to the set for its datacenter. Otherwise, remove it from the set.
+ * This method is idempotent - adding an already-present node or removing an absent node has no effect.
+ *
+ * @param endpoint The endpoint whose state has changed.
+ * @param usageState The new disk usage state value.
+ */
+ private void computeUsageStateForEpDatacenter(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ Locator locator = DatabaseDescriptor.getLocator();
+ if (locator == null)
+ {
+ logger.debug("Unable to track disk usage by datacenter for endpoint {} because locator is null", endpoint);
+ return;
+ }
+
+ Location location = locator.location(endpoint);
+ if (location == null || location.equals(Location.UNKNOWN))
+ {
+ logger.debug("Unable to track disk usage by datacenter for endpoint {} because null or UNKNOWN location was returned",
+ endpoint);
+ return;
+ }
+
+ String datacenter = location.datacenter;
+
+ if (usageState.isFull())
+ {
+ // Add this node to the set of full nodes for its datacenter and remove if from the stuffed nodes
Review Comment:
... and remove it from ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687674996
##########
src/java/org/apache/cassandra/db/guardrails/Guardrails.java:
##########
@@ -556,6 +556,19 @@ public final class Guardrails implements GuardrailsMBean
(isWarning, value) ->
isWarning ? "Replica disk usage exceeds warning threshold"
: "Write request failed because disk usage exceeds failure threshold");
+ /**
+ * Guardrail on the data disk usage of replicas across a datacenter, used at write time to verify the status of
+ * any node which might replicate a given keyspace
+ */
+ public static final Predicates<InetAddressAndPort> keyspaceDiskUsage =
+ new Predicates<>("keyspace_disk_usage",
+ null,
+ state -> DiskUsageBroadcaster.instance::isDataCenterStuffed,
+ state -> DiskUsageBroadcaster.instance::isDataCenterFull,
+ (isWarning, value) ->
+ isWarning ? "Disk usage in datacenter exceeds warning threshold"
Review Comment:
could you align this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683912398
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
how does `replica_disk_usage` work? I do not see how it is configured in cassandra.yaml. It is just active all the time? On the other hand `local_data_disk_usage` is a _threshold_ (in config based on two thresholds). What approach are you suggesting we go with here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683972851
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
yeah try `keyspaceDiskUsage`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "asf-gitbox-commits (via GitHub)" <gi...@apache.org>.
asf-gitbox-commits closed pull request #4547: CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold.
URL: https://github.com/apache/cassandra/pull/4547
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#issuecomment-3738679642
@isaacreath you also need to update GuardrailsConfigCommandsTest
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683875106
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -164,10 +244,41 @@ public void onRestart(InetAddressAndPort endpoint, EndpointState state)
@Override
public void onRemove(InetAddressAndPort endpoint)
{
+ DiskUsageState usageState = usageInfo.getOrDefault(endpoint, DiskUsageState.NOT_AVAILABLE);
+ updateDiskUsageStateForDatacenterOnRemoval(endpoint, usageState);
usageInfo.remove(endpoint);
hasStuffedOrFullNode = usageInfo.values().stream().anyMatch(DiskUsageState::isStuffedOrFull);
}
+ private void updateDiskUsageStateForDatacenterOnRemoval(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ if (!Guardrails.getDiskUsageStopWritesForKeyspaceOnFail())
Review Comment:
Yes good point. Will add a test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683890122
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
@isaacreath I think that would be way better. I guess it might be also a _Predicates_ guardrail. I was thinking we are doing something similar to `replicaDiskUsage` in `Guardrails` which is a _Predicates_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687462007
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
Pushed up some initial changes. I still see some room for improvement in the `ModificaitonStatement` codepaths that I'd like to add.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683847940
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
Not sure about the semantics here just saying that if you do it like that then it will not be "flag" anymore (in guardrails context).
Actually, I do not see the actual implementation of that guardrail in `Guardrails` class. Roughly first half of that class is about guardrails declarations, you just put the setter and getter there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687712891
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# keyspace_disk_usage_enabeld: false
Review Comment:
enabeld -> enabled
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2682426503
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
I do not think this is a good naming convention. We have _flags_ (ending on "enabled"), _thresholds, ending on _threshold_, and values, ending on "_ignored, _disallowed, _warned".
Your implementation does not adhere to anything like that.
If you do not want to have two levels - _warn and _fail and you want to have just "fail" one, then `data_disk_usage_stop_writes_on_any_full_node_enabled: false` seems to be better.
I do not `...for_keyspace_on_fail` - that is - there is no _keyspace_ we would set, right? Your guardrail is "keyspace agnostic", any keyspace would fill the node, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "pauloricardomg (via GitHub)" <gi...@apache.org>.
pauloricardomg commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683875691
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
the problem with `datacenter` is that it might be confused with "writes will only be rejected if there's any full node in the same DC of the originating write" not "writes will be rejected if there's any full node across all dcs that replicate the keyspace"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683962722
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
Yeah that makes sense. So we can have 3 levels of guardrails, the existing `localDataDiskUsage` and `replicaDiskUsage` and the new one `keyspaceDiskUsage` (or `data_disk_usage_stop_writes_on_any_full_node_enabled` or whatever we like to call it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687710023
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -164,10 +313,36 @@ public void onRestart(InetAddressAndPort endpoint, EndpointState state)
@Override
public void onRemove(InetAddressAndPort endpoint)
{
+ DiskUsageState usageState = usageInfo.getOrDefault(endpoint, DiskUsageState.NOT_AVAILABLE);
+ updateDiskUsageStateForDatacenterOnRemoval(endpoint, usageState);
usageInfo.remove(endpoint);
hasStuffedOrFullNode = usageInfo.values().stream().anyMatch(DiskUsageState::isStuffedOrFull);
}
+ private void updateDiskUsageStateForDatacenterOnRemoval(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ Locator locator = DatabaseDescriptor.getLocator();
Review Comment:
there seems to be a lot of duplicity around this locator / location logic, copying it over from other places, maybe extracting that to a standalone method and having it like a one-liner here would be more readable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683972851
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
yeah try `keyspace_disk_usage` , so that will be `PercentageThreshold` with a proper warn / fail levels?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "pauloricardomg (via GitHub)" <gi...@apache.org>.
pauloricardomg commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683506205
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -67,11 +73,39 @@ public boolean hasStuffedOrFullNode()
return hasStuffedOrFullNode;
}
+ public boolean hasFullNodeInDataCenter(String datacenter)
+ {
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ return fullNodes != null && !fullNodes.isEmpty();
+ }
+
/**
- * @return {@code true} if given node's disk usage is FULL
+ * @return {@code true} if given node's disk usage is FULL or if the node is in a data center that contains
+ * a full node and the data_disk_usage_stop_writes_for_keyspace_on_fail config is enabled.
*/
public boolean isFull(InetAddressAndPort endpoint)
{
+ if (Guardrails.getDiskUsageStopWritesForKeyspaceOnFail())
+ {
+ if (!hasStuffedOrFullNode())
+ {
+ // If no node in the cluster is full or stuffed then we exit early.
+ return false;
+ }
+ Locator locator = DatabaseDescriptor.getLocator();
+ if (locator == null)
+ {
+ logger.debug("Unable to block writes for keyspace because we cannot determine the datacenter of {}.", endpoint);
Review Comment:
should probably use a nospam logger in case this is ever hit
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -164,10 +244,41 @@ public void onRestart(InetAddressAndPort endpoint, EndpointState state)
@Override
public void onRemove(InetAddressAndPort endpoint)
{
+ DiskUsageState usageState = usageInfo.getOrDefault(endpoint, DiskUsageState.NOT_AVAILABLE);
+ updateDiskUsageStateForDatacenterOnRemoval(endpoint, usageState);
usageInfo.remove(endpoint);
hasStuffedOrFullNode = usageInfo.values().stream().anyMatch(DiskUsageState::isStuffedOrFull);
}
+ private void updateDiskUsageStateForDatacenterOnRemoval(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ if (!Guardrails.getDiskUsageStopWritesForKeyspaceOnFail())
Review Comment:
why is this check done here but not on `computeUsageStateForEpDatacenter` ? What if the check is disabled, a full node is removed, and the check is re-enabled - won't this cause the full node to remain in the `fullNodesByDatacenter` map unnecessarily blocking writes ? If so can you add a test case for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683791278
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
Or perhaps:
```
# Fail writes to the datacenter with any node above fail threshold
data_disk_usage_failure_scope: datacenter
```
Not too much preference on either though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#issuecomment-3750326931
I'll go with `data_disk_usage_keyspace_wide_protection_enabled` then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2688148567
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -83,6 +89,74 @@ public boolean isStuffed(InetAddressAndPort endpoint)
return state(endpoint).isStuffed();
}
+ /**
+ * @return {@code true} if there exists any node in the datacenter of {@code endpoint} which has FULL disk usage.
+ */
+ public boolean isDataCenterFull(InetAddressAndPort endpoint)
+ {
+ if (!hasStuffedOrFullNode())
+ {
+ return false;
+ }
+ Locator locator = DatabaseDescriptor.getLocator();
Review Comment:
There is a small window in the startup sequence where `DatabaseDescriptor.getLocator()` could return null and will later return a locator instance. I think practically we won't ever this the case where it is null by the time we are creating the `DiskUsageBroadcaster`, but I think out of safety it makes sense to ask for it from the `DatabaseDescriptor`. We could store it as a class variable and only pull it from the `DatabaseDescriptor` when our class variable is null but that seems like it would be more complicated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#issuecomment-3750071667
@smiklosovic, any objection to taking the approach of `data_disk_usage_failure_scope: <replica | keyspace>` (or similarly `data_disk_usage_protection_scope`).
Otherwise, thoughts on `data_disk_usage_keyspace_wide_protection_enabled`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "pauloricardomg (via GitHub)" <gi...@apache.org>.
pauloricardomg commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2699976273
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -131,6 +166,85 @@ private boolean computeHasStuffedOrFullNode()
return false;
}
+ /**
+ * Update the set of full nodes by datacenter based on the disk usage state for the given endpoint.
+ * If the node is FULL, add it to the set for its datacenter. Otherwise, remove it from the set.
+ * This method is idempotent - adding an already-present node or removing an absent node has no effect.
+ *
+ * @param endpoint The endpoint whose state has changed.
+ * @param usageState The new disk usage state value.
+ */
+ private void computeUsageStateForEpDatacenter(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ Location location = location(endpoint);
+ if (location.equals(Location.UNKNOWN))
+ {
+ noSpamLogger.warn("Unable to track disk usage by datacenter for endpoint {} because we are unable to determine its location.",
+ endpoint);
+ return;
+ }
+
+ String datacenter = location.datacenter;
+ if (usageState.isFull())
+ {
+ // Add this node to the set of full nodes for its datacenter and remove it from the stuffed nodes
+ // if it was there.
+ fullNodesByDatacenter.computeIfAbsent(datacenter, dc -> ConcurrentHashMap.newKeySet())
+ .add(endpoint);
+ noSpamLogger.debug("Endpoint {} is FULL, added to full nodes set for datacenter {}", endpoint, datacenter);
+ Set<InetAddressAndPort> stuffedNodes = stuffedNodesByDatacenter.get(datacenter);
+ if (stuffedNodes != null && stuffedNodes.remove(endpoint))
+ {
+ noSpamLogger.debug("Endpoint {} is now FULL. Removed it from the stuffed nodes set for datacenter {}",
+ endpoint, datacenter);
+ }
+ }
+ else if (usageState.isStuffed())
+ {
+ // Add this node to the set of stuffed nodes for its datacenter and remove it from the full nodes
+ // if it was there.
+ stuffedNodesByDatacenter.computeIfAbsent(datacenter, dc -> ConcurrentHashMap.newKeySet())
+ .add(endpoint);
+ noSpamLogger.debug("Endpoint {} is now STUFFED. Added it to the stuffed nodes set for datacenter {}",
+ endpoint, datacenter);
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ if (fullNodes != null && fullNodes.remove(endpoint))
+ {
+ noSpamLogger.debug("Endpoint {} is now STUFFED. Removed it from full nodes set for datacenter {}",
+ endpoint, datacenter);
+ }
+ }
+ else
+ {
+ // Remove this node from the set of full nodes and set of stuffed nodes for its datacenter if it was there.
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ if (fullNodes != null && (fullNodes.remove(endpoint)))
Review Comment:
remove extra parenthesis
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -131,6 +166,85 @@ private boolean computeHasStuffedOrFullNode()
return false;
}
+ /**
+ * Update the set of full nodes by datacenter based on the disk usage state for the given endpoint.
+ * If the node is FULL, add it to the set for its datacenter. Otherwise, remove it from the set.
+ * This method is idempotent - adding an already-present node or removing an absent node has no effect.
+ *
+ * @param endpoint The endpoint whose state has changed.
+ * @param usageState The new disk usage state value.
+ */
+ private void computeUsageStateForEpDatacenter(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ Location location = location(endpoint);
+ if (location.equals(Location.UNKNOWN))
+ {
+ noSpamLogger.warn("Unable to track disk usage by datacenter for endpoint {} because we are unable to determine its location.",
+ endpoint);
+ return;
+ }
+
+ String datacenter = location.datacenter;
+ if (usageState.isFull())
+ {
+ // Add this node to the set of full nodes for its datacenter and remove it from the stuffed nodes
+ // if it was there.
+ fullNodesByDatacenter.computeIfAbsent(datacenter, dc -> ConcurrentHashMap.newKeySet())
+ .add(endpoint);
+ noSpamLogger.debug("Endpoint {} is FULL, added to full nodes set for datacenter {}", endpoint, datacenter);
+ Set<InetAddressAndPort> stuffedNodes = stuffedNodesByDatacenter.get(datacenter);
+ if (stuffedNodes != null && stuffedNodes.remove(endpoint))
+ {
+ noSpamLogger.debug("Endpoint {} is now FULL. Removed it from the stuffed nodes set for datacenter {}",
+ endpoint, datacenter);
+ }
+ }
+ else if (usageState.isStuffed())
+ {
+ // Add this node to the set of stuffed nodes for its datacenter and remove it from the full nodes
+ // if it was there.
+ stuffedNodesByDatacenter.computeIfAbsent(datacenter, dc -> ConcurrentHashMap.newKeySet())
+ .add(endpoint);
+ noSpamLogger.debug("Endpoint {} is now STUFFED. Added it to the stuffed nodes set for datacenter {}",
+ endpoint, datacenter);
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ if (fullNodes != null && fullNodes.remove(endpoint))
+ {
+ noSpamLogger.debug("Endpoint {} is now STUFFED. Removed it from full nodes set for datacenter {}",
+ endpoint, datacenter);
+ }
+ }
+ else
+ {
+ // Remove this node from the set of full nodes and set of stuffed nodes for its datacenter if it was there.
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ if (fullNodes != null && (fullNodes.remove(endpoint)))
+ {
+ noSpamLogger.debug("Endpoint {} is no longer STUFFED or FULL, removed from stuffed for datacenter {}",
+ endpoint, datacenter);
+ }
+ Set<InetAddressAndPort> stuffedNodes = stuffedNodesByDatacenter.get(datacenter);
+ if (stuffedNodes != null && (stuffedNodes.remove(endpoint)))
+ {
+ noSpamLogger.debug("Endpoint {} is not longer STUFFED, removed from the stuffed set for datacenter {}",
Review Comment:
typo: no longer
##########
src/java/org/apache/cassandra/db/guardrails/Guardrails.java:
##########
@@ -556,6 +556,19 @@ public final class Guardrails implements GuardrailsMBean
(isWarning, value) ->
isWarning ? "Replica disk usage exceeds warning threshold"
: "Write request failed because disk usage exceeds failure threshold");
+ /**
+ * Guardrail on the data disk usage of replicas across a datacenter which replicates a given keyspace.
+ * This is used at write time to verify the status of any node which might replicate a given keyspace.
+ */
+ public static final Predicates<String> diskUsageKeyspaceWideProtection =
+ new Predicates<>("disk_usage_keyspace_wide_protection",
+ null,
+ state -> DiskUsageBroadcaster.instance::isDatacenterStuffed,
+ state -> DiskUsageBroadcaster.instance::isDatacenterFull,
+ (isWarning, value) ->
+ isWarning ? "Disk usage in datacenter exceeds warning threshold"
Review Comment:
I think the message should be "disk usage exceeds warning threshold in keyspace datacenter", since it could be wrongly be interpreted that it's in the coordinator's datacenter while it could be in another dc.
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -131,6 +166,85 @@ private boolean computeHasStuffedOrFullNode()
return false;
}
+ /**
+ * Update the set of full nodes by datacenter based on the disk usage state for the given endpoint.
+ * If the node is FULL, add it to the set for its datacenter. Otherwise, remove it from the set.
+ * This method is idempotent - adding an already-present node or removing an absent node has no effect.
+ *
+ * @param endpoint The endpoint whose state has changed.
+ * @param usageState The new disk usage state value.
+ */
+ private void computeUsageStateForEpDatacenter(InetAddressAndPort endpoint, DiskUsageState usageState)
+ {
+ Location location = location(endpoint);
+ if (location.equals(Location.UNKNOWN))
+ {
+ noSpamLogger.warn("Unable to track disk usage by datacenter for endpoint {} because we are unable to determine its location.",
+ endpoint);
+ return;
+ }
+
+ String datacenter = location.datacenter;
+ if (usageState.isFull())
+ {
+ // Add this node to the set of full nodes for its datacenter and remove it from the stuffed nodes
+ // if it was there.
+ fullNodesByDatacenter.computeIfAbsent(datacenter, dc -> ConcurrentHashMap.newKeySet())
+ .add(endpoint);
+ noSpamLogger.debug("Endpoint {} is FULL, added to full nodes set for datacenter {}", endpoint, datacenter);
+ Set<InetAddressAndPort> stuffedNodes = stuffedNodesByDatacenter.get(datacenter);
+ if (stuffedNodes != null && stuffedNodes.remove(endpoint))
+ {
+ noSpamLogger.debug("Endpoint {} is now FULL. Removed it from the stuffed nodes set for datacenter {}",
+ endpoint, datacenter);
+ }
+ }
+ else if (usageState.isStuffed())
+ {
+ // Add this node to the set of stuffed nodes for its datacenter and remove it from the full nodes
+ // if it was there.
+ stuffedNodesByDatacenter.computeIfAbsent(datacenter, dc -> ConcurrentHashMap.newKeySet())
+ .add(endpoint);
+ noSpamLogger.debug("Endpoint {} is now STUFFED. Added it to the stuffed nodes set for datacenter {}",
+ endpoint, datacenter);
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ if (fullNodes != null && fullNodes.remove(endpoint))
+ {
+ noSpamLogger.debug("Endpoint {} is now STUFFED. Removed it from full nodes set for datacenter {}",
+ endpoint, datacenter);
+ }
+ }
+ else
+ {
+ // Remove this node from the set of full nodes and set of stuffed nodes for its datacenter if it was there.
+ Set<InetAddressAndPort> fullNodes = fullNodesByDatacenter.get(datacenter);
+ if (fullNodes != null && (fullNodes.remove(endpoint)))
+ {
+ noSpamLogger.debug("Endpoint {} is no longer STUFFED or FULL, removed from stuffed for datacenter {}",
+ endpoint, datacenter);
+ }
+ Set<InetAddressAndPort> stuffedNodes = stuffedNodesByDatacenter.get(datacenter);
+ if (stuffedNodes != null && (stuffedNodes.remove(endpoint)))
Review Comment:
remove extra parenthesis
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687678412
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -83,6 +89,74 @@ public boolean isStuffed(InetAddressAndPort endpoint)
return state(endpoint).isStuffed();
}
+ /**
+ * @return {@code true} if there exists any node in the datacenter of {@code endpoint} which has FULL disk usage.
+ */
+ public boolean isDataCenterFull(InetAddressAndPort endpoint)
+ {
+ if (!hasStuffedOrFullNode())
+ {
+ return false;
+ }
+ Locator locator = DatabaseDescriptor.getLocator();
+ if (locator == null)
+ {
+ logger.debug("Unable to block writes for keyspace because we cannot determine the datacenter of {}.", endpoint);
Review Comment:
how often is this going to happen? does not it warrant the usage of no-spam logger? similar at other places
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2687681235
##########
src/java/org/apache/cassandra/service/disk/usage/DiskUsageBroadcaster.java:
##########
@@ -83,6 +89,74 @@ public boolean isStuffed(InetAddressAndPort endpoint)
return state(endpoint).isStuffed();
}
+ /**
+ * @return {@code true} if there exists any node in the datacenter of {@code endpoint} which has FULL disk usage.
+ */
+ public boolean isDataCenterFull(InetAddressAndPort endpoint)
+ {
+ if (!hasStuffedOrFullNode())
+ {
+ return false;
+ }
+ Locator locator = DatabaseDescriptor.getLocator();
Review Comment:
is locator ever going to be changed? can not it be a class variable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#issuecomment-3745946242
@isaacreath looks way better conceptually, I think it the name of that guardrail might be reworked a bit though. We can come up with something more descriptive for sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683870771
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
It's not about creating a new guardrail, but adding new behavior to the `replicaDiskUsage` guardrail. Though I suppose I could make this as a totally separate guardrail which implements this behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "isaacreath (via GitHub)" <gi...@apache.org>.
isaacreath commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2682465387
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
I agree, I do not love the name I chose either. I struggle to find the right way to express the concept.
I like `data_disk_usage_stop_writes_on_any_full_node_enabled`, but with the caveat that this doesn't block writes to the whole cluster, just to any node in any data center which overlaps with the the full node.
For example, if we have a cluster with 3 DCs, `DC1`, `DC2`, and `DC3` and two keyspaces: `keyspace_1` and `keyspace_2`. `keyspace_1` has replication in `DC1` and `DC2` and `keyspace_2` has replication `DC2` and `DC3`. If there is a full node in `DC1`, writes to `keyspace_1` will fail, but `keyspace_2` will succeed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "pauloricardomg (via GitHub)" <gi...@apache.org>.
pauloricardomg commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683499348
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
How about
```yaml
# Fail writes to replicas with disk usage above fail threshold
data_disk_usage_failure_scope: replica
```
vs
```yaml
# Fail writes to the keyspace with any replica above fail threshold
data_disk_usage_failure_scope: keyspace
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21024: Add configuration to disk usage guardrails to stop writes across all replicas of a keyspace when any node replicating that keyspace exceeds the disk usage failure threshold. [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4547:
URL: https://github.com/apache/cassandra/pull/4547#discussion_r2683972851
##########
conf/cassandra.yaml:
##########
@@ -2517,6 +2517,10 @@ drop_compact_storage_enabled: false
# Min unit: B
# data_disk_usage_max_disk_size:
#
+# Configures the disk usage guardrails to block all writes to a keyspace if any node which replicates that keyspace
+# is full. By default, this is disabled.
+# data_disk_usage_stop_writes_for_keyspace_on_fail: false
Review Comment:
yeah try `keyspace_disk_usage`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org