[ 
https://issues.apache.org/jira/browse/DRILL-7360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16917012#comment-16917012
 ] 

ASF GitHub Bot commented on DRILL-7360:
---------------------------------------

sohami commented on pull request #1848: DRILL-7360: Refactor WatchService in 
Drillbit class and fix concurrency issues
URL: https://github.com/apache/drill/pull/1848#discussion_r318241977
 
 

 ##########
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
 ##########
 @@ -371,66 +376,98 @@ private void javaPropertiesToSystemOptions() {
     }
   }
 
-
-  // Polls for graceful file to check if graceful shutdown is triggered from 
the script.
+  /**
+   * Polls for graceful file to check if graceful shutdown is triggered from 
the script.
+   */
   private static class GracefulShutdownThread extends Thread {
 
+    private static final String DRILL_HOME = "DRILL_HOME";
+    private static final String GRACEFUL_SIGFILE = "GRACEFUL_SIGFILE";
+    private static final String NOT_SUPPORTED_MESSAGE = "Graceful shutdown 
from command line will not be supported.";
+
     private final Drillbit drillbit;
     private final StackTrace stackTrace;
-    GracefulShutdownThread(final Drillbit drillbit, final StackTrace 
stackTrace) {
+
+    GracefulShutdownThread(Drillbit drillbit, StackTrace stackTrace) {
       this.drillbit = drillbit;
       this.stackTrace = stackTrace;
+
+      setName("Drillbit-Graceful-Shutdown#" + getName());
     }
 
     @Override
     public void run () {
       try {
-        pollShutdown(drillbit);
-      } catch (InterruptedException  e) {
-        logger.debug("Interrupted GracefulShutdownThread");
+        pollShutdown();
+      } catch (InterruptedException e) {
+        drillbit.interruptPollShutdown = false;
+        logger.debug("Graceful Shutdown thread was interrupted", e);
       } catch (IOException e) {
-        throw new RuntimeException("Caught exception while polling for 
gracefulshutdown\n" + stackTrace, e);
+        throw new RuntimeException("Exception while polling for graceful 
shutdown\n" + stackTrace, e);
       }
     }
 
-    /*
-     * Poll for the graceful file, if the file is found cloase the drillbit. 
In case if the DRILL_HOME path is not
-     * set, graceful shutdown will not be supported from the command line.
+    /**
+     * Poll for the graceful file, if the file is found or modified, close the 
Drillbit.
+     * In case if the {@link #DRILL_HOME} or {@link #GRACEFUL_SIGFILE} 
environment variables are not set,
+     * graceful shutdown will not be supported from the command line.
      */
-    private void pollShutdown(Drillbit drillbit) throws IOException, 
InterruptedException {
-      final String drillHome = System.getenv("DRILL_HOME");
-      final String gracefulFile = System.getenv("GRACEFUL_SIGFILE");
-      final Path drillHomePath;
+    private void pollShutdown() throws IOException, InterruptedException {
+      String drillHome = System.getenv(DRILL_HOME);
+      String gracefulFile = System.getenv(GRACEFUL_SIGFILE);
+      Path drillHomePath;
       if (drillHome == null || gracefulFile == null) {
-        logger.warn("Cannot access graceful file. Graceful shutdown from 
command line will not be supported.");
+        if (logger.isWarnEnabled()) {
+          StringBuilder builder = new StringBuilder(NOT_SUPPORTED_MESSAGE);
+          if (drillHome == null) {
+            builder.append(" ").append(DRILL_HOME).append(" is not set.");
+          }
+          if (gracefulFile == null) {
+            builder.append(" ").append(GRACEFUL_SIGFILE).append(" is not 
set.");
+          }
+          logger.warn(builder.toString());
+        }
         return;
       }
       try {
         drillHomePath = Paths.get(drillHome);
+        if (!Files.exists(drillHomePath)) {
+          logger.warn("{} path [{}] does not exist. {}", DRILL_HOME, 
drillHomePath, NOT_SUPPORTED_MESSAGE);
+          return;
+        }
       } catch (InvalidPathException e) {
-        logger.warn("Cannot access graceful file. Graceful shutdown from 
command line will not be supported.");
+        logger.warn("Unable to construct {}} path [{}]: {}. {}",
+          DRILL_HOME, drillHome, e.getMessage(), NOT_SUPPORTED_MESSAGE);
         return;
       }
-      boolean triggered_shutdown = false;
-      WatchKey wk = null;
-      try (final WatchService watchService = 
drillHomePath.getFileSystem().newWatchService()) {
-        drillHomePath.register(watchService, 
StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.ENTRY_CREATE);
-        while (!triggered_shutdown) {
-          wk = watchService.take();
-          for (WatchEvent<?> event : wk.pollEvents()) {
-            final Path changed = (Path) event.context();
-            if (changed != null && changed.endsWith(gracefulFile)) {
-              drillbit.interruptPollShutdown = false;
-              triggered_shutdown = true;
-              drillbit.close();
+
+      poll(drillHomePath, gracefulFile);
+    }
+
+    private void poll(Path drillHomePath, String gracefulFile) throws 
IOException, InterruptedException {
+      while (true) {
+        try (WatchService watchService = 
drillHomePath.getFileSystem().newWatchService()) {
+          drillHomePath.register(watchService, 
StandardWatchEventKinds.ENTRY_CREATE, StandardWatchEventKinds.ENTRY_MODIFY);
+          while (true) {
+            WatchKey watchKey = watchService.take();
+            for (WatchEvent<?> event : watchKey.pollEvents()) {
+              if (StandardWatchEventKinds.OVERFLOW != event.kind()) {
+                Path changedPath = (Path) event.context();
+                if (changedPath != null && changedPath.endsWith(gracefulFile)) 
{
+                  drillbit.interruptPollShutdown = false;
+                  drillbit.close();
+                  return;
+                }
+              }
+            }
+
+            if (!watchKey.reset()) {
+              // if watch key is no longer valid, exit Watch Service loop
+              // and attempt to re-register Drill home path in Watch Service
 
 Review comment:
   please add a debug log here to keep track of these events.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Refactor WatchService code in Drillbit class
> --------------------------------------------
>
>                 Key: DRILL-7360
>                 URL: https://issues.apache.org/jira/browse/DRILL-7360
>             Project: Apache Drill
>          Issue Type: Task
>    Affects Versions: 1.16.0
>            Reporter: Arina Ielchiieva
>            Assignee: Arina Ielchiieva
>            Priority: Major
>             Fix For: 1.17.0
>
>
> Refactor WatchService to user proper code (see 
> https://docs.oracle.com/javase/tutorial/essential/io/notification.html for 
> details) in Drillbit class and fix concurrency issues connected with 
> variables assigning from different thread.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)

Reply via email to