[2/9] drill git commit: DRILL-2520: Foreman is being removed from the running query table prematurely WorkManager: - reinstate retireForeman() on WorkerBee - don't use SelfCleaningRunnable to remove Foreman from running query list

[ja...@apache.org]

DRILL-2520: Foreman is being removed from the running query table prematurely
WorkManager:
- reinstate retireForeman() on WorkerBee
- don't use SelfCleaningRunnable to remove Foreman from running query list

Foreman:
- use retireForeman() to remove self from running query list during cleanup


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/bdeb3449
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/bdeb3449
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/bdeb3449

Branch: refs/heads/master
Commit: bdeb3449bbbda7737c86b197304cf5639a414b47
Parents: 141a1d6
Author: Chris Westin <cw...@yahoo.com>
Authored: Mon Mar 23 13:31:56 2015 -0700
Committer: Jacques Nadeau <ja...@apache.org>
Committed: Thu Mar 26 09:58:35 2015 -0700

----------------------------------------------------------------------
 .../org/apache/drill/exec/work/WorkManager.java | 31 ++++++++++++++++----
 .../apache/drill/exec/work/foreman/Foreman.java |  3 ++
 2 files changed, 28 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/bdeb3449/exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java
index a08630a..231e49a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java
@@ -52,6 +52,7 @@ import org.apache.drill.exec.work.user.UserWorker;
 
 import com.codahale.metrics.Gauge;
 import com.codahale.metrics.MetricRegistry;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
@@ -173,12 +174,30 @@ public class WorkManager implements AutoCloseable {
   public class WorkerBee {
     public void addNewForeman(final Foreman foreman) {
       queries.put(foreman.getQueryId(), foreman);
-      executor.execute(new SelfCleaningRunnable(foreman) {
-        @Override
-        protected void cleanup() {
-          queries.remove(foreman.getQueryId(), foreman);
-        }
-      });
+
+      // We're relying on the Foreman to clean itself up with retireForeman().
+      executor.execute(foreman);
+    }
+
+    /**
+     * Remove the given Foreman from the running query list.
+     *
+     * <p>The running query list is a bit of a misnomer, because it doesn't
+     * necessarily mean that {@link org.apache.drill.exec.work.foreman.Foreman#run()}
+     * is executing. That only lasts for the duration of query setup, after which
+     * the Foreman instance survives as a state machine that reacts to events
+     * from the local root fragment as well as RPC responses from remote Drillbits.</p>
+     *
+     * @param foreman the Foreman to retire
+     */
+    public void retireForeman(final Foreman foreman) {
+      Preconditions.checkNotNull(foreman);
+
+      final QueryId queryId = foreman.getQueryId();
+      final boolean wasRemoved = queries.remove(queryId, foreman);
+      if (!wasRemoved) {
+        throw new IllegalStateException("Couldn't find retiring Foreman for query " + queryId);
+      }
     }
 
     public Foreman getForemanForQueryId(final QueryId queryId) {

http://git-wip-us.apache.org/repos/asf/drill/blob/bdeb3449/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
index 44112c3..32fd650 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
@@ -633,6 +633,9 @@ public class Foreman implements Runnable {
         logger.warn("Exception sending result to client", resultException);
       }
 
+      // Remove the Foreman from the running query list.
+      bee.retireForeman(Foreman.this);
+
       try {
         releaseLease();
       } finally {