[GitHub] drill pull request #1196: DRILL-6286: Fixed incorrect reference to shutdown ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/1196


---

[GitHub] drill pull request #1196: DRILL-6286: Fixed incorrect reference to shutdown ...

[vrozov]

Github user vrozov commented on a diff in the pull request:

https://github.com/apache/drill/pull/1196#discussion_r178375583
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java ---
@@ -86,6 +86,9 @@
   private final StatusThread statusThread;
   private final Lock isEmptyLock = new ReentrantLock();
   private final Condition isEmptyCondition = isEmptyLock.newCondition();
+  private boolean isShutdownTriggered = false;
--- End diff --

Is this boolean necessary? Can you delay getting `isEmptyCondition` till 
shutdown is requested and use it in place of `isShutdownTriggered`?


---

[GitHub] drill pull request #1196: DRILL-6286: Fixed incorrect reference to shutdown ...

[vrozov]

Github user vrozov commented on a diff in the pull request:

https://github.com/apache/drill/pull/1196#discussion_r178376213
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java ---
@@ -212,19 +218,29 @@ private boolean areQueriesAndFragmentsEmpty() {
 return queries.isEmpty() && runningFragments.isEmpty();
   }
 
+  /**
+   * Check if there any new queries or fragments that are added after the 
shutdown is triggered
+   */
+  private boolean areNewQueriesOrFragmentsAdded() {
+return runningFragments.size() > numOfRunningFragments || 
queries.size() > numOfRunningQueries;
--- End diff --

This condition is not reliable. What if some fragments exited and some were 
added? The total number may still be less than the number of fragments when the 
shutdown was requested.


---

[GitHub] drill pull request #1196: DRILL-6286: Fixed incorrect reference to shutdown ...

[vrozov]

Github user vrozov commented on a diff in the pull request:

https://github.com/apache/drill/pull/1196#discussion_r178376362
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java ---
@@ -212,19 +218,29 @@ private boolean areQueriesAndFragmentsEmpty() {
 return queries.isEmpty() && runningFragments.isEmpty();
   }
 
+  /**
+   * Check if there any new queries or fragments that are added after the 
shutdown is triggered
+   */
+  private boolean areNewQueriesOrFragmentsAdded() {
+return runningFragments.size() > numOfRunningFragments || 
queries.size() > numOfRunningQueries;
+  }
+
   /**
* A thread calling the {@link #waitToExit(boolean)} method is notified 
when a foreman is retired.
*/
   private void indicateIfSafeToExit() {
 isEmptyLock.lock();
 try {
-  logger.info("Waiting for "+ queries.size() +" queries to complete 
before shutting down");
-  logger.info("Waiting for "+ runningFragments.size() +" running 
fragments to complete before shutting down");
+  if (isShutdownTriggered) {
+logger.info("Waiting for "+ queries.size() +" queries to complete 
before shutting down");
--- End diff --

Use slf4j smart logging.


---

[GitHub] drill pull request #1196: DRILL-6286: Fixed incorrect reference to shutdown ...

[dvjyothsna]

GitHub user dvjyothsna opened a pull request:

https://github.com/apache/drill/pull/1196

DRILL-6286: Fixed incorrect reference to shutdown in drillbit.log

@vrozov , @ilooner Please review

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/dvjyothsna/drill DRILL-6286

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/1196.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1196


commit db251cf61e2e07880fd5a6756ccf31c2df60e338
Author: dvjyothsna 
Date:   2018-03-30T00:11:49Z

DRILL-6286: Fixed incorrect reference to shutdown in drillbit.log




---