You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sm...@apache.org on 2022/12/06 11:57:50 UTC

[cassandra] branch trunk updated: Print exception message without stacktrace when nodetool commands fail on probe.getOwnershipWithPort()

This is an automated email from the ASF dual-hosted git repository.

smiklosovic pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new ccada788c4 Print exception message without stacktrace when nodetool commands fail on probe.getOwnershipWithPort()
ccada788c4 is described below

commit ccada788c47882bfb10d7cb86f7f39c9865428f2
Author: Stefan Miklosovic <sm...@apache.org>
AuthorDate: Wed Nov 30 21:31:43 2022 +0100

    Print exception message without stacktrace when nodetool commands fail 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 +
 .../cassandra/tools/nodetool/DescribeCluster.java  | 32 ++++++++++++++--------
 .../org/apache/cassandra/tools/nodetool/Ring.java  | 24 ++++++++++------
 .../apache/cassandra/tools/nodetool/Status.java    | 12 ++++++--
 .../apache/cassandra/tools/nodetool/RingTest.java  |  3 +-
 5 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 68a73159cc..ce975c6fa1 100644
--- a/CHANGES.txt
+++ b/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)
diff --git a/src/java/org/apache/cassandra/tools/nodetool/DescribeCluster.java b/src/java/org/apache/cassandra/tools/nodetool/DescribeCluster.java
index 33a0a4df6b..f855e626b2 100644
--- a/src/java/org/apache/cassandra/tools/nodetool/DescribeCluster.java
+++ b/src/java/org/apache/cassandra/tools/nodetool/DescribeCluster.java
@@ -32,8 +32,6 @@ import org.apache.cassandra.tools.NodeProbe;
 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 class DescribeCluster extends NodeToolCmd
         // 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,6 +84,7 @@ public class DescribeCluster extends NodeToolCmd
         out.println("\tUnreachable: " + unreachableNodes.size());
 
         Map<String, String> tokensToEndpoints = probe.getTokenToEndpointMap(withPort);
+        StringBuilder errors = new StringBuilder();
         Map<String, Float> ownerships = null;
         try
         {
@@ -93,12 +92,20 @@ public class DescribeCluster extends NodeToolCmd
         }
         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 class DescribeCluster extends NodeToolCmd
         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 class DescribeCluster extends NodeToolCmd
         // 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 class DescribeCluster extends NodeToolCmd
             {
                 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);
     }
 }
diff --git a/src/java/org/apache/cassandra/tools/nodetool/Ring.java b/src/java/org/apache/cassandra/tools/nodetool/Ring.java
index ccc7912da8..eb7645d6df 100644
--- a/src/java/org/apache/cassandra/tools/nodetool/Ring.java
+++ b/src/java/org/apache/cassandra/tools/nodetool/Ring.java
@@ -85,21 +85,29 @@ public class Ring extends NodeToolCmd
         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 class Ring extends NodeToolCmd
             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 @@ public class Ring extends NodeToolCmd
             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);
         }
diff --git a/src/java/org/apache/cassandra/tools/nodetool/Status.java b/src/java/org/apache/cassandra/tools/nodetool/Status.java
index 369affc9bc..a89ca64b8d 100644
--- a/src/java/org/apache/cassandra/tools/nodetool/Status.java
+++ b/src/java/org/apache/cassandra/tools/nodetool/Status.java
@@ -79,8 +79,16 @@ public class Status extends NodeToolCmd
         }
         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)
         {
diff --git a/test/unit/org/apache/cassandra/tools/nodetool/RingTest.java b/test/unit/org/apache/cassandra/tools/nodetool/RingTest.java
index 00c8bbadec..28fffd9619 100644
--- a/test/unit/org/apache/cassandra/tools/nodetool/RingTest.java
+++ b/test/unit/org/apache/cassandra/tools/nodetool/RingTest.java
@@ -20,6 +20,7 @@ package org.apache.cassandra.tools.nodetool;
 
 import java.util.Arrays;
 
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -155,7 +156,7 @@ public class RingTest extends CQLTester
     {
         // 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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org