From d4203239a23249ad626e02c206d2dfdaa86a8938 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 10 May 2018 11:18:00 +0100 Subject: [PATCH 1/2] Remove Discovery.AckListener.onTimeout() The MasterService takes responsibility for timeouts of the AckListeners that it creates, and the rest of the Discovery subsystem is unaware of these timeouts, so there's no need for this to appear in the Discovery.AckListener interface. Also fix a typo in the name of DelegatingAckListener. --- .../elasticsearch/cluster/service/MasterService.java | 12 +++--------- .../java/org/elasticsearch/discovery/Discovery.java | 1 - .../zen/PublishClusterStateActionTests.java | 10 ---------- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java b/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java index 20a6602b5c5ad..54a6568af3fa2 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java @@ -386,7 +386,7 @@ public Discovery.AckListener createAckListener(ThreadPool threadPool, ClusterSta } }); - return new DelegetingAckListener(ackListeners); + return new DelegatingAckListener(ackListeners); } public boolean clusterStateUnchanged() { @@ -541,11 +541,11 @@ protected void warnAboutSlowTaskIfNeeded(TimeValue executionTime, String source) } } - private static class DelegetingAckListener implements Discovery.AckListener { + private static class DelegatingAckListener implements Discovery.AckListener { private final List listeners; - private DelegetingAckListener(List listeners) { + private DelegatingAckListener(List listeners) { this.listeners = listeners; } @@ -555,11 +555,6 @@ public void onNodeAck(DiscoveryNode node, @Nullable Exception e) { listener.onNodeAck(node, e); } } - - @Override - public void onTimeout() { - throw new UnsupportedOperationException("no timeout delegation"); - } } private static class AckCountDownListener implements Discovery.AckListener { @@ -614,7 +609,6 @@ public void onNodeAck(DiscoveryNode node, @Nullable Exception e) { } } - @Override public void onTimeout() { if (countDown.fastForward()) { logger.trace("timeout waiting for acknowledgement for cluster_state update (version: {})", clusterStateVersion); diff --git a/server/src/main/java/org/elasticsearch/discovery/Discovery.java b/server/src/main/java/org/elasticsearch/discovery/Discovery.java index 3842e68d1006b..9c70876032442 100644 --- a/server/src/main/java/org/elasticsearch/discovery/Discovery.java +++ b/server/src/main/java/org/elasticsearch/discovery/Discovery.java @@ -49,7 +49,6 @@ public interface Discovery extends LifecycleComponent { interface AckListener { void onNodeAck(DiscoveryNode node, @Nullable Exception e); - void onTimeout(); } class FailedToCommitClusterStateException extends ElasticsearchException { diff --git a/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java b/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java index 42ec72c981007..7683097570cf2 100644 --- a/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java @@ -829,16 +829,6 @@ public void onNodeAck(DiscoveryNode node, @Nullable Exception e) { countDown.countDown(); } - @Override - public void onTimeout() { - timeoutOccurred.set(true); - // Fast forward the counter - no reason to wait here - long currentCount = countDown.getCount(); - for (long i = 0; i < currentCount; i++) { - countDown.countDown(); - } - } - public void await(long timeout, TimeUnit unit) throws InterruptedException { assertThat(awaitErrors(timeout, unit), emptyIterable()); } From 09e5747d2cf59fab68f7f0d1b685c4ced559ad21 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 10 May 2018 11:35:00 +0100 Subject: [PATCH 2/2] timeoutOccurred is now always false --- .../discovery/zen/PublishClusterStateActionTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java b/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java index 7683097570cf2..c8e85382994c7 100644 --- a/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/zen/PublishClusterStateActionTests.java @@ -814,7 +814,6 @@ public AssertingAckListener publishState(PublishClusterStateAction action, Clust public static class AssertingAckListener implements Discovery.AckListener { private final List> errors = new CopyOnWriteArrayList<>(); - private final AtomicBoolean timeoutOccurred = new AtomicBoolean(); private final CountDownLatch countDown; public AssertingAckListener(int nodeCount) { @@ -835,7 +834,6 @@ public void await(long timeout, TimeUnit unit) throws InterruptedException { public List> awaitErrors(long timeout, TimeUnit unit) throws InterruptedException { countDown.await(timeout, unit); - assertFalse(timeoutOccurred.get()); return errors; }