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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
