From d75ba7b2123af27b3971181893a46a839894200d Mon Sep 17 00:00:00 2001 From: mck Date: Sun, 5 Nov 2017 19:43:22 +1100 Subject: [PATCH] Don't log every jmx connection failure within JmxProxyImpl. The thrown exception is logged appropriately when caught. Also reduces visibility of the not-for-public-use JmxConnectionFactory.connect(..) method. --- .../io/cassandrareaper/jmx/JmxConnectionFactory.java | 10 +++++----- .../main/java/io/cassandrareaper/jmx/JmxProxyImpl.java | 7 +++---- .../java/io/cassandrareaper/acceptance/BasicSteps.java | 2 +- .../cassandrareaper/resources/ClusterResourceTest.java | 2 +- .../resources/RepairRunResourceTest.java | 2 +- .../io/cassandrareaper/service/RepairRunnerTest.java | 6 +++--- .../io/cassandrareaper/service/SegmentRunnerTest.java | 4 ++-- 7 files changed, 16 insertions(+), 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 index 57d25660b..9eb90145a 100644 --- 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 handler, String host, int connectionTimeout) - throws ReaperException, NumberFormatException, InterruptedException { + protected JmxProxy connect(Optional 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 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.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 index 5e57b10e3..f1a13a51e 100644 --- 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 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 index 26b28a1fe..9c62e997b 100644 --- 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(Optional 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 index 103546b58..312bcd821 100644 --- 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(Optional 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 index f81480435..a4c57cd2d 100644 --- 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(Optional 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 index d985f9c79..e5406a2e5 100644 --- 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(final Optional 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(final Optional 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(final Optional 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 index 2e5fe49e8..25a598399 100644 --- 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(final Optional 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 handler, String host, int connectionTimeout) + protected JmxProxy connect(final Optional handler, String host, int connectionTimeout) throws ReaperException { JmxProxy jmx = mock(JmxProxy.class);