[drill] 02/04: DRILL-6912: NPE when other drillbit is already running

[vitalii]

This is an automated email from the ASF dual-hosted git repository.

vitalii pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 9dec5144f669b1fb97208a0b9c848af14b7cbccf
Author: Igor Guzenko <ihor.huzenko....@gmail.com>
AuthorDate: Wed Dec 19 13:45:02 2018 +0200

    DRILL-6912: NPE when other drillbit is already running
    
    closes #1577
---
 .../org/apache/drill/exec/server/Drillbit.java     |  9 ++++-
 .../apache/drill/test/TestGracefulShutdown.java    | 43 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

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 4c595a2..8d41397 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 @@ public class Drillbit implements AutoCloseable {
   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 class Drillbit implements AutoCloseable {
   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 class Drillbit implements AutoCloseable {
     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 class Drillbit implements AutoCloseable {
     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 9e3721c..9d649ba 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.net.URL;
 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 class TestGracefulShutdown extends BaseTestQuery {
     }
   }
 
+  @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) {