Print exception message without stacktrace when nodetool commands fai…

…l on probe.getOwnershipWithPort()

Consequently, there is also alignement of nodetool ring command returning
exit code 1 in case there is unrecoverable exception thrown,
same as was already done for status and describecluster commands.

patch by Stefan Miklosovic; reviewed by Brandon Williams and Yifan Cai for CASSANDRA-18079

CHANGES.txt

@@ -1,4 +1,5 @@
 4.2
+ * Print exception message without stacktrace when nodetool commands fail on probe.getOwnershipWithPort() (CASSANDRA-18079)
  * Add option to print level in nodetool getsstables output (CASSANDRA-18023)
  * Implement a guardrail for not having zero default_time_to_live on tables with TWCS (CASSANDRA-18042)
  * Add CQL scalar functions for collection aggregation (CASSANDRA-18060)

src/java/org/apache/cassandra/tools/nodetool/DescribeCluster.java

@@ -32,8 +32,6 @@
 import org.apache.cassandra.tools.NodeTool;
 import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
 
-import static java.lang.String.format;
-
 @Command(name = "describecluster", description = "Print the name, snitch, partitioner and schema version of a cluster")
 public class DescribeCluster extends NodeToolCmd
 {
@@ -62,9 +60,9 @@ public void execute(NodeProbe probe)
         // display schema version for each node
         out.println("\tSchema versions:");
         Map<String, List<String>> schemaVersions = printPort ? probe.getSpProxy().getSchemaVersionsWithPort() : probe.getSpProxy().getSchemaVersions();
-        for (String version : schemaVersions.keySet())
+        for (Map.Entry<String, List<String>> entry : schemaVersions.entrySet())
         {
-            out.println(format("\t\t%s: %s%n", version, schemaVersions.get(version)));
+            out.printf("\t\t%s: %s%n%n", entry.getKey(), entry.getValue());
         }
 
         // Collect status information of all nodes
@@ -86,19 +84,28 @@ public void execute(NodeProbe probe)
         out.println("\tUnreachable: " + unreachableNodes.size());
 
         Map<String, String> tokensToEndpoints = probe.getTokenToEndpointMap(withPort);
+        StringBuilder errors = new StringBuilder();
         Map<String, Float> ownerships = null;
         try
         {
             ownerships = probe.effectiveOwnershipWithPort(keyspace);
         }
         catch (IllegalStateException ex)
         {
-            ownerships = probe.getOwnershipWithPort();
-            out.println("Error: " + ex.getMessage());
+            try
+            {
+                ownerships = probe.getOwnershipWithPort();
+                errors.append("Note: ").append(ex.getMessage()).append("%n");
+            }
+            catch (Exception e)
+            {
+                out.printf("%nError: %s%n", e.getMessage());
+                System.exit(1);
+            }
         }
         catch (IllegalArgumentException ex)
         {
-            out.println("%nError: " + ex.getMessage());
+            out.printf("%nError: %s%n", ex.getMessage());
             System.exit(1);
         }
 
@@ -107,7 +114,7 @@ public void execute(NodeProbe probe)
         out.println("\nData Centers: ");
         for (Map.Entry<String, SetHostStatWithPort> dc : dcs.entrySet())
         {
-            out.print("\t" + dc.getKey());
+            out.print('\t' + dc.getKey());
 
             ArrayListMultimap<InetAddressAndPort, HostStatWithPort> hostToTokens = ArrayListMultimap.create();
             for (HostStatWithPort stat : dc.getValue())
@@ -129,9 +136,9 @@ public void execute(NodeProbe probe)
         // display database version for each node
         out.println("\nDatabase versions:");
         Map<String, List<String>> databaseVersions = probe.getGossProxy().getReleaseVersionsWithPort();
-        for (String version : databaseVersions.keySet())
+        for (Map.Entry<String, List<String>> entry : databaseVersions.entrySet())
         {
-            out.println(format("\t%s: %s%n", version, databaseVersions.get(version)));
+            out.printf("\t%s: %s%n%n", entry.getKey(), entry.getValue());
         }
 
         out.println("Keyspaces:");
@@ -142,7 +149,10 @@ public void execute(NodeProbe probe)
             {
                 out.println("something went wrong for keyspace: " + keyspaceName);
             }
-            out.println("\t" + keyspaceName + " -> Replication class: " + replicationInfo);
+            out.printf("\t%s -> Replication class: %s%n", keyspaceName, replicationInfo);
         }
+
+        if (errors.length() != 0)
+            out.printf("%n" + errors);
     }
 }

src/java/org/apache/cassandra/tools/nodetool/Ring.java

@@ -85,21 +85,29 @@ public void execute(NodeProbe probe)
         boolean showEffectiveOwnership = true;
 
         // Calculate per-token ownership of the ring
-        Map<String, Float> ownerships;
+        Map<String, Float> ownerships = null;
         try
         {
             ownerships = probe.effectiveOwnershipWithPort(keyspace);
         }
         catch (IllegalStateException ex)
         {
-            ownerships = probe.getOwnershipWithPort();
-            errors.append("Note: ").append(ex.getMessage()).append("%n");
-            showEffectiveOwnership = false;
+            try
+            {
+                ownerships = probe.getOwnershipWithPort();
+                errors.append("Note: ").append(ex.getMessage()).append("%n");
+                showEffectiveOwnership = false;
+            }
+            catch (Exception e)
+            {
+                out.printf("%nError: %s%n", ex.getMessage());
+                System.exit(1);
+            }
         }
         catch (IllegalArgumentException ex)
         {
             out.printf("%nError: %s%n", ex.getMessage());
-            return;
+            System.exit(1);
         }
 
         out.println();
@@ -112,7 +120,7 @@ public void execute(NodeProbe probe)
             out.println("  To view status related info of a node use \"nodetool status\" instead.\n");
         }
 
-        out.printf("%n  " + errors.toString());
+        out.printf("%n  " + errors);
     }
 
     private void printDc(String format, String dc,
@@ -167,9 +175,7 @@ else if (leavingNodes.contains(endpoint))
             else if (movingNodes.contains(endpoint))
                 state = "Moving";
 
-            String load = loadMap.containsKey(endpoint)
-                          ? loadMap.get(endpoint)
-                          : "?";
+            String load = loadMap.getOrDefault(endpoint, "?");
             String owns = stat.owns != null && showEffectiveOwnership? new DecimalFormat("##0.00%").format(stat.owns) : "?";
             out.printf(format, stat.ipOrDns(printPort), rack, status, state, load, owns, stat.token);
         }

src/java/org/apache/cassandra/tools/nodetool/Status.java

@@ -79,8 +79,16 @@ public void execute(NodeProbe probe)
         }
         catch (IllegalStateException e)
         {
-            ownerships = probe.getOwnershipWithPort();
-            errors.append("Note: ").append(e.getMessage()).append("%n");
+            try
+            {
+                ownerships = probe.getOwnershipWithPort();
+                errors.append("Note: ").append(e.getMessage()).append("%n");
+            }
+            catch (Exception ex)
+            {
+                out.printf("%nError: %s%n", ex.getMessage());
+                System.exit(1);
+            }
         }
         catch (IllegalArgumentException ex)
         {

test/unit/org/apache/cassandra/tools/nodetool/RingTest.java

@@ -20,6 +20,7 @@
 
 import java.util.Arrays;
 
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -155,7 +156,7 @@ public void testRingKeyspace()
     {
         // Bad KS
         ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("ring", "mockks");
-        tool.assertOnCleanExit();
+        Assert.assertEquals(1, tool.getExitCode());
         assertThat(tool.getStdout()).contains("The keyspace mockks, does not exist");
 
         // Good KS