drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sudhe...@apache.org
Subject [06/29] drill git commit: DRILL-5257: Run-time control of query profiles
Date Sat, 25 Feb 2017 07:17:59 GMT
DRILL-5257: Run-time control of query profiles

Adds a run-time option to save (default) or not save query profiles.

Adds a run-time option to save query profiles in "debug" mode:
that is, after returning the last client response. (Normal mode is
to return the response before writing the profile.)

Tests for normal case are normal unit tests. Tests for debug mode
case are unit tests using the new framework that parse profiles.
The test framework is extended to save query profiles using this
new option.

Modifies the test framework to use the new options when a test
asks to save query profiles.

closes #747


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

Branch: refs/heads/master
Commit: 456e26cf0113088a4089226bf088f49cdccf6821
Parents: 93bd7d0
Author: Paul Rogers <progers@maprtech.com>
Authored: Sun Feb 12 19:50:35 2017 -0800
Committer: Sudheesh Katkam <sudheesh@apache.org>
Committed: Fri Feb 24 19:01:39 2017 -0800

----------------------------------------------------------------------
 .../org/apache/drill/exec/ExecConstants.java    | 18 ++++++++++
 .../server/options/SystemOptionManager.java     |  4 ++-
 .../apache/drill/exec/work/foreman/Foreman.java | 36 ++++++++++++++++++--
 .../org/apache/drill/test/FixtureBuilder.java   | 35 ++++++++++---------
 4 files changed, 72 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/456e26cf/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
index ce16c32..846cd8b 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
@@ -414,4 +414,22 @@ public interface ExecConstants {
 
   String DYNAMIC_UDF_SUPPORT_ENABLED = "exec.udf.enable_dynamic_support";
   BooleanValidator DYNAMIC_UDF_SUPPORT_ENABLED_VALIDATOR = new BooleanValidator(DYNAMIC_UDF_SUPPORT_ENABLED,
true, true);
+
+  /**
+   * Option to save query profiles. If false, no query profile will be saved
+   * for any query.
+   */
+  String ENABLE_QUERY_PROFILE_OPTION = "exec.query_profile.save";
+  BooleanValidator ENABLE_QUERY_PROFILE_VALIDATOR = new BooleanValidator(
+      ENABLE_QUERY_PROFILE_OPTION, true, false);
+
+  /**
+   * Profiles are normally written after the last client message to reduce latency.
+   * When running tests, however, we want the profile written <i>before</i> the
+   * return so that the client can immediately read the profile for test
+   * verification.
+   */
+  String QUERY_PROFILE_DEBUG_OPTION = "exec.query_profile.debug_mode";
+  BooleanValidator QUERY_PROFILE_DEBUG_VALIDATOR = new BooleanValidator(
+      QUERY_PROFILE_DEBUG_OPTION, false, false);
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/456e26cf/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
index 26a77ec..425131c 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
@@ -163,7 +163,9 @@ public class SystemOptionManager extends BaseOptionManager implements
AutoClosea
       ExecConstants.CODE_GEN_EXP_IN_METHOD_SIZE_VALIDATOR,
       ExecConstants.CREATE_PREPARE_STATEMENT_TIMEOUT_MILLIS_VALIDATOR,
       ExecConstants.DYNAMIC_UDF_SUPPORT_ENABLED_VALIDATOR,
-      ExecConstants.EXTERNAL_SORT_DISABLE_MANAGED_OPTION
+      ExecConstants.EXTERNAL_SORT_DISABLE_MANAGED_OPTION,
+      ExecConstants.ENABLE_QUERY_PROFILE_VALIDATOR,
+      ExecConstants.QUERY_PROFILE_DEBUG_VALIDATOR
     };
     final Map<String, OptionValidator> tmp = new HashMap<>();
     for (final OptionValidator validator : validators) {

http://git-wip-us.apache.org/repos/asf/drill/blob/456e26cf/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 30718b6..c58edce 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
@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit;
 import org.apache.drill.common.CatastrophicFailure;
 import org.apache.drill.common.EventProcessor;
 import org.apache.drill.common.concurrent.ExtendedLatch;
+import org.apache.drill.common.config.DrillConfig;
 import org.apache.drill.common.exceptions.ExecutionSetupException;
 import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.logical.LogicalPlan;
@@ -117,6 +118,8 @@ public class Foreman implements Runnable {
   private static final org.slf4j.Logger queryLogger = org.slf4j.LoggerFactory.getLogger("query.logger");
   private static final ControlsInjector injector = ControlsInjectorFactory.getInjector(Foreman.class);
 
+  public enum ProfileOption { SYNC, ASYNC, NONE };
+
   private static final ObjectMapper MAPPER = new ObjectMapper();
   private static final long RPC_WAIT_IN_MSECS_PER_FRAGMENT = 5000;
 
@@ -134,6 +137,7 @@ public class Foreman implements Runnable {
   private final UserClientConnection initiatingClient; // used to send responses
   private volatile QueryState state;
   private boolean resume = false;
+  private final ProfileOption profileOption;
 
   private volatile DistributedLease lease; // used to limit the number of concurrent queries
 
@@ -178,6 +182,19 @@ public class Foreman implements Runnable {
     final QueryState initialState = queuingEnabled ? QueryState.ENQUEUED : QueryState.STARTING;
     recordNewState(initialState);
     enqueuedQueries.inc();
+
+    profileOption = setProfileOption(queryContext.getOptions());
+  }
+
+  private ProfileOption setProfileOption(OptionManager options) {
+    if (! options.getOption(ExecConstants.ENABLE_QUERY_PROFILE_VALIDATOR)) {
+      return ProfileOption.NONE;
+    }
+    if (options.getOption(ExecConstants.QUERY_PROFILE_DEBUG_VALIDATOR)) {
+      return ProfileOption.SYNC;
+    } else {
+      return ProfileOption.ASYNC;
+    }
   }
 
   private class ConnectionClosedListener implements GenericFutureListener<Future<Void>>
{
@@ -828,6 +845,13 @@ public class Foreman implements Runnable {
         uex = null;
       }
 
+      // Debug option: write query profile before sending final results so that
+      // the client can be certain the profile exists.
+
+      if (profileOption == ProfileOption.SYNC) {
+        queryManager.writeFinalProfile(uex);
+      }
+
       /*
        * If sending the result fails, we don't really have any way to modify the result we
tried to send;
        * it is possible it got sent but the result came from a later part of the code path.
It is also
@@ -844,7 +868,8 @@ public class Foreman implements Runnable {
 
       // Store the final result here so we can capture any error/errorId in the
       // profile for later debugging.
-      // Write the query profile AFTER sending results to the user. The observed
+      // Normal behavior is to write the query profile AFTER sending results to the user.
+      // The observed
       // user behavior is a possible time-lag between query return and appearance
       // of the query profile in persistent storage. Also, the query might
       // succeed, but the profile never appear if the profile write fails. This
@@ -853,7 +878,9 @@ public class Foreman implements Runnable {
       // storage write; query completion occurs in parallel with profile
       // persistence.
 
-      queryManager.writeFinalProfile(uex);
+      if (profileOption == ProfileOption.ASYNC) {
+        queryManager.writeFinalProfile(uex);
+      }
 
       // Remove the Foreman from the running query list.
       bee.retireForeman(Foreman.this);
@@ -1041,8 +1068,10 @@ public class Foreman implements Runnable {
    */
   private void setupRootFragment(final PlanFragment rootFragment, final FragmentRoot rootOperator)
       throws ExecutionSetupException {
+    @SuppressWarnings("resource")
     final FragmentContext rootContext = new FragmentContext(drillbitContext, rootFragment,
queryContext,
         initiatingClient, drillbitContext.getFunctionImplementationRegistry());
+    @SuppressWarnings("resource")
     final IncomingBuffers buffers = new IncomingBuffers(rootFragment, rootContext);
     rootContext.setBuffers(buffers);
 
@@ -1169,6 +1198,7 @@ public class Foreman implements Runnable {
    */
   private void sendRemoteFragments(final DrillbitEndpoint assignment, final Collection<PlanFragment>
fragments,
       final CountDownLatch latch, final FragmentSubmitFailures fragmentSubmitFailures) {
+    @SuppressWarnings("resource")
     final Controller controller = drillbitContext.getController();
     final InitializeFragments.Builder fb = InitializeFragments.newBuilder();
     for(final PlanFragment planFragment : fragments) {

http://git-wip-us.apache.org/repos/asf/drill/blob/456e26cf/exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java b/exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java
index b1c0bee..461371a 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java
@@ -241,23 +241,6 @@ public class FixtureBuilder {
   }
 
   /**
-   * Enable saving of query profiles. The only way to save them is
-   * to enable local store provider writes, which also saves the
-   * storage plugin configs. Doing so causes the CTTAS feature to
-   * fail on the next run, so the test fixture deletes all local
-   * files on start and close, unless
-   * {@link #keepLocalFiles()} is set.
-   *
-   * @return this builder
-   */
-
-  public FixtureBuilder saveProfiles() {
-    configProperty(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, true);
-//  configProperty(ExecConstants.QUERY_PROFILE_OPTION, "sync") // Temporary until DRILL-5257
is available
-    return this;
-  }
-
-  /**
    * Starting with the addition of the CTTAS feature, a Drillbit will
    * not restart unless we delete all local storage files before
    * starting the Drillbit again. In particular, the stored copies
@@ -280,6 +263,24 @@ public class FixtureBuilder {
   }
 
   /**
+   * Enable saving of query profiles. The only way to save them is
+   * to enable local store provider writes, which also saves the
+   * storage plugin configs. Doing so causes the CTTAS feature to
+   * fail on the next run, so the test fixture deletes all local
+   * files on start and close, unless
+   * {@link #keepLocalFiles()} is set.
+   *
+   * @return this builder
+   */
+
+  public FixtureBuilder saveProfiles() {
+    configProperty(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, true);
+    systemOption(ExecConstants.ENABLE_QUERY_PROFILE_OPTION, true);
+    systemOption(ExecConstants.QUERY_PROFILE_DEBUG_OPTION, true);
+    return this;
+  }
+
+  /**
    * Create the embedded Drillbit and client, applying the options set
    * in the builder. Best to use this in a try-with-resources block:
    * <pre><code>


Mime
View raw message