[GitHub] [cassandra] driftx commented on a diff in pull request #2390: CASSANDRA-18555 expose failed decommission via system.local

["driftx (via GitHub)" <gi...@apache.org>]

driftx commented on code in PR #2390:
URL: https://github.com/apache/cassandra/pull/2390#discussion_r1235345897


##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -118,6 +118,7 @@
 import org.apache.cassandra.db.SizeEstimatesRecorder;
 import org.apache.cassandra.db.SnapshotDetailsTabularData;
 import org.apache.cassandra.db.SystemKeyspace;
+import org.apache.cassandra.db.SystemKeyspace.BootstrapState;

Review Comment:
   Do we really need this convenience?  We still have to import SystemKeyspace so it doesn't seem like much savings.



##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -5182,42 +5215,48 @@ public void decommission(boolean force) throws InterruptedException
             }
 
             startLeaving();
-            long timeout = Math.max(RING_DELAY_MILLIS, BatchlogManager.instance.getBatchlogTimeout());
+            long timeout = Math.max(RING_DELAY_MILLIS, BatchlogManager.getBatchlogTimeout());

Review Comment:
   Is there some nuance to this that I don't see, or is it unnecessary?



##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -5150,27 +5175,35 @@ public void decommission(boolean force) throws InterruptedException
 
             String dc = DatabaseDescriptor.getEndpointSnitch().getLocalDatacenter();
 
-            if (operationMode != Mode.LEAVING) // If we're already decommissioning there is no point checking RF/pending ranges
+            // If we're already decommissioning there is no point checking RF/pending ranges
+            if (operationMode != Mode.LEAVING)
             {
                 int rf, numNodes;
                 for (String keyspaceName : Schema.instance.getNonLocalStrategyKeyspaces().names())
                 {
                     if (!force)
                     {
+                        boolean throwException = false;

Review Comment:
   can we name this more descriptively, like 'sufficientNodes' or something?



-- 
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