Don't log every jmx connection failure within JmxProxyImpl. The throw…

…n exception is logged appropriately when caught. Also reduces visibility of the not-for-public-use JmxConnectionFactory.connect(..) method.
thelastpickle · Nov 8, 2017 · d75ba7b · d75ba7b
1 parent 5d3d469
commit d75ba7b
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 17 deletions.
diff --git a/src/server/src/main/java/io/cassandrareaper/jmx/JmxConnectionFactory.java b/src/server/src/main/java/io/cassandrareaper/jmx/JmxConnectionFactory.java
@@ -54,8 +54,8 @@ public JmxConnectionFactory(MetricRegistry metricRegistry) {
     hostConnectionCounters = new HostConnectionCounters(metricRegistry);
   }
 
-  public JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
-      throws ReaperException, NumberFormatException, InterruptedException {
+  protected JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      throws ReaperException, InterruptedException {
     // use configured jmx port for host if provided
     if (jmxPorts != null && jmxPorts.containsKey(host) && !host.contains(":")) {
       host = host + ":" + jmxPorts.get(host);
@@ -77,8 +77,7 @@ public JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int
     }
   }
 
-  public final JmxProxy connect(String host, int connectionTimeout)
-      throws ReaperException, NumberFormatException, InterruptedException {
+  public final JmxProxy connect(String host, int connectionTimeout) throws ReaperException, InterruptedException {
     return connect(Optional.<RepairStatusHandler>absent(), host, connectionTimeout);
   }
 
@@ -100,7 +99,8 @@ public final JmxProxy connectAny(
           try {
             return connect(handler, host, connectionTimeout);
           } catch (ReaperException | RuntimeException e) {
-            LOG.info("Unreachable host", e);
+            LOG.info("Unreachable host: {}: {}", e.getMessage(), e.getCause().getMessage());
+            LOG.debug("Unreachable host: ", e);
           } catch (InterruptedException expected) {
             LOG.trace("Expected exception", expected);
           }

diff --git a/src/server/src/main/java/io/cassandrareaper/jmx/JmxProxyImpl.java b/src/server/src/main/java/io/cassandrareaper/jmx/JmxProxyImpl.java
@@ -150,7 +150,7 @@ static JmxProxy connect(
       String password,
       final EC2MultiRegionAddressTranslator addressTranslator,
       int connectionTimeout)
-      throws ReaperException, NumberFormatException, InterruptedException {
+      throws ReaperException, InterruptedException {
 
     if (host == null) {
       throw new ReaperException("Null host given to JmxProxy.connect()");
@@ -177,7 +177,7 @@ static JmxProxy connect(
    * @param addressTranslator if EC2MultiRegionAddressTranslator isn't null it will be used to
    *     translate addresses
    */
-  static JmxProxy connect(
+  private static JmxProxy connect(
       Optional<RepairStatusHandler> handler,
       String originalHost,
       int port,
@@ -251,8 +251,7 @@ static JmxProxy connect(
 
       return proxy;
     } catch (IOException | ExecutionException | TimeoutException | InstanceNotFoundException e) {
-      LOG.error("Failed to establish JMX connection to {}:{}", host, port);
-      throw new ReaperException("Failure when establishing JMX connection", e);
+      throw new ReaperException("Failure when establishing JMX connection to " + host + ":" + port, e);
     } catch (InterruptedException expected) {
       LOG.debug(
           "JMX connection to {}:{} was interrupted by Reaper. "

diff --git a/src/server/src/test/java/io/cassandrareaper/acceptance/BasicSteps.java b/src/server/src/test/java/io/cassandrareaper/acceptance/BasicSteps.java
@@ -132,7 +132,7 @@ private void setupReaperTestRunner() throws Exception {
 
       context.jmxConnectionFactory = new JmxConnectionFactory() {
         @Override
-        public JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+        protected JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
             throws ReaperException {
 
           return proxies.get(host);

diff --git a/src/server/src/test/java/io/cassandrareaper/resources/ClusterResourceTest.java b/src/server/src/test/java/io/cassandrareaper/resources/ClusterResourceTest.java
@@ -190,7 +190,7 @@ private MockObjects initMocks() throws ReaperException {
 
     context.jmxConnectionFactory = new JmxConnectionFactory() {
       @Override
-      public JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
         return jmxProxy;
       }

diff --git a/src/server/src/test/java/io/cassandrareaper/resources/RepairRunResourceTest.java b/src/server/src/test/java/io/cassandrareaper/resources/RepairRunResourceTest.java
@@ -127,7 +127,7 @@ public void setUp() throws Exception {
 
     context.jmxConnectionFactory = new JmxConnectionFactory() {
       @Override
-      public JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
         return proxy;
       }

diff --git a/src/server/src/test/java/io/cassandrareaper/service/RepairRunnerTest.java b/src/server/src/test/java/io/cassandrareaper/service/RepairRunnerTest.java
@@ -119,7 +119,7 @@ public void testHangingRepair() throws InterruptedException, ReaperException {
       final AtomicInteger repairAttempts = new AtomicInteger(1);
 
       @Override
-      public JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
 
         final JmxProxy jmx = mock(JmxProxy.class);
@@ -270,7 +270,7 @@ public void testHangingRepairNewAPI() throws InterruptedException, ReaperExcepti
       final AtomicInteger repairAttempts = new AtomicInteger(1);
 
       @Override
-      public JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
 
         final JmxProxy jmx = mock(JmxProxy.class);
@@ -403,7 +403,7 @@ public void testResumeRepair() throws InterruptedException, ReaperException {
     assertEquals(storage.getRepairSegment(RUN_ID, SEGMENT_ID).get().getState(), RepairSegment.State.NOT_STARTED);
     context.jmxConnectionFactory = new JmxConnectionFactory() {
       @Override
-      public JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
 
         final JmxProxy jmx = mock(JmxProxy.class);

diff --git a/src/server/src/test/java/io/cassandrareaper/service/SegmentRunnerTest.java b/src/server/src/test/java/io/cassandrareaper/service/SegmentRunnerTest.java
@@ -192,7 +192,7 @@ public void successTest() throws InterruptedException, ReaperException, Executio
 
     context.jmxConnectionFactory = new JmxConnectionFactory() {
       @Override
-      public JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
 
         JmxProxy jmx = mock(JmxProxy.class);
@@ -307,7 +307,7 @@ public void failureTest() throws InterruptedException, ReaperException, Executio
 
     context.jmxConnectionFactory = new JmxConnectionFactory() {
       @Override
-      public JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
+      protected JmxProxy connect(final Optional<RepairStatusHandler> handler, String host, int connectionTimeout)
           throws ReaperException {
 
         JmxProxy jmx = mock(JmxProxy.class);