From 5edc6da33beef89d0a6e4ba84c4826ff2870f1c7 Mon Sep 17 00:00:00 2001 From: Michael Trelinski Date: Wed, 24 Oct 2018 16:44:35 -0700 Subject: [PATCH 01/11] Update init Fix bin/init to source from proper directory. --- examples/bin/init | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/bin/init b/examples/bin/init index eba2b5a1cf21..3119c3e1aa80 100644 --- a/examples/bin/init +++ b/examples/bin/init @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -gzip -c -d quickstart/wikiticker-2015-09-12-sampled.json.gz > "quickstart/wikiticker-2015-09-12-sampled.json" +gzip -c -d quickstart/tutorial/wikiticker-2015-09-12-sampled.json.gz > "quickstart/wikiticker-2015-09-12-sampled.json" LOG_DIR=var From b9e8650bb2e7c495486c3f6fd6ff59a63fb07a87 Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Thu, 13 Dec 2018 15:03:35 -0800 Subject: [PATCH 02/11] Fix for Proposal #6518: Shutdown druid processes upon complete loss of ZK connectivity --- ...oundedExponentialBackoffRetryWithQuit.java | 59 +++++++++ .../apache/druid/curator/CuratorConfig.java | 14 +++ .../apache/druid/curator/CuratorModule.java | 30 ++++- ...edExponentialBackoffRetryWithQuitTest.java | 118 ++++++++++++++++++ 4 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java create mode 100644 server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java new file mode 100644 index 000000000000..7ee1b7320e82 --- /dev/null +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.curator; + +import org.apache.curator.RetrySleeper; +import org.apache.curator.retry.BoundedExponentialBackoffRetry; +import org.apache.druid.java.util.common.logger.Logger; + +import java.util.function.Function; + +public class BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry +{ + + private static final Logger log = new Logger(BoundedExponentialBackoffRetryWithQuit.class); + + private final Function exitFunction; + + public BoundedExponentialBackoffRetryWithQuit(Function exitFunction, int baseSleepTimeMs, int maxSleepTimeMs, int maxRetries) + { + super(baseSleepTimeMs, maxSleepTimeMs, maxRetries); + this.exitFunction = exitFunction; + log.info("BoundedExponentialBackoffRetryWithQuit Retry Policy selected."); + } + + @Override + public boolean allowRetry(int retryCount, long elapsedTimeMs, RetrySleeper sleeper) + { + log.warn("Zookeeper can't be reached, retrying (retryCount = " + retryCount + " out of " + this.getN() + ")..."); + boolean shouldRetry = super.allowRetry(retryCount, elapsedTimeMs, sleeper); + if (!shouldRetry) { + log.warn("Since Zookeeper can't be reached after retries exhausted, calling exit function..."); + getExitFunction().apply(null); + } + return shouldRetry; + } + + public Function getExitFunction() + { + return exitFunction; + } + +} diff --git a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java index d07b9d9f18fe..c1c8dfbf47e8 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java @@ -52,6 +52,10 @@ public class CuratorConfig @JsonProperty("authScheme") private String authScheme = "digest"; + @JsonProperty("quitOnConnectFail") + private boolean quitOnConnectFail = false; + + public String getZkHosts() { return zkHosts; @@ -109,4 +113,14 @@ public String getAuthScheme() return authScheme; } + public boolean getQuitOnConnectFail() + { + return quitOnConnectFail; + } + + public void setQuitOnConnectFail(Boolean quitOnConnectFail) + { + this.quitOnConnectFail = quitOnConnectFail == null ? false : quitOnConnectFail; + } + } diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index 115a20eb0dcb..655431082447 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -44,6 +44,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.function.Function; /** */ @@ -81,10 +82,24 @@ public CuratorFramework makeCurator( StringUtils.format("%s:%s", config.getZkUser(), config.getZkPwd()).getBytes(StandardCharsets.UTF_8) ); } + + final Function exitFunction = new Function() + { + @Override + public Void apply(Void someVoid) + { + log.error("Zookeeper can't be reached, forcefully stopping lifecycle..."); + lifecycle.stop(); + log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + System.exit(1); + return null; + } + }; + final CuratorFramework framework = builder .ensembleProvider(ensembleProvider) .sessionTimeoutMs(config.getZkSessionTimeoutMs()) - .retryPolicy(new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES)) + .retryPolicy(config.getQuitOnConnectFail() ? new BoundedExponentialBackoffRetryWithQuit(exitFunction, BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) : new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES)) .compressionProvider(new PotentiallyGzippedCompressionProvider(config.getEnableCompression())) .aclProvider(config.getEnableAcl() ? new SecuredACLProvider() : new DefaultACLProvider()) .build(); @@ -129,6 +144,17 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf return new FixedEnsembleProvider(config.getZkHosts()); } + final Function exitFunction = new Function() + { + @Override + public Void apply(Void aVoid) + { + log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + System.exit(1); + return null; + } + }; + return new ExhibitorEnsembleProvider( new Exhibitors( exConfig.getHosts(), @@ -138,7 +164,7 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf new DefaultExhibitorRestClient(exConfig.getUseSsl()), exConfig.getRestUriPath(), exConfig.getPollingMs(), - new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) + config.getQuitOnConnectFail() ? new BoundedExponentialBackoffRetryWithQuit(exitFunction, BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) : new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) ) { @Override diff --git a/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java b/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java new file mode 100644 index 000000000000..c71935a77005 --- /dev/null +++ b/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.curator; + +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.test.TestingServer; +import org.apache.druid.java.util.common.lifecycle.Lifecycle; +import org.apache.druid.java.util.common.logger.Logger; +import org.easymock.EasyMock; +import org.junit.Assert; +import org.junit.Test; + +import java.util.function.Function; + +public final class BoundedExponentialBackoffRetryWithQuitTest +{ + + private static final Logger log = new Logger(BoundedExponentialBackoffRetryWithQuitTest.class); + + /* + Methodology (order is important!): + 1. Zookeeper Server Service started + 2. Lifecycle started + 3. Curator invokes connection to service + 4. Service is stopped + 5. Curator attempts to do something, which invokes the retries policy + 6. Retries exceed limit, call function which simulates an exit (since mocking System.exit() is hard to do without + changing a lot of dependencies) + */ + @Test + public void testExitWithLifecycle() throws Exception + { + final Lifecycle actualNoop = new Lifecycle() { + @Override + public void start() throws Exception + { + super.start(); + log.info("Starting lifecycle..."); + } + + @Override + public void stop() + { + super.stop(); + log.info("Stopping lifecycle..."); + } + }; + Lifecycle noop = EasyMock.mock(Lifecycle.class); + + noop.start(); + EasyMock.expectLastCall().andDelegateTo(actualNoop); + noop.stop(); + EasyMock.expectLastCall().andDelegateTo(actualNoop); + EasyMock.replay(noop); + + Function exitFunction = new Function() + { + @Override + public Void apply(Void aVoid) + { + log.info("Zookeeper retries exhausted, exiting..."); + noop.stop(); + throw new RuntimeException("Simulated exit"); + } + }; + + TestingServer server = new TestingServer(); + BoundedExponentialBackoffRetryWithQuit retry = new BoundedExponentialBackoffRetryWithQuit(exitFunction, 1, 1, 2); + CuratorFramework curator = CuratorFrameworkFactory + .builder() + .connectString(server.getConnectString()) + .sessionTimeoutMs(1000) + .connectionTimeoutMs(1) + .retryPolicy(retry) + .build(); + server.start(); + System.out.println("Server started."); + curator.start(); + noop.start(); + curator.checkExists().forPath("/tmp"); + log.info("Connected."); + boolean failed = false; + try { + server.stop(); + log.info("Stopped."); + curator.checkExists().forPath("/tmp"); + Thread.sleep(10); + curator.checkExists().forPath("/tmp"); + } + catch (Exception e) { + Assert.assertTrue("Correct exception type", e instanceof RuntimeException); + EasyMock.verify(noop); + curator.close(); + failed = true; + } + Assert.assertTrue("Must be marked in failure state", failed); + log.info("Lifecycle stopped."); + } + +} From 8b64673a8c8d38e86dde0b3e5d8ba1cf0f4e226a Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Thu, 20 Dec 2018 17:53:13 -0800 Subject: [PATCH 03/11] Zookeeper Loss: - Add feature documentation - Cosmetic refactors - Variable extractions - Remove getter --- docs/content/configuration/index.md | 1 + ...oundedExponentialBackoffRetryWithQuit.java | 14 ++-- .../apache/druid/curator/CuratorConfig.java | 12 ++-- .../apache/druid/curator/CuratorModule.java | 68 +++++++++++++------ 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/docs/content/configuration/index.md b/docs/content/configuration/index.md index 10e8f333049e..b56b7dda4057 100644 --- a/docs/content/configuration/index.md +++ b/docs/content/configuration/index.md @@ -167,6 +167,7 @@ We recommend just setting the base ZK path and the ZK service host, but all ZK p |`druid.zk.service.user`|The username to authenticate with ZooKeeper. This is an optional property.|none| |`druid.zk.service.pwd`|The [Password Provider](../operations/password-provider.html) or the string password to authenticate with ZooKeeper. This is an optional property.|none| |`druid.zk.service.authScheme`|digest is the only authentication scheme supported. |digest| +|`druid.zk.service.terminateDruidProcess`|When set to 'true', changes behavior of Druid processes that rely on Zookeeper to forcefully exit. If the connection to Zookeeper fails (after exhausting all exponential backoff retries), the process will die with an errorlevel of 1.|false| #### Zookeeper Behavior diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java index 7ee1b7320e82..a43c0826ceaf 100644 --- a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -32,7 +32,12 @@ public class BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBa private final Function exitFunction; - public BoundedExponentialBackoffRetryWithQuit(Function exitFunction, int baseSleepTimeMs, int maxSleepTimeMs, int maxRetries) + public BoundedExponentialBackoffRetryWithQuit( + Function exitFunction, + int baseSleepTimeMs, + int maxSleepTimeMs, + int maxRetries + ) { super(baseSleepTimeMs, maxSleepTimeMs, maxRetries); this.exitFunction = exitFunction; @@ -46,14 +51,9 @@ public boolean allowRetry(int retryCount, long elapsedTimeMs, RetrySleeper sleep boolean shouldRetry = super.allowRetry(retryCount, elapsedTimeMs, sleeper); if (!shouldRetry) { log.warn("Since Zookeeper can't be reached after retries exhausted, calling exit function..."); - getExitFunction().apply(null); + exitFunction.apply(null); } return shouldRetry; } - public Function getExitFunction() - { - return exitFunction; - } - } diff --git a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java index c1c8dfbf47e8..a1ec04d2b213 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java @@ -52,8 +52,8 @@ public class CuratorConfig @JsonProperty("authScheme") private String authScheme = "digest"; - @JsonProperty("quitOnConnectFail") - private boolean quitOnConnectFail = false; + @JsonProperty("terminateDruidProcess") + private boolean terminateDruidProcess = false; public String getZkHosts() @@ -113,14 +113,14 @@ public String getAuthScheme() return authScheme; } - public boolean getQuitOnConnectFail() + public boolean getTerminateDruidProcess() { - return quitOnConnectFail; + return terminateDruidProcess; } - public void setQuitOnConnectFail(Boolean quitOnConnectFail) + public void setTerminateDruidProcess(Boolean terminateDruidProcess) { - this.quitOnConnectFail = quitOnConnectFail == null ? false : quitOnConnectFail; + this.terminateDruidProcess = terminateDruidProcess == null ? false : terminateDruidProcess; } } diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index fe44e50338af..254b51c8a1a7 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -22,6 +22,7 @@ import com.google.inject.Binder; import com.google.inject.Module; import com.google.inject.Provides; +import org.apache.curator.RetryPolicy; import org.apache.curator.ensemble.EnsembleProvider; import org.apache.curator.ensemble.exhibitor.DefaultExhibitorRestClient; import org.apache.curator.ensemble.exhibitor.ExhibitorEnsembleProvider; @@ -81,23 +82,34 @@ public CuratorFramework makeCurator(CuratorConfig config, EnsembleProvider ensem ); } - final Function exitFunction = new Function() - { - @Override - public Void apply(Void someVoid) + RetryPolicy retryPolicy; + if (config.getTerminateDruidProcess()) { + final Function exitFunction = new Function() { - log.error("Zookeeper can't be reached, forcefully stopping lifecycle..."); - lifecycle.stop(); - log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); - System.exit(1); - return null; - } - }; + @Override + public Void apply(Void someVoid) + { + log.error("Zookeeper can't be reached, forcefully stopping lifecycle..."); + lifecycle.stop(); + log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + System.exit(1); + return null; + } + }; + retryPolicy = new BoundedExponentialBackoffRetryWithQuit( + exitFunction, + BASE_SLEEP_TIME_MS, + MAX_SLEEP_TIME_MS, + MAX_RETRIES + ); + } else { + retryPolicy = new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES); + } final CuratorFramework framework = builder .ensembleProvider(ensembleProvider) .sessionTimeoutMs(config.getZkSessionTimeoutMs()) - .retryPolicy(config.getQuitOnConnectFail() ? new BoundedExponentialBackoffRetryWithQuit(exitFunction, BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) : new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES)) + .retryPolicy(retryPolicy) .compressionProvider(new PotentiallyGzippedCompressionProvider(config.getEnableCompression())) .aclProvider(config.getEnableAcl() ? new SecuredACLProvider() : new DefaultACLProvider()) .build(); @@ -142,16 +154,28 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf return new FixedEnsembleProvider(config.getZkHosts()); } - final Function exitFunction = new Function() - { - @Override - public Void apply(Void aVoid) + RetryPolicy retryPolicy; + if (config.getTerminateDruidProcess()) { + final Function exitFunction = new Function() { - log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); - System.exit(1); - return null; - } - }; + @Override + public Void apply(Void aVoid) + { + log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + System.exit(1); + return null; + } + }; + + retryPolicy = new BoundedExponentialBackoffRetryWithQuit( + exitFunction, + BASE_SLEEP_TIME_MS, + MAX_SLEEP_TIME_MS, + MAX_RETRIES + ); + } else { + retryPolicy = new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES); + } return new ExhibitorEnsembleProvider( new Exhibitors( @@ -162,7 +186,7 @@ public Void apply(Void aVoid) new DefaultExhibitorRestClient(exConfig.getUseSsl()), exConfig.getRestUriPath(), exConfig.getPollingMs(), - config.getQuitOnConnectFail() ? new BoundedExponentialBackoffRetryWithQuit(exitFunction, BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) : new BoundedExponentialBackoffRetry(BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES) + retryPolicy ) { @Override From 39d538d534f60a44852d84b228b75b6760330f90 Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Mon, 24 Dec 2018 15:29:38 -0800 Subject: [PATCH 04/11] =?UTF-8?q?-=20Change=20config=20key=20name=20and=20?= =?UTF-8?q?reword=20documentation=20-=20Switch=20from=20Function=20to=20Runnable/Lambda=20-=20try=20{=20=E2=80=A6=20}=20finall?= =?UTF-8?q?y=20{=20=E2=80=A6=20}?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/content/configuration/index.md | 2 +- ...oundedExponentialBackoffRetryWithQuit.java | 10 +++---- .../apache/druid/curator/CuratorConfig.java | 12 ++++---- .../apache/druid/curator/CuratorModule.java | 29 ++++++++----------- ...edExponentialBackoffRetryWithQuitTest.java | 15 +++------- 5 files changed, 27 insertions(+), 41 deletions(-) diff --git a/docs/content/configuration/index.md b/docs/content/configuration/index.md index b56b7dda4057..7de92a2d288b 100644 --- a/docs/content/configuration/index.md +++ b/docs/content/configuration/index.md @@ -167,7 +167,7 @@ We recommend just setting the base ZK path and the ZK service host, but all ZK p |`druid.zk.service.user`|The username to authenticate with ZooKeeper. This is an optional property.|none| |`druid.zk.service.pwd`|The [Password Provider](../operations/password-provider.html) or the string password to authenticate with ZooKeeper. This is an optional property.|none| |`druid.zk.service.authScheme`|digest is the only authentication scheme supported. |digest| -|`druid.zk.service.terminateDruidProcess`|When set to 'true', changes behavior of Druid processes that rely on Zookeeper to forcefully exit. If the connection to Zookeeper fails (after exhausting all exponential backoff retries), the process will die with an errorlevel of 1.|false| +|`druid.zk.service.terminateDruidProcessOnConnectFail`|If set to 'true' and the connection to ZooKeeper fails (after exhausting all potential backoff retires), Druid process terminates itself with exit code 1.|false| #### Zookeeper Behavior diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java index a43c0826ceaf..6672e00f4ee0 100644 --- a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -23,24 +23,22 @@ import org.apache.curator.retry.BoundedExponentialBackoffRetry; import org.apache.druid.java.util.common.logger.Logger; -import java.util.function.Function; - public class BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry { private static final Logger log = new Logger(BoundedExponentialBackoffRetryWithQuit.class); - private final Function exitFunction; + private final Runnable exitRunner; public BoundedExponentialBackoffRetryWithQuit( - Function exitFunction, + Runnable exitRunner, int baseSleepTimeMs, int maxSleepTimeMs, int maxRetries ) { super(baseSleepTimeMs, maxSleepTimeMs, maxRetries); - this.exitFunction = exitFunction; + this.exitRunner = exitRunner; log.info("BoundedExponentialBackoffRetryWithQuit Retry Policy selected."); } @@ -51,7 +49,7 @@ public boolean allowRetry(int retryCount, long elapsedTimeMs, RetrySleeper sleep boolean shouldRetry = super.allowRetry(retryCount, elapsedTimeMs, sleeper); if (!shouldRetry) { log.warn("Since Zookeeper can't be reached after retries exhausted, calling exit function..."); - exitFunction.apply(null); + exitRunner.run(); } return shouldRetry; } diff --git a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java index a1ec04d2b213..fe600b61e102 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java @@ -52,8 +52,8 @@ public class CuratorConfig @JsonProperty("authScheme") private String authScheme = "digest"; - @JsonProperty("terminateDruidProcess") - private boolean terminateDruidProcess = false; + @JsonProperty("terminateDruidProcessOnConnectFail") + private boolean terminateDruidProcessOnConnectFail = false; public String getZkHosts() @@ -113,14 +113,14 @@ public String getAuthScheme() return authScheme; } - public boolean getTerminateDruidProcess() + public boolean getTerminateDruidProcessOnConnectFail() { - return terminateDruidProcess; + return terminateDruidProcessOnConnectFail; } - public void setTerminateDruidProcess(Boolean terminateDruidProcess) + public void setTerminateDruidProcessOnConnectFail(Boolean terminateDruidProcessOnConnectFail) { - this.terminateDruidProcess = terminateDruidProcess == null ? false : terminateDruidProcess; + this.terminateDruidProcessOnConnectFail = terminateDruidProcessOnConnectFail == null ? false : terminateDruidProcessOnConnectFail; } } diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index 254b51c8a1a7..5152e1b678c8 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -45,7 +45,6 @@ import java.nio.charset.StandardCharsets; import java.util.List; -import java.util.function.Function; /** */ @@ -83,21 +82,19 @@ public CuratorFramework makeCurator(CuratorConfig config, EnsembleProvider ensem } RetryPolicy retryPolicy; - if (config.getTerminateDruidProcess()) { - final Function exitFunction = new Function() - { - @Override - public Void apply(Void someVoid) - { + if (config.getTerminateDruidProcessOnConnectFail()) { + final Runnable exitRunner = () -> { + try { log.error("Zookeeper can't be reached, forcefully stopping lifecycle..."); lifecycle.stop(); log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + } + finally { System.exit(1); - return null; } }; retryPolicy = new BoundedExponentialBackoffRetryWithQuit( - exitFunction, + exitRunner, BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES @@ -155,20 +152,18 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf } RetryPolicy retryPolicy; - if (config.getTerminateDruidProcess()) { - final Function exitFunction = new Function() - { - @Override - public Void apply(Void aVoid) - { + if (config.getTerminateDruidProcessOnConnectFail()) { + final Runnable exitRunner = () -> { + try { log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + } + finally { System.exit(1); - return null; } }; retryPolicy = new BoundedExponentialBackoffRetryWithQuit( - exitFunction, + exitRunner, BASE_SLEEP_TIME_MS, MAX_SLEEP_TIME_MS, MAX_RETRIES diff --git a/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java b/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java index c71935a77005..a5fb4fea9ec0 100644 --- a/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java +++ b/server/src/test/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuitTest.java @@ -28,8 +28,6 @@ import org.junit.Assert; import org.junit.Test; -import java.util.function.Function; - public final class BoundedExponentialBackoffRetryWithQuitTest { @@ -71,15 +69,10 @@ public void stop() EasyMock.expectLastCall().andDelegateTo(actualNoop); EasyMock.replay(noop); - Function exitFunction = new Function() - { - @Override - public Void apply(Void aVoid) - { - log.info("Zookeeper retries exhausted, exiting..."); - noop.stop(); - throw new RuntimeException("Simulated exit"); - } + Runnable exitFunction = () -> { + log.info("Zookeeper retries exhausted, exiting..."); + noop.stop(); + throw new RuntimeException("Simulated exit"); }; TestingServer server = new TestingServer(); From 0df72749b491551f60ba5b0003c908b2a7ae3509 Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Fri, 1 Feb 2019 16:15:32 -0800 Subject: [PATCH 05/11] Fix line length too long --- .../main/java/org/apache/druid/curator/CuratorConfig.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java index fe600b61e102..52af44e7cbd7 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorConfig.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorConfig.java @@ -120,7 +120,11 @@ public boolean getTerminateDruidProcessOnConnectFail() public void setTerminateDruidProcessOnConnectFail(Boolean terminateDruidProcessOnConnectFail) { - this.terminateDruidProcessOnConnectFail = terminateDruidProcessOnConnectFail == null ? false : terminateDruidProcessOnConnectFail; + if (terminateDruidProcessOnConnectFail == null) { + this.terminateDruidProcessOnConnectFail = false; + } else { + this.terminateDruidProcessOnConnectFail = terminateDruidProcessOnConnectFail; + } } } From 1ece6978b63e12041db405d2f1bfed751e6fda8f Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Mon, 4 Feb 2019 14:11:24 -0800 Subject: [PATCH 06/11] - change to formatted string for logging - use System.err.println after lifecycle stops --- .../druid/curator/BoundedExponentialBackoffRetryWithQuit.java | 2 +- .../src/main/java/org/apache/druid/curator/CuratorModule.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java index 6672e00f4ee0..c6f8e632a5a1 100644 --- a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -45,7 +45,7 @@ public BoundedExponentialBackoffRetryWithQuit( @Override public boolean allowRetry(int retryCount, long elapsedTimeMs, RetrySleeper sleeper) { - log.warn("Zookeeper can't be reached, retrying (retryCount = " + retryCount + " out of " + this.getN() + ")..."); + log.warn("Zookeeper can't be reached, retrying (retryCount = %s out of %s)...", retryCount, this.getN()); boolean shouldRetry = super.allowRetry(retryCount, elapsedTimeMs, sleeper); if (!shouldRetry) { log.warn("Since Zookeeper can't be reached after retries exhausted, calling exit function..."); diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index 5152e1b678c8..2a3309095829 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -22,6 +22,7 @@ import com.google.inject.Binder; import com.google.inject.Module; import com.google.inject.Provides; +import io.netty.util.SuppressForbidden; import org.apache.curator.RetryPolicy; import org.apache.curator.ensemble.EnsembleProvider; import org.apache.curator.ensemble.exhibitor.DefaultExhibitorRestClient; @@ -71,6 +72,7 @@ public void configure(Binder binder) @Provides @LazySingleton + @SuppressForbidden(reason = "System#err") public CuratorFramework makeCurator(CuratorConfig config, EnsembleProvider ensembleProvider, Lifecycle lifecycle) { final Builder builder = CuratorFrameworkFactory.builder(); @@ -87,7 +89,7 @@ public CuratorFramework makeCurator(CuratorConfig config, EnsembleProvider ensem try { log.error("Zookeeper can't be reached, forcefully stopping lifecycle..."); lifecycle.stop(); - log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); + System.err.println("Zookeeper can't be reached, forcefully stopping virtual machine..."); } finally { System.exit(1); From 3fb949e7284cb861cc92ddccde6ad92463c27dfe Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Mon, 4 Feb 2019 14:17:39 -0800 Subject: [PATCH 07/11] commenting on makeEnsembleProvider()-created Zookeeper termination --- server/src/main/java/org/apache/druid/curator/CuratorModule.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index 2a3309095829..8023faf96ae2 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -155,6 +155,7 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf RetryPolicy retryPolicy; if (config.getTerminateDruidProcessOnConnectFail()) { + // It's unknown whether or not this precaution is needed. Tests revealed that this path was never taken. final Runnable exitRunner = () -> { try { log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); From 1e7774ea18d06e3fdcd03fef6b227a9a253898db Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Tue, 19 Mar 2019 16:01:23 -0700 Subject: [PATCH 08/11] Add javadoc --- .../curator/BoundedExponentialBackoffRetryWithQuit.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java index c6f8e632a5a1..94680edb5c70 100644 --- a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -23,6 +23,11 @@ import org.apache.curator.retry.BoundedExponentialBackoffRetry; import org.apache.druid.java.util.common.logger.Logger; +/** + * BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry for simplicity. It's not actually a + * BoundedExponentialBackoffRetry from the Liskov substitution principle point of view, + * but it doesn't matter in this code. + */ public class BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry { From 2d1630981534170bee2c254d75c7e1cac337fb94 Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Wed, 20 Mar 2019 16:19:22 -0700 Subject: [PATCH 09/11] added java doc reference back to apache discussion thread. --- .../druid/curator/BoundedExponentialBackoffRetryWithQuit.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java index 94680edb5c70..8bf64c2855bc 100644 --- a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -27,6 +27,8 @@ * BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry for simplicity. It's not actually a * BoundedExponentialBackoffRetry from the Liskov substitution principle point of view, * but it doesn't matter in this code. + * + * see discussions in https://github.com/apache/incubator-druid/pull/6740 */ public class BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry { From 1ecd4891b5cbe20029766f80af6c4fa6dfce11c8 Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Wed, 20 Mar 2019 16:55:22 -0700 Subject: [PATCH 10/11] move comment to other class --- .../curator/BoundedExponentialBackoffRetryWithQuit.java | 1 - .../main/java/org/apache/druid/curator/CuratorModule.java | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java index 8bf64c2855bc..531edb4a1025 100644 --- a/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java +++ b/server/src/main/java/org/apache/druid/curator/BoundedExponentialBackoffRetryWithQuit.java @@ -28,7 +28,6 @@ * BoundedExponentialBackoffRetry from the Liskov substitution principle point of view, * but it doesn't matter in this code. * - * see discussions in https://github.com/apache/incubator-druid/pull/6740 */ public class BoundedExponentialBackoffRetryWithQuit extends BoundedExponentialBackoffRetry { diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index 8023faf96ae2..8d87a6ea3da5 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -155,7 +155,11 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf RetryPolicy retryPolicy; if (config.getTerminateDruidProcessOnConnectFail()) { - // It's unknown whether or not this precaution is needed. Tests revealed that this path was never taken. + /** It's unknown whether or not this precaution is needed. Tests revealed that this path was never taken. + * + * see discussions in https://github.com/apache/incubator-druid/pull/6740 + * + */ final Runnable exitRunner = () -> { try { log.error("Zookeeper can't be reached, forcefully stopping virtual machine..."); From 44592577f64e5df0cc63ad62622d7a1c56dd16ab Mon Sep 17 00:00:00 2001 From: Mike Trelinski Date: Thu, 21 Mar 2019 14:00:25 -0700 Subject: [PATCH 11/11] favor two-slash comments instead of multiline comments --- .../main/java/org/apache/druid/curator/CuratorModule.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/apache/druid/curator/CuratorModule.java b/server/src/main/java/org/apache/druid/curator/CuratorModule.java index 8d87a6ea3da5..2c27dbaa9600 100644 --- a/server/src/main/java/org/apache/druid/curator/CuratorModule.java +++ b/server/src/main/java/org/apache/druid/curator/CuratorModule.java @@ -155,11 +155,9 @@ public EnsembleProvider makeEnsembleProvider(CuratorConfig config, ExhibitorConf RetryPolicy retryPolicy; if (config.getTerminateDruidProcessOnConnectFail()) { - /** It's unknown whether or not this precaution is needed. Tests revealed that this path was never taken. - * - * see discussions in https://github.com/apache/incubator-druid/pull/6740 - * - */ + // It's unknown whether or not this precaution is needed. Tests revealed that this path was never taken. + // see discussions in https://github.com/apache/incubator-druid/pull/6740 + final Runnable exitRunner = () -> { try { log.error("Zookeeper can't be reached, forcefully stopping virtual machine...");