asfgit closed pull request #1577: DRILL-6912: NPE when other drillbit is
already running
URL: https://github.com/apache/drill/pull/1577
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
index 4c595a2aa60..8d413977f5c 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
@@ -98,7 +98,7 @@
private DrillbitStateManager stateManager;
private boolean quiescentMode;
private boolean forcefulShutdown = false;
- GracefulShutdownThread gracefulShutdownThread;
+ private GracefulShutdownThread gracefulShutdownThread;
private boolean interruptPollShutdown = true;
public void setQuiescentMode(boolean quiescentMode) {
@@ -196,6 +196,7 @@ public int getWebServerPort() {
public void run() throws Exception {
final Stopwatch w = Stopwatch.createStarted();
logger.debug("Startup begun.");
+ gracefulShutdownThread = new GracefulShutdownThread(this, new
StackTrace());
coord.start(10000);
stateManager.setState(DrillbitState.ONLINE);
storeProvider.start();
@@ -222,7 +223,6 @@ public void run() throws Exception {
drillbitContext.startRM();
Runtime.getRuntime().addShutdownHook(new ShutdownThread(this, new
StackTrace()));
- gracefulShutdownThread = new GracefulShutdownThread(this, new
StackTrace());
gracefulShutdownThread.start();
logger.info("Startup completed ({} ms).",
w.elapsed(TimeUnit.MILLISECONDS));
}
@@ -470,6 +470,11 @@ public DrillbitContext getContext() {
return manager.getContext();
}
+ @VisibleForTesting
+ public GracefulShutdownThread getGracefulShutdownThread() {
+ return gracefulShutdownThread;
+ }
+
public static void main(final String[] cli) throws DrillbitStartupException {
final StartupOptions options = StartupOptions.parse(cli);
start(options);
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java
b/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java
index 9e3721cf029..9d649ba4521 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/test/TestGracefulShutdown.java
@@ -17,9 +17,11 @@
*/
package org.apache.drill.test;
import org.apache.drill.categories.SlowTest;
+import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
import org.apache.drill.exec.server.Drillbit;
+import org.apache.hadoop.net.ServerSocketUtil;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -36,6 +38,9 @@
import java.nio.file.Path;
import java.util.Collection;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
@Category({SlowTest.class})
@@ -174,6 +179,44 @@ public void testRestApiShutdown() throws Exception {
}
}
+ @Test // DRILL-6912
+ public void gracefulShutdownThreadShouldBeInitializedBeforeClosingDrillbit()
throws Exception {
+ Drillbit drillbit = null;
+ Drillbit drillbitWithSamePort = null;
+
+ int userPort = ServerSocketUtil.getPort(31170, 300);
+ int bitPort = ServerSocketUtil.getPort(31180, 300);
+ ClusterFixtureBuilder fixtureBuilder =
ClusterFixture.bareBuilder(dirTestWatcher).withLocalZk()
+ .configProperty(ExecConstants.INITIAL_USER_PORT, userPort)
+ .configProperty(ExecConstants.INITIAL_BIT_PORT, bitPort);
+ try (ClusterFixture clusterFixture = fixtureBuilder.build()) {
+ drillbit = clusterFixture.drillbit();
+
+ // creating another drillbit instance using same config
+ drillbitWithSamePort = new Drillbit(clusterFixture.config(),
fixtureBuilder.configBuilder().getDefinitions(),
+ clusterFixture.serviceSet());
+
+ try {
+ drillbitWithSamePort.run();
+ fail("drillbitWithSamePort.run() should throw UserException");
+ } catch (UserException e) {
+ // it's expected that second drillbit can't be started because port is
busy
+ assertThat(e.getMessage(), containsString("RESOURCE ERROR: Drillbit
could not bind to port"));
+ }
+ } finally {
+ // preconditions
+ assertNotNull(drillbit);
+ assertNotNull(drillbitWithSamePort);
+ assertNotNull("gracefulShutdownThread should be initialized, otherwise
NPE will be thrown from close()",
+ drillbit.getGracefulShutdownThread());
+ // main test case
+ assertNotNull("gracefulShutdownThread should be initialized, otherwise
NPE will be thrown from close()",
+ drillbitWithSamePort.getGracefulShutdownThread());
+ drillbit.close();
+ drillbitWithSamePort.close();
+ }
+ }
+
private static boolean waitAndAssertDrillbitCount(ClusterFixture cluster,
int zkRefresh) throws InterruptedException {
while (true) {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
