You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by paul-rogers <gi...@git.apache.org> on 2016/12/28 07:10:14 UTC

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

GitHub user paul-rogers opened a pull request:

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

    DRILL-5126: Provide simplified, unified "cluster fixture" for test

    Drill provides a robust selection of test frameworks that have evolved to satisfy the needs of a variety of test cases. However, some do some of what a given test needs, while others to other parts. Also, the various frameworks make assumptions (in the form of boot-time configuration) that differs from what some test may need, forcing the test to start, then stop, then restart a Drillbit - an expensive operation.
    
    Also, many ways exist to run queries, but they all do part of the job. Several ways exist to change runtime options.
    
    This checkin shamelessly grabs the best parts from existing frameworks, adds a fluent builder facade and provides a complete, versitie test framework for new tests. Old tests are unaffected by this new code.
    
    An adjustment was made to allow use of the existing TestBuilder mechanism. TestBuilder used to depend on static members of BaseTestQuery. A "shim" allows the same code to work in the old way for old tests, but with the new ClusterFixture for new tests.
    
    Details are in the org.apache.drill.test.package-info.java file.
    
    This commit modifies a single test case, TestSimpleExternalSort, to use the new framework. More cases will follow once this framework itself is committed.
    
    Also, the framework will eventually allow use of the extended mock data source from SQL. However, that change must await checkin of the mock data source changes.

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

    $ git pull https://github.com/paul-rogers/drill DRILL-5126

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

    https://github.com/apache/drill/pull/710.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 #710
    
----
commit 1df75e40535209c74f4e8dd807ced560bbea2e50
Author: Paul Rogers <pr...@maprtech.com>
Date:   2016-12-13T21:41:23Z

    DRILL-5126: Provide simplified, unified "cluster fixture" for test
    
    Drill provides a robust selection of test frameworks that have evolved to satisfy the needs of a variety of test cases.
    However, some do some of what a given test needs, while others to other parts. Also, the various frameworks make
    assumptions (in the form of boot-time configuration) that differs from what some test may need, forcing the test
    to start, then stop, then restart a Drillbit - an expensive operation.
    
    Also, many ways exist to run queries, but they all do part of the job. Several ways exist to channge
    runtime options.
    
    This checkin shamelessly grabs the best parts from existing frameworks, adds a fluent builder facade
    and provides a complete, versitie test framework for new tests. Old tests are unaffected by this
    new code.
    
    An adjustment was made to allow use of the existing TestBuilder mechanism. TestBuilder used to
    depend on static members of BaseTestQuery. A "shim" allows the same code to work in the old
    way for old tests, but with the new ClusterFixture for new tests.
    
    Details are in the org.apache.drill.test.package-info.java file.
    
    This commit modifies a single test case, TestSimpleExternalSort, to use the new framework.
    More cases will follow once this framework itself is committed.
    
    Also, the framework will eventually allow use of the extended mock data source
    from SQL. However, that change must await checkin of the mock data source changes.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #710: DRILL-5126: Provide simplified, unified "cluster fixture" ...

Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on the issue:

    https://github.com/apache/drill/pull/710
  
    +1, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95052737
  
    --- Diff: exec/java-exec/pom.xml ---
    @@ -458,6 +458,12 @@
           <artifactId>httpdlog-parser</artifactId>
           <version>2.4</version>
         </dependency>
    +	<dependency>
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95044899
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.FileReader;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.regex.Matcher;
    +import java.util.regex.Pattern;
    +
    +import javax.json.Json;
    +import javax.json.JsonArray;
    +import javax.json.JsonNumber;
    +import javax.json.JsonObject;
    +import javax.json.JsonReader;
    +import javax.json.JsonValue;
    +
    +/**
    + * Parses a query profile and provides access to various bits of the profile
    + * for diagnostic purposes during tests.
    + */
    +
    +public class ProfileParser {
    +
    +  JsonObject profile;
    +  List<String> plans;
    +
    +  public ProfileParser( File file ) throws IOException {
    +    try (FileReader fileReader = new FileReader(file);
    +         JsonReader reader = Json.createReader(fileReader)) {
    +      profile = (JsonObject) reader.read();
    +    }
    +  }
    +
    +  public String getQuery( ) {
    +    return profile.get("query").toString();
    +  }
    +
    +  public String getPlan() {
    +    return profile.get("plan").toString();
    +  }
    +
    +  public List<String> getPlans() {
    +    if ( plans != null ) {
    +      return plans; }
    +    String plan = getPlan( );
    +    Pattern p = Pattern.compile( "(\\d\\d-\\d+[^\\\\]*)\\\\n", Pattern.MULTILINE );
    +    Matcher m = p.matcher(plan);
    +    plans = new ArrayList<>( );
    +    while ( m.find() ) {
    +      plans.add(m.group(1));
    +    }
    +    return plans;
    +  }
    +
    +  public String getScan( ) {
    +    int n = getPlans( ).size();
    +    Pattern p = Pattern.compile( "\\d+-\\d+\\s+(\\w+)\\(" );
    --- End diff --
    
    So we are just trying to find if this plan has Scan keyword after fragment number. Can't we simply use plan.contains("Scan") ? I guess the scan keyword will only be in plans which are actually Scan's.?
    
    Also there can be multiple Scans in the plan right ? In which case we should return list of plans here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96785415
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -49,6 +57,107 @@
     public class QueryBuilder {
     
       /**
    +   * Listener used to retrieve the query summary (only) asynchronously
    +   * using a {@link QuerySummaryFuture}.
    +   */
    +
    +  public class SummaryOnlyQueryEventListener implements UserResultsListener {
    +
    +    private final QuerySummaryFuture future;
    +    private QueryId queryId;
    +    private int recordCount;
    +    private int batchCount;
    +    private long startTime;
    +
    +    public SummaryOnlyQueryEventListener(QuerySummaryFuture future) {
    +      this.future = future;
    +      startTime = System.currentTimeMillis();
    +    }
    +
    +    @Override
    +    public void queryIdArrived(QueryId queryId) {
    +      this.queryId = queryId;
    +    }
    +
    +    @Override
    +    public void submissionFailed(UserException ex) {
    +      future.completed(new QuerySummary(queryId, recordCount, batchCount,
    +          System.currentTimeMillis() - startTime, ex));
    --- End diff --
    
    formatting


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #710: DRILL-5126: Provide simplified, unified "cluster fixture" ...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the issue:

    https://github.com/apache/drill/pull/710
  
    Rebased onto master. Pulled in additional changes from development branch that have accumulated in the three weeks that this PR has worked its way though the process. See the description in the third commit for details.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95053180
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java ---
    @@ -0,0 +1,251 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Properties;
    +
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +
    +/**
    + * Build a Drillbit and client with the options provided. The simplest
    + * builder starts an embedded Drillbit, with the "dfs_test" name space,
    + * a max width (parallelization) of 2.
    + */
    +
    +public class FixtureBuilder {
    +
    +  public static class RuntimeOption {
    +    public String key;
    +    public Object value;
    +
    +    public RuntimeOption(String key, Object value) {
    +      this.key = key;
    +      this.value = value;
    +    }
    +  }
    +
    +  // Values in the drill-module.conf file for values that are customized
    +  // in the defaults.
    +
    +  public static final int DEFAULT_ZK_REFRESH = 500; // ms
    +  public static final int DEFAULT_SERVER_RPC_THREADS = 10;
    +  public static final int DEFAULT_SCAN_THREADS = 8;
    +
    +  public static Properties defaultProps() {
    +    Properties props = new Properties();
    +    props.putAll(ClusterFixture.TEST_CONFIGURATIONS);
    +    return props;
    +  }
    +
    +  String configResource;
    +  Properties configProps;
    +  boolean enableFullCache;
    +  List<RuntimeOption> sessionOptions;
    +  List<RuntimeOption> systemOptions;
    +  int bitCount = 1;
    +  String bitNames[];
    +  int zkCount;
    +  ZookeeperHelper zkHelper;
    +
    +  /**
    +   * Use the given configuration properties to start the embedded Drillbit.
    +   * @param configProps a collection of config properties
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configProps(Properties configProps) {
    +    this.configProps = configProps;
    +    return this;
    +  }
    +
    +  /**
    +   * Use the given configuration file, stored as a resource, to start the
    +   * embedded Drillbit. Note that the resource file should have the two
    +   * following settings to work as a test:
    +   * <pre><code>
    +   * drill.exec.sys.store.provider.local.write : false,
    +   * drill.exec.http.enabled : false
    +   * </code></pre>
    +   * It may be more convenient to add your settings to the default
    +   * config settings with {@link #configProperty(String, Object)}.
    +   * @param configResource path to the file that contains the
    +   * config file to be read
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configResource(String configResource) {
    +
    +    // TypeSafe gets unhappy about a leading slash, but other functions
    +    // require it. Silently discard the leading slash if given to
    +    // preserve the test writer's sanity.
    +
    +    this.configResource = ClusterFixture.trimSlash(configResource);
    +    return this;
    +  }
    +
    +  /**
    +   * Add an additional boot-time property for the embedded Drillbit.
    +   * @param key config property name
    +   * @param value property value
    +   * @return this builder
    +   */
    +
    +  public FixtureBuilder configProperty(String key, Object value) {
    +    if (configProps == null) {
    +      configProps = defaultProps();
    +    }
    +    configProps.put(key, value.toString());
    +    return this;
    +  }
    +
    +   /**
    +   * Provide a session option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the session option
    +   * @param value the value of the session option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSession(String, Object)}
    +   */
    +
    +  public FixtureBuilder sessionOption(String key, Object value) {
    +    if (sessionOptions == null) {
    +      sessionOptions = new ArrayList<>();
    +    }
    +    sessionOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Provide a system option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the system option
    +   * @param value the value of the system option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSystem(String, Object)}
    +   */
    +
    +  public FixtureBuilder systemOption(String key, Object value) {
    +    if (systemOptions == null) {
    +      systemOptions = new ArrayList<>();
    +    }
    +    systemOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Set the maximum parallelization (max width per node). Defaults
    +   * to 2.
    +   *
    +   * @param n the "max width per node" parallelization option.
    +   * @return this builder
    +   */
    +  public FixtureBuilder maxParallelization(int n) {
    +    return sessionOption(ExecConstants.MAX_WIDTH_PER_NODE_KEY, n);
    +  }
    +
    +  public FixtureBuilder enableFullCache() {
    +    enableFullCache = true;
    +    return this;
    +  }
    +
    +  /**
    +   * The number of Drillbits to start in the cluster.
    +   *
    +   * @param n the desired cluster size
    +   * @return this builder
    +   */
    +  public FixtureBuilder clusterSize(int n) {
    +    bitCount = n;
    +    bitNames = null;
    +    return this;
    +  }
    +
    +  /**
    +   * Define a cluster by providing names to the Drillbits.
    +   * The cluster size is the same as the number of names provided.
    +   *
    +   * @param bitNames array of (unique) Drillbit names
    +   * @return this builder
    +   */
    +  public FixtureBuilder withBits(String bitNames[]) {
    +    this.bitNames = bitNames;
    +    bitCount = bitNames.length;
    +    return this;
    +  }
    +
    +  /**
    +   * By default the embedded Drillbits use an in-memory cluster coordinator.
    +   * Use this option to start an in-memory ZK instance to coordinate the
    +   * Drillbits.
    +   * @return this builder
    +   */
    +  public FixtureBuilder withZk() {
    +    return withZk(1);
    +  }
    +
    +  public FixtureBuilder withZk(int count) {
    +    zkCount = count;
    +
    +    // Using ZK. Turn refresh wait back on.
    +
    +    configProperty(ExecConstants.ZK_REFRESH, DEFAULT_ZK_REFRESH);
    +    return this;
    +  }
    +
    +  /**
    +   * Run the cluster using a Zookeeper started externally. Use this if
    +   * multiple tests start a cluster: allows ZK to be started once for
    +   * the entire suite rather than once per test case.
    +   *
    +   * @param zk the global Zookeeper to use
    +   * @return this builder
    +   */
    +  public FixtureBuilder withZk(ZookeeperHelper zk) {
    +    zkHelper = zk;
    +
    +    // Using ZK. Turn refresh wait back on.
    +
    +    configProperty(ExecConstants.ZK_REFRESH, DEFAULT_ZK_REFRESH);
    +    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>
    +   * FixtureBuilder builder = ClientFixture.newBuilder()
    --- End diff --
    
    Yes. A case of the comment not staying current with the code... Also revised the example to show that you have to create both the cluster and client in the current version of the code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95026609
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java ---
    @@ -0,0 +1,251 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Properties;
    +
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +
    +/**
    + * Build a Drillbit and client with the options provided. The simplest
    + * builder starts an embedded Drillbit, with the "dfs_test" name space,
    + * a max width (parallelization) of 2.
    + */
    +
    +public class FixtureBuilder {
    +
    +  public static class RuntimeOption {
    +    public String key;
    +    public Object value;
    +
    +    public RuntimeOption(String key, Object value) {
    +      this.key = key;
    +      this.value = value;
    +    }
    +  }
    +
    +  // Values in the drill-module.conf file for values that are customized
    +  // in the defaults.
    +
    +  public static final int DEFAULT_ZK_REFRESH = 500; // ms
    +  public static final int DEFAULT_SERVER_RPC_THREADS = 10;
    +  public static final int DEFAULT_SCAN_THREADS = 8;
    +
    +  public static Properties defaultProps() {
    +    Properties props = new Properties();
    +    props.putAll(ClusterFixture.TEST_CONFIGURATIONS);
    +    return props;
    +  }
    +
    +  String configResource;
    +  Properties configProps;
    +  boolean enableFullCache;
    +  List<RuntimeOption> sessionOptions;
    +  List<RuntimeOption> systemOptions;
    +  int bitCount = 1;
    +  String bitNames[];
    +  int zkCount;
    +  ZookeeperHelper zkHelper;
    +
    +  /**
    +   * Use the given configuration properties to start the embedded Drillbit.
    +   * @param configProps a collection of config properties
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configProps(Properties configProps) {
    +    this.configProps = configProps;
    +    return this;
    +  }
    +
    +  /**
    +   * Use the given configuration file, stored as a resource, to start the
    +   * embedded Drillbit. Note that the resource file should have the two
    +   * following settings to work as a test:
    +   * <pre><code>
    +   * drill.exec.sys.store.provider.local.write : false,
    +   * drill.exec.http.enabled : false
    +   * </code></pre>
    +   * It may be more convenient to add your settings to the default
    +   * config settings with {@link #configProperty(String, Object)}.
    +   * @param configResource path to the file that contains the
    +   * config file to be read
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configResource(String configResource) {
    +
    +    // TypeSafe gets unhappy about a leading slash, but other functions
    +    // require it. Silently discard the leading slash if given to
    +    // preserve the test writer's sanity.
    +
    +    this.configResource = ClusterFixture.trimSlash(configResource);
    +    return this;
    +  }
    +
    +  /**
    +   * Add an additional boot-time property for the embedded Drillbit.
    +   * @param key config property name
    +   * @param value property value
    +   * @return this builder
    +   */
    +
    +  public FixtureBuilder configProperty(String key, Object value) {
    +    if (configProps == null) {
    +      configProps = defaultProps();
    +    }
    +    configProps.put(key, value.toString());
    +    return this;
    +  }
    +
    +   /**
    +   * Provide a session option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the session option
    +   * @param value the value of the session option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSession(String, Object)}
    +   */
    +
    +  public FixtureBuilder sessionOption(String key, Object value) {
    +    if (sessionOptions == null) {
    +      sessionOptions = new ArrayList<>();
    +    }
    +    sessionOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Provide a system option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the system option
    +   * @param value the value of the system option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSystem(String, Object)}
    +   */
    +
    +  public FixtureBuilder systemOption(String key, Object value) {
    +    if (systemOptions == null) {
    +      systemOptions = new ArrayList<>();
    +    }
    +    systemOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Set the maximum parallelization (max width per node). Defaults
    +   * to 2.
    +   *
    +   * @param n the "max width per node" parallelization option.
    +   * @return this builder
    +   */
    +  public FixtureBuilder maxParallelization(int n) {
    +    return sessionOption(ExecConstants.MAX_WIDTH_PER_NODE_KEY, n);
    +  }
    +
    +  public FixtureBuilder enableFullCache() {
    +    enableFullCache = true;
    +    return this;
    +  }
    +
    +  /**
    +   * The number of Drillbits to start in the cluster.
    +   *
    +   * @param n the desired cluster size
    +   * @return this builder
    +   */
    +  public FixtureBuilder clusterSize(int n) {
    +    bitCount = n;
    +    bitNames = null;
    +    return this;
    +  }
    +
    +  /**
    +   * Define a cluster by providing names to the Drillbits.
    +   * The cluster size is the same as the number of names provided.
    +   *
    +   * @param bitNames array of (unique) Drillbit names
    +   * @return this builder
    +   */
    +  public FixtureBuilder withBits(String bitNames[]) {
    +    this.bitNames = bitNames;
    +    bitCount = bitNames.length;
    +    return this;
    +  }
    +
    +  /**
    +   * By default the embedded Drillbits use an in-memory cluster coordinator.
    +   * Use this option to start an in-memory ZK instance to coordinate the
    +   * Drillbits.
    +   * @return this builder
    +   */
    +  public FixtureBuilder withZk() {
    +    return withZk(1);
    +  }
    +
    +  public FixtureBuilder withZk(int count) {
    +    zkCount = count;
    +
    +    // Using ZK. Turn refresh wait back on.
    +
    +    configProperty(ExecConstants.ZK_REFRESH, DEFAULT_ZK_REFRESH);
    +    return this;
    +  }
    +
    +  /**
    +   * Run the cluster using a Zookeeper started externally. Use this if
    +   * multiple tests start a cluster: allows ZK to be started once for
    +   * the entire suite rather than once per test case.
    +   *
    +   * @param zk the global Zookeeper to use
    +   * @return this builder
    +   */
    +  public FixtureBuilder withZk(ZookeeperHelper zk) {
    +    zkHelper = zk;
    +
    +    // Using ZK. Turn refresh wait back on.
    +
    +    configProperty(ExecConstants.ZK_REFRESH, DEFAULT_ZK_REFRESH);
    +    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>
    +   * FixtureBuilder builder = ClientFixture.newBuilder()
    --- End diff --
    
    Should be clusterFixture ? Please correct below as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95053415
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -0,0 +1,314 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.List;
    +
    +import org.apache.drill.PlanTestBase;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.expression.SchemaPath;
    +import org.apache.drill.exec.client.PrintingResultsListener;
    +import org.apache.drill.exec.client.QuerySubmitter.Format;
    +import org.apache.drill.exec.proto.UserBitShared.QueryId;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.proto.helper.QueryIdHelper;
    +import org.apache.drill.exec.record.RecordBatchLoader;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.rpc.RpcException;
    +import org.apache.drill.exec.rpc.user.AwaitableUserResultsListener;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.rpc.user.UserResultsListener;
    +import org.apache.drill.exec.util.VectorUtil;
    +import org.apache.drill.exec.vector.NullableVarCharVector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.test.BufferingQueryEventListener.QueryEvent;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Builder for a Drill query. Provides all types of query formats,
    + * and a variety of ways to run the query.
    + */
    +
    +public class QueryBuilder {
    +
    +  /**
    +   * Summary results of a query: records, batches, run time.
    +   */
    +
    +  public static class QuerySummary {
    +    private final QueryId queryId;
    +    private final int records;
    +    private final int batches;
    +    private final long ms;
    +
    +    public QuerySummary(QueryId queryId, int recordCount, int batchCount, long elapsed) {
    +      this.queryId = queryId;
    +      records = recordCount;
    +      batches = batchCount;
    +      ms = elapsed;
    +    }
    +
    +    public long recordCount( ) { return records; }
    +    public int batchCount( ) { return batches; }
    +    public long runTimeMs( ) { return ms; }
    +    public QueryId queryId( ) { return queryId; }
    +    public String queryIdString( ) { return QueryIdHelper.getQueryId(queryId); }
    +
    +  }
    +
    +  private final ClientFixture client;
    +  private QueryType queryType;
    +  private String queryText;
    +
    +  QueryBuilder(ClientFixture client) {
    +    this.client = client;
    +  }
    +
    +  public QueryBuilder query(QueryType type, String text) {
    +    queryType = type;
    +    queryText = text;
    +    return this;
    +  }
    +
    +  public QueryBuilder sql(String sql) {
    +    return query( QueryType.SQL, sql );
    +  }
    +
    +  public QueryBuilder sql(String query, Object... args) {
    +    return sql(String.format(query, args));
    +  }
    +
    +  public QueryBuilder physical(String plan) {
    +    return query( QueryType.PHYSICAL, plan);
    +  }
    +
    +  public QueryBuilder sqlResource(String resource) {
    +    sql(ClusterFixture.loadResource(resource));
    +    return this;
    +  }
    +
    +  public QueryBuilder sqlResource(String resource, Object... args) {
    +    sql(ClusterFixture.loadResource(resource), args);
    +    return this;
    +  }
    +
    +  public QueryBuilder physicalResource(String resource) {
    +    physical(ClusterFixture.loadResource(resource));
    +    return this;
    +  }
    +
    +  /**
    +   * Run the query returning just a summary of the results: record count,
    +   * batch count and run time. Handy when doing performance tests when the
    +   * validity of the results is verified in some other test.
    +   *
    +   * @return the query summary
    +   */
    +
    +  public QuerySummary run() {
    +    return produceSummary(withEventListener());
    +  }
    +
    +  /**
    +   * Run the query and return a list of the result batches. Use
    +   * if the batch count is small and you want to work with them.
    +   * @return a list of batches resulting from the query
    +   * @throws RpcException
    +   */
    +
    +  public List<QueryDataBatch> results() throws RpcException {
    +    Preconditions.checkNotNull(queryType, "Query not provided.");
    +    Preconditions.checkNotNull(queryText, "Query not provided.");
    +    return client.client().runQuery(queryType, queryText);
    +  }
    +
    +  /**
    +   * Run the query with the listener provided. Use when the result
    +   * count will be large, or you don't need the results.
    +   *
    +   * @param listener the Drill listener
    +   */
    +
    +  public void withListener(UserResultsListener listener) {
    +    Preconditions.checkNotNull(queryType, "Query not provided.");
    +    Preconditions.checkNotNull(queryText, "Query not provided.");
    +    client.client().runQuery(queryType, queryText, listener);
    +  }
    +
    +  /**
    +   * Run the query, return an easy-to-use event listener to process
    +   * the query results. Use when the result set is large. The listener
    +   * allows the caller to iterate over results in the test thread.
    +   * (The listener implements a producer-consumer model to hide the
    +   * details of Drill listeners.)
    +   *
    +   * @return the query event listener
    +   */
    +
    +  public BufferingQueryEventListener withEventListener( ) {
    +    BufferingQueryEventListener listener = new BufferingQueryEventListener( );
    +    withListener(listener);
    +    return listener;
    +  }
    +
    +  public long printCsv() {
    +    return print(Format.CSV);
    +  }
    +
    +  public long print( Format format ) {
    +    return print(format,20);
    +  }
    +
    +  public long print(Format format, int colWidth) {
    +    return runAndWait( new PrintingResultsListener( client.cluster().config( ), format, colWidth ) );
    +  }
    +
    +  /**
    +   * Run a query and optionally print the output in TSV format.
    +   * Similar to {@link QueryTestUtil#test} with one query. Output is printed
    +   * only if the tests are running as verbose.
    +   *
    +   * @return the number of rows returned
    +   */
    +  public long print() {
    +    DrillConfig config = client.cluster().config( );
    +
    +    // Note: verbose check disabled until that change is
    +    // committed.
    +
    +    boolean verbose = ! config.getBoolean(QueryTestUtil.TEST_QUERY_PRINTING_SILENT) /* ||
    +                      DrillTest.verbose() */;
    +    if (verbose) {
    +      return print(Format.TSV, VectorUtil.DEFAULT_COLUMN_WIDTH);
    +    } else {
    +      return run().recordCount();
    +    }
    +  }
    +
    +  public long runAndWait(UserResultsListener listener) {
    +    AwaitableUserResultsListener resultListener =
    +        new AwaitableUserResultsListener(listener);
    +    withListener( resultListener );
    +    try {
    +      return resultListener.await();
    +    } catch (Exception e) {
    +      throw new IllegalStateException(e);
    +    }
    +  }
    +
    +  /**
    +   * Submit an "EXPLAIN" statement, and return text form of the
    +   * plan.
    +   * @throws Exception if the query fails
    +   */
    +
    +  public String explainText() throws Exception {
    +    return explain(ClusterFixture.EXPLAIN_PLAN_TEXT);
    +  }
    +
    +  /**
    +   * Submit an "EXPLAIN" statement, and return the JSON form of the
    +   * plan.
    +   * @throws Exception if the query fails
    +   */
    +
    +  public String explainJson() throws Exception {
    +    return explain(ClusterFixture.EXPLAIN_PLAN_JSON);
    +  }
    +
    +  public String explain(String format) throws Exception {
    +    queryText = "EXPLAIN PLAN FOR " + queryText;
    +    return queryPlan(format);
    +  }
    +
    +  private QuerySummary produceSummary(BufferingQueryEventListener listener) {
    +    long start = System.currentTimeMillis();
    +    int recordCount = 0;
    +    int batchCount = 0;
    +    QueryId queryId = null;
    +    loop:
    +    for ( ; ; ) {
    +      QueryEvent event = listener.get();
    --- End diff --
    
    Really? The listener handles errors as follows:
    ```
      @Override
      public void submissionFailed(UserException ex) {
        silentPut( new QueryEvent( ex ) );
      }
    ```
    
    That is, when an error arrives from the server, we create an event and put it into the event queue, later to be retrieved by the above get. Perhaps show me the error case you discovered as I'm likely missing something here.
    
    I've seen other tests where any failure is delivered using the `submissionFailed` call...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95053052
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    +          name = DEFAULT_BIT_NAME;
    +        } else {
    +          name = DEFAULT_BIT_NAME + Integer.toString(i+1);
    +        }
    +      }
    +      bits.put(name, bit);
    +    }
    +
    +    // Some operations need an allocator.
    +
    +    allocator = RootAllocatorFactory.newRoot(config);
    +
    +    // Apply system options
    +    if ( builder.systemOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.systemOptions ) {
    +        clientFixture( ).alterSystem( option.key, option.value );
    +      }
    +    }
    +    // Apply session options.
    +
    +    boolean sawMaxWidth = false;
    +    if ( builder.sessionOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.sessionOptions ) {
    +        clientFixture( ).alterSession( option.key, option.value );
    +        if ( option.key.equals( ExecConstants.MAX_WIDTH_PER_NODE_KEY ) ) {
    +          sawMaxWidth = true;
    +        }
    +      }
    +    }
    +
    +    // Set the default parallelization unless already set by the caller.
    +
    +    if ( ! sawMaxWidth ) {
    +      clientFixture( ).alterSession( ExecConstants.MAX_WIDTH_PER_NODE_KEY, MAX_WIDTH_PER_NODE );
    +    }
    +  }
    +
    +  private Properties configProperties(Properties configProps) {
    +    Properties effectiveProps = new Properties( );
    +    for (Entry<Object, Object> entry : configProps.entrySet()) {
    +      effectiveProps.put(entry.getKey(), entry.getValue().toString());
    +    }
    +    if (zkHelper != null) {
    +      effectiveProps.put(ExecConstants.ZK_CONNECTION, zkHelper.getConfig().getString(ExecConstants.ZK_CONNECTION));
    +    }
    +    return effectiveProps;
    +  }
    +
    +  public Drillbit drillbit( ) { return bits.get(DEFAULT_BIT_NAME); }
    +  public Drillbit drillbit(String name) { return bits.get(name); }
    +  public Collection<Drillbit> drillbits( ) { return bits.values(); }
    +  public RemoteServiceSet serviceSet( ) { return serviceSet; }
    +  public BufferAllocator allocator( ) { return allocator; }
    +  public DrillConfig config() { return config; }
    +
    +  public ClientFixture.ClientBuilder clientBuilder( ) {
    +    return new ClientFixture.ClientBuilder(this);
    +  }
    +
    +  public ClientFixture clientFixture() {
    +    if ( clients.isEmpty( ) ) {
    +      clientBuilder( ).build( );
    +    }
    +    return clients.get(0);
    +  }
    +
    +  public DrillClient client() {
    +    return clientFixture().client();
    +  }
    +
    +  @Override
    +  public void close() throws Exception {
    +    Exception ex = null;
    +
    +    // Close clients. Clients remove themselves from the client
    +    // list.
    +
    +    while (! clients.isEmpty( )) {
    +      ex = safeClose(clients.get(0), ex);
    +    }
    +
    +    for (Drillbit bit : drillbits()) {
    --- End diff --
    
    Because I really do want to throw the first exception (if any) encountered during the close. Such an exception might be an indication that something is wrong. There have been cases where we silently ignore such errors leading to bugs going uncaught. Some recent JIRAs are of that nature. As far as I can tell, AutoCloseables.close just ignores the exception, or chains it to an already-created exception which is just a nuisance.
    
    There is a Jira saying that Drill abuses Autocloseables. As part of fixing that, perhaps we can move the functionality here into the new DrillCloseables class to get the "throw the first exception, if any" behavior.
    
    Added a comment explaining most of this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95036224
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    +          name = DEFAULT_BIT_NAME;
    +        } else {
    +          name = DEFAULT_BIT_NAME + Integer.toString(i+1);
    +        }
    +      }
    +      bits.put(name, bit);
    +    }
    +
    +    // Some operations need an allocator.
    +
    +    allocator = RootAllocatorFactory.newRoot(config);
    +
    +    // Apply system options
    +    if ( builder.systemOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.systemOptions ) {
    +        clientFixture( ).alterSystem( option.key, option.value );
    +      }
    +    }
    +    // Apply session options.
    +
    +    boolean sawMaxWidth = false;
    +    if ( builder.sessionOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.sessionOptions ) {
    +        clientFixture( ).alterSession( option.key, option.value );
    +        if ( option.key.equals( ExecConstants.MAX_WIDTH_PER_NODE_KEY ) ) {
    +          sawMaxWidth = true;
    +        }
    +      }
    +    }
    +
    +    // Set the default parallelization unless already set by the caller.
    +
    +    if ( ! sawMaxWidth ) {
    +      clientFixture( ).alterSession( ExecConstants.MAX_WIDTH_PER_NODE_KEY, MAX_WIDTH_PER_NODE );
    +    }
    +  }
    +
    +  private Properties configProperties(Properties configProps) {
    +    Properties effectiveProps = new Properties( );
    +    for (Entry<Object, Object> entry : configProps.entrySet()) {
    +      effectiveProps.put(entry.getKey(), entry.getValue().toString());
    +    }
    +    if (zkHelper != null) {
    +      effectiveProps.put(ExecConstants.ZK_CONNECTION, zkHelper.getConfig().getString(ExecConstants.ZK_CONNECTION));
    +    }
    +    return effectiveProps;
    +  }
    +
    +  public Drillbit drillbit( ) { return bits.get(DEFAULT_BIT_NAME); }
    --- End diff --
    
    if we change the above "else" condition while inserting into map then here we can just return the first entry.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #710: DRILL-5126: Provide simplified, unified "cluster fixture" ...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the issue:

    https://github.com/apache/drill/pull/710
  
    Rebased on master, resolved conflict and squashed commits.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95052969
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    +          name = DEFAULT_BIT_NAME;
    +        } else {
    +          name = DEFAULT_BIT_NAME + Integer.toString(i+1);
    +        }
    +      }
    +      bits.put(name, bit);
    +    }
    +
    +    // Some operations need an allocator.
    +
    +    allocator = RootAllocatorFactory.newRoot(config);
    +
    +    // Apply system options
    +    if ( builder.systemOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.systemOptions ) {
    +        clientFixture( ).alterSystem( option.key, option.value );
    +      }
    +    }
    +    // Apply session options.
    +
    +    boolean sawMaxWidth = false;
    +    if ( builder.sessionOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.sessionOptions ) {
    +        clientFixture( ).alterSession( option.key, option.value );
    +        if ( option.key.equals( ExecConstants.MAX_WIDTH_PER_NODE_KEY ) ) {
    +          sawMaxWidth = true;
    +        }
    +      }
    +    }
    +
    +    // Set the default parallelization unless already set by the caller.
    +
    +    if ( ! sawMaxWidth ) {
    +      clientFixture( ).alterSession( ExecConstants.MAX_WIDTH_PER_NODE_KEY, MAX_WIDTH_PER_NODE );
    +    }
    +  }
    +
    +  private Properties configProperties(Properties configProps) {
    +    Properties effectiveProps = new Properties( );
    +    for (Entry<Object, Object> entry : configProps.entrySet()) {
    +      effectiveProps.put(entry.getKey(), entry.getValue().toString());
    +    }
    +    if (zkHelper != null) {
    +      effectiveProps.put(ExecConstants.ZK_CONNECTION, zkHelper.getConfig().getString(ExecConstants.ZK_CONNECTION));
    +    }
    +    return effectiveProps;
    +  }
    +
    +  public Drillbit drillbit( ) { return bits.get(DEFAULT_BIT_NAME); }
    --- End diff --
    
    Yes, it is a bit messy. Changed this a different way by defining a "default drillbit" that is returned by this method. That way, we don't rely on the name. (Which, when bits are named by the caller, we don't know anyway.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95019120
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java ---
    @@ -0,0 +1,251 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Properties;
    +
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +
    +/**
    + * Build a Drillbit and client with the options provided. The simplest
    + * builder starts an embedded Drillbit, with the "dfs_test" name space,
    + * a max width (parallelization) of 2.
    + */
    +
    +public class FixtureBuilder {
    +
    +  public static class RuntimeOption {
    +    public String key;
    +    public Object value;
    +
    +    public RuntimeOption(String key, Object value) {
    +      this.key = key;
    +      this.value = value;
    +    }
    +  }
    +
    +  // Values in the drill-module.conf file for values that are customized
    +  // in the defaults.
    +
    +  public static final int DEFAULT_ZK_REFRESH = 500; // ms
    +  public static final int DEFAULT_SERVER_RPC_THREADS = 10;
    +  public static final int DEFAULT_SCAN_THREADS = 8;
    +
    +  public static Properties defaultProps() {
    +    Properties props = new Properties();
    +    props.putAll(ClusterFixture.TEST_CONFIGURATIONS);
    +    return props;
    +  }
    +
    +  String configResource;
    +  Properties configProps;
    +  boolean enableFullCache;
    +  List<RuntimeOption> sessionOptions;
    +  List<RuntimeOption> systemOptions;
    +  int bitCount = 1;
    +  String bitNames[];
    +  int zkCount;
    +  ZookeeperHelper zkHelper;
    +
    +  /**
    +   * Use the given configuration properties to start the embedded Drillbit.
    +   * @param configProps a collection of config properties
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configProps(Properties configProps) {
    +    this.configProps = configProps;
    +    return this;
    +  }
    +
    +  /**
    +   * Use the given configuration file, stored as a resource, to start the
    +   * embedded Drillbit. Note that the resource file should have the two
    +   * following settings to work as a test:
    +   * <pre><code>
    +   * drill.exec.sys.store.provider.local.write : false,
    +   * drill.exec.http.enabled : false
    +   * </code></pre>
    +   * It may be more convenient to add your settings to the default
    +   * config settings with {@link #configProperty(String, Object)}.
    +   * @param configResource path to the file that contains the
    +   * config file to be read
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configResource(String configResource) {
    +
    +    // TypeSafe gets unhappy about a leading slash, but other functions
    +    // require it. Silently discard the leading slash if given to
    +    // preserve the test writer's sanity.
    +
    +    this.configResource = ClusterFixture.trimSlash(configResource);
    +    return this;
    +  }
    +
    +  /**
    +   * Add an additional boot-time property for the embedded Drillbit.
    +   * @param key config property name
    +   * @param value property value
    +   * @return this builder
    +   */
    +
    +  public FixtureBuilder configProperty(String key, Object value) {
    +    if (configProps == null) {
    +      configProps = defaultProps();
    +    }
    +    configProps.put(key, value.toString());
    +    return this;
    +  }
    +
    +   /**
    +   * Provide a session option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the session option
    +   * @param value the value of the session option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSession(String, Object)}
    +   */
    +
    +  public FixtureBuilder sessionOption(String key, Object value) {
    +    if (sessionOptions == null) {
    +      sessionOptions = new ArrayList<>();
    +    }
    +    sessionOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Provide a system option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the system option
    +   * @param value the value of the system option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSystem(String, Object)}
    +   */
    +
    +  public FixtureBuilder systemOption(String key, Object value) {
    +    if (systemOptions == null) {
    +      systemOptions = new ArrayList<>();
    +    }
    +    systemOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Set the maximum parallelization (max width per node). Defaults
    +   * to 2.
    +   *
    +   * @param n the "max width per node" parallelization option.
    +   * @return this builder
    +   */
    +  public FixtureBuilder maxParallelization(int n) {
    +    return sessionOption(ExecConstants.MAX_WIDTH_PER_NODE_KEY, n);
    --- End diff --
    
    you should return Builder here and set the sessionOption ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96924435
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/LogFixture.java ---
    @@ -0,0 +1,255 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.test;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import org.slf4j.LoggerFactory;
    +
    +import ch.qos.logback.classic.Level;
    +import ch.qos.logback.classic.Logger;
    +import ch.qos.logback.classic.LoggerContext;
    +import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
    +import ch.qos.logback.classic.spi.ILoggingEvent;
    +import ch.qos.logback.core.ConsoleAppender;
    +
    +/**
    + * Establishes test-specific logging without having to alter the global
    + * <tt>logback-test.xml</tt> file. Allows directing output to the console
    + * (if not already configured) and setting the log level on specific loggers
    + * of interest in the test. The fixture automatically restores the original
    + * log configuration on exit.
    + * <p>
    + * Typical usage: <pre><code>
    + * {@literal @}Test
    + * public void myTest() {
    + *   LogFixtureBuilder logBuilder = LogFixture.builder()
    + *          .toConsole()
    + *          .disable() // Silence all other loggers
    + *          .logger(ExternalSortBatch.class, Level.DEBUG);
    + *   try (LogFixture logs = logBuilder.build()) {
    + *     // Test code here
    + *   }
    + * }</code></pre>
    + *  <p>
    + * You can &ndash; and should &ndash; combine the log fixtue with the
    + * cluster and client fixtures to have complete control over your test-time
    + * Drill environment.
    + */
    +
    +public class LogFixture implements AutoCloseable {
    +
    +  // Elapsed time in ms, log level, thread, logger, message.
    +
    +  public static final String DEFAULT_CONSOLE_FORMAT = "%r %level [%thread] [%logger] - %msg%n";
    +  private static final String DRILL_PACKAGE_NAME = "org.apache.drill";
    +
    +  /**
    +   * Memento for a logger name and level.
    +   */
    +  public static class LogSpec {
    +    String loggerName;
    +    Level logLevel;
    +
    +    public LogSpec(String loggerName, Level level) {
    +      this.loggerName = loggerName;
    +      this.logLevel = level;
    +    }
    +  }
    +
    +  /**
    +   * Builds the log settings to be used for a test. The log settings here
    +   * add to those specified in a <tt>logback.xml</tt> or
    +   * <tt>logback-test.xml</tt> file on your class path. In particular, if
    +   * the logging configuration already redirects the Drill logger to the
    +   * console, setting console logging here does nothing.
    +   */
    +
    +  public static class LogFixtureBuilder {
    +
    +    private String consoleFormat = DEFAULT_CONSOLE_FORMAT;
    +    private boolean logToConsole;
    +    private List<LogSpec> loggers = new ArrayList<>();
    +
    +    /**
    +     * Send all enabled logging to the console (if not already configured.) Some
    +     * Drill log configuration files send the root to the console (or file), but
    +     * the Drill loggers to Lilith. In that case, Lilith "hides" the console
    +     * logger. Using this call adds a console logger to the Drill logger so that
    +     * output does, in fact, go to the console regardless of the configuration
    +     * in the Logback configuration file.
    +     *
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder toConsole() {
    +      logToConsole = true;
    +      return this;
    +    }
    +
    +    /**
    +     * Send logging to the console using the defined format.
    +     *
    +     * @param format valid Logback log format
    +     * @return this builder
    +     */
    +
    +    public LogFixtureBuilder toConsole(String format) {
    +      consoleFormat = format;
    +      return toConsole();
    +    }
    +
    +    /**
    +     * Set a specific logger to the given level.
    +     *
    +     * @param loggerName name of the logger (typically used for package-level
    +     * loggers)
    +     * @param level the desired Logback-defined level
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder logger(String loggerName, Level level) {
    +      loggers.add(new LogSpec(loggerName, level));
    +      return this;
    +    }
    +
    +    /**
    +     * Set a specific logger to the given level.
    +     *
    +     * @param loggerClass class that defines the logger (typically used for
    +     * class-specific loggers)
    +     * @param level the desired Logback-defined level
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder logger(Class<?> loggerClass, Level level) {
    +      loggers.add(new LogSpec(loggerClass.getName(), level));
    +      return this;
    +    }
    +
    +    /**
    +     * Turns off all logging. If called first, you can set disable as your
    +     * general policy, then turn back on loggers selectively for those
    +     * of interest.
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder disable() {
    +      return rootLogger(Level.OFF);
    +    }
    +
    +    /**
    +     * Set the desired log level on the root logger.
    +     * @param level the desired Logback log level
    +     * @return this builder
    +     */
    +
    +    public LogFixtureBuilder rootLogger(Level level) {
    +      loggers.add(new LogSpec(Logger.ROOT_LOGGER_NAME, level));
    +      return this;
    +    }
    +
    +    /**
    +     * Apply the log levels and output, then return a fixture to be used
    +     * in a try-with-resources block. The fixture automatically restores
    +     * the original configuration on completion of the try block.
    +     * @return the log fixture
    +     */
    +    public LogFixture build() {
    +      return new LogFixture(this);
    +    }
    +  }
    +
    +  private PatternLayoutEncoder ple;
    +  private ConsoleAppender<ILoggingEvent> appender;
    +  private List<LogSpec> loggers = new ArrayList<>();
    +  private Logger drillLogger;
    +
    +  public LogFixture(LogFixtureBuilder builder) {
    +    if (builder.logToConsole) {
    +      setupConsole(builder);
    +    }
    +    setupLoggers(builder);
    +  }
    +
    +  /**
    +   * Creates a new log fixture builder.
    +   * @return the log fixture builder
    +   */
    +
    +  public static LogFixtureBuilder builder() {
    +    return new LogFixtureBuilder();
    +  }
    +
    +  private void setupConsole(LogFixtureBuilder builder) {
    +    Logger drillLogger = (Logger)LoggerFactory.getLogger(DRILL_PACKAGE_NAME);
    +    if (drillLogger.getAppender("STDOUT") != null) {
    +      return;
    +    }
    +    LoggerContext lc = (LoggerContext) LoggerFactory.getILoggerFactory();
    +    ple = new PatternLayoutEncoder();
    +    ple.setPattern(builder.consoleFormat);
    +    ple.setContext(lc);
    +    ple.start();
    +
    +    appender = new ConsoleAppender<>( );
    +    appender.setContext(lc);
    +    appender.setName("Console");
    +    appender.setEncoder( ple );
    +    appender.start();
    +
    +    Logger root = (Logger)LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
    --- End diff --
    
    As I understand the semantics, each logger can have multiple appenders. We add ours to any that exist. When we remove ours, we only remove that one, not any others that might be registered. Of course, I still might be missing something...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95053125
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FieldDef.java ---
    @@ -0,0 +1,74 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.regex.Matcher;
    +import java.util.regex.Pattern;
    +
    +/**
    + * Basic representation of a column parsed from a query profile.
    + * Idea is to use this to generate mock data that represents a
    + * query obtained from a user. This is a work in progress.
    + */
    +
    +public class FieldDef {
    +  public enum Type { VARCHAR, DOUBLE };
    +  public enum TypeHint { DATE, TIME };
    +
    +  public final String name;
    +  public final String typeStr;
    +  public final Type type;
    +  public int length;
    +  public TypeHint hint;
    +
    +  public FieldDef( String name, String typeStr ) {
    +    this.name = name;
    +    this.typeStr = typeStr;
    +    Pattern p = Pattern.compile( "(\\w+)(?:\\((\\d+)\\))?" );
    +    Matcher m = p.matcher(typeStr);
    +    if ( ! m.matches() ) { throw new IllegalStateException( ); }
    +    if ( m.group(2) == null ) {
    +      length = 0;
    +    } else {
    +      length = Integer.parseInt( m.group(2) );
    --- End diff --
    
    Done. There is much to do still here. This is a work in progress that needs expansion as we use it for more test cases.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96739079
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -0,0 +1,314 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.List;
    +
    +import org.apache.drill.PlanTestBase;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.expression.SchemaPath;
    +import org.apache.drill.exec.client.PrintingResultsListener;
    +import org.apache.drill.exec.client.QuerySubmitter.Format;
    +import org.apache.drill.exec.proto.UserBitShared.QueryId;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.proto.helper.QueryIdHelper;
    +import org.apache.drill.exec.record.RecordBatchLoader;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.rpc.RpcException;
    +import org.apache.drill.exec.rpc.user.AwaitableUserResultsListener;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.rpc.user.UserResultsListener;
    +import org.apache.drill.exec.util.VectorUtil;
    +import org.apache.drill.exec.vector.NullableVarCharVector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.test.BufferingQueryEventListener.QueryEvent;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Builder for a Drill query. Provides all types of query formats,
    + * and a variety of ways to run the query.
    + */
    +
    +public class QueryBuilder {
    +
    +  /**
    +   * Summary results of a query: records, batches, run time.
    +   */
    +
    +  public static class QuerySummary {
    +    private final QueryId queryId;
    +    private final int records;
    +    private final int batches;
    +    private final long ms;
    +
    +    public QuerySummary(QueryId queryId, int recordCount, int batchCount, long elapsed) {
    +      this.queryId = queryId;
    +      records = recordCount;
    +      batches = batchCount;
    +      ms = elapsed;
    +    }
    +
    +    public long recordCount( ) { return records; }
    +    public int batchCount( ) { return batches; }
    +    public long runTimeMs( ) { return ms; }
    +    public QueryId queryId( ) { return queryId; }
    +    public String queryIdString( ) { return QueryIdHelper.getQueryId(queryId); }
    +
    +  }
    +
    +  private final ClientFixture client;
    +  private QueryType queryType;
    +  private String queryText;
    +
    +  QueryBuilder(ClientFixture client) {
    +    this.client = client;
    +  }
    +
    +  public QueryBuilder query(QueryType type, String text) {
    +    queryType = type;
    +    queryText = text;
    +    return this;
    +  }
    +
    +  public QueryBuilder sql(String sql) {
    +    return query( QueryType.SQL, sql );
    +  }
    +
    +  public QueryBuilder sql(String query, Object... args) {
    +    return sql(String.format(query, args));
    +  }
    +
    +  public QueryBuilder physical(String plan) {
    +    return query( QueryType.PHYSICAL, plan);
    +  }
    +
    +  public QueryBuilder sqlResource(String resource) {
    +    sql(ClusterFixture.loadResource(resource));
    +    return this;
    +  }
    +
    +  public QueryBuilder sqlResource(String resource, Object... args) {
    +    sql(ClusterFixture.loadResource(resource), args);
    +    return this;
    +  }
    +
    +  public QueryBuilder physicalResource(String resource) {
    +    physical(ClusterFixture.loadResource(resource));
    +    return this;
    +  }
    +
    +  /**
    +   * Run the query returning just a summary of the results: record count,
    +   * batch count and run time. Handy when doing performance tests when the
    +   * validity of the results is verified in some other test.
    +   *
    +   * @return the query summary
    +   */
    +
    +  public QuerySummary run() {
    +    return produceSummary(withEventListener());
    +  }
    +
    +  /**
    +   * Run the query and return a list of the result batches. Use
    +   * if the batch count is small and you want to work with them.
    +   * @return a list of batches resulting from the query
    +   * @throws RpcException
    +   */
    +
    +  public List<QueryDataBatch> results() throws RpcException {
    +    Preconditions.checkNotNull(queryType, "Query not provided.");
    +    Preconditions.checkNotNull(queryText, "Query not provided.");
    +    return client.client().runQuery(queryType, queryText);
    +  }
    +
    +  /**
    +   * Run the query with the listener provided. Use when the result
    +   * count will be large, or you don't need the results.
    +   *
    +   * @param listener the Drill listener
    +   */
    +
    +  public void withListener(UserResultsListener listener) {
    +    Preconditions.checkNotNull(queryType, "Query not provided.");
    +    Preconditions.checkNotNull(queryText, "Query not provided.");
    +    client.client().runQuery(queryType, queryText, listener);
    +  }
    +
    +  /**
    +   * Run the query, return an easy-to-use event listener to process
    +   * the query results. Use when the result set is large. The listener
    +   * allows the caller to iterate over results in the test thread.
    +   * (The listener implements a producer-consumer model to hide the
    +   * details of Drill listeners.)
    +   *
    +   * @return the query event listener
    +   */
    +
    +  public BufferingQueryEventListener withEventListener( ) {
    +    BufferingQueryEventListener listener = new BufferingQueryEventListener( );
    +    withListener(listener);
    +    return listener;
    +  }
    +
    +  public long printCsv() {
    +    return print(Format.CSV);
    +  }
    +
    +  public long print( Format format ) {
    +    return print(format,20);
    +  }
    +
    +  public long print(Format format, int colWidth) {
    +    return runAndWait( new PrintingResultsListener( client.cluster().config( ), format, colWidth ) );
    +  }
    +
    +  /**
    +   * Run a query and optionally print the output in TSV format.
    +   * Similar to {@link QueryTestUtil#test} with one query. Output is printed
    +   * only if the tests are running as verbose.
    +   *
    +   * @return the number of rows returned
    +   */
    +  public long print() {
    +    DrillConfig config = client.cluster().config( );
    +
    +    // Note: verbose check disabled until that change is
    +    // committed.
    +
    +    boolean verbose = ! config.getBoolean(QueryTestUtil.TEST_QUERY_PRINTING_SILENT) /* ||
    +                      DrillTest.verbose() */;
    +    if (verbose) {
    +      return print(Format.TSV, VectorUtil.DEFAULT_COLUMN_WIDTH);
    +    } else {
    +      return run().recordCount();
    +    }
    +  }
    +
    +  public long runAndWait(UserResultsListener listener) {
    +    AwaitableUserResultsListener resultListener =
    +        new AwaitableUserResultsListener(listener);
    +    withListener( resultListener );
    +    try {
    +      return resultListener.await();
    +    } catch (Exception e) {
    +      throw new IllegalStateException(e);
    +    }
    +  }
    +
    +  /**
    +   * Submit an "EXPLAIN" statement, and return text form of the
    +   * plan.
    +   * @throws Exception if the query fails
    +   */
    +
    +  public String explainText() throws Exception {
    +    return explain(ClusterFixture.EXPLAIN_PLAN_TEXT);
    +  }
    +
    +  /**
    +   * Submit an "EXPLAIN" statement, and return the JSON form of the
    +   * plan.
    +   * @throws Exception if the query fails
    +   */
    +
    +  public String explainJson() throws Exception {
    +    return explain(ClusterFixture.EXPLAIN_PLAN_JSON);
    +  }
    +
    +  public String explain(String format) throws Exception {
    +    queryText = "EXPLAIN PLAN FOR " + queryText;
    +    return queryPlan(format);
    +  }
    +
    +  private QuerySummary produceSummary(BufferingQueryEventListener listener) {
    +    long start = System.currentTimeMillis();
    +    int recordCount = 0;
    +    int batchCount = 0;
    +    QueryId queryId = null;
    +    loop:
    +    for ( ; ; ) {
    +      QueryEvent event = listener.get();
    --- End diff --
    
    Next commit will handle this by wrapping the `InterruptedException` in an event and returning it. That particular exception should never occur in a test case, but no harm in being complete.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95045450
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FieldDef.java ---
    @@ -0,0 +1,74 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.regex.Matcher;
    +import java.util.regex.Pattern;
    +
    +/**
    + * Basic representation of a column parsed from a query profile.
    + * Idea is to use this to generate mock data that represents a
    + * query obtained from a user. This is a work in progress.
    + */
    +
    +public class FieldDef {
    +  public enum Type { VARCHAR, DOUBLE };
    +  public enum TypeHint { DATE, TIME };
    +
    +  public final String name;
    +  public final String typeStr;
    +  public final Type type;
    +  public int length;
    +  public TypeHint hint;
    +
    +  public FieldDef( String name, String typeStr ) {
    +    this.name = name;
    +    this.typeStr = typeStr;
    +    Pattern p = Pattern.compile( "(\\w+)(?:\\((\\d+)\\))?" );
    +    Matcher m = p.matcher(typeStr);
    +    if ( ! m.matches() ) { throw new IllegalStateException( ); }
    +    if ( m.group(2) == null ) {
    +      length = 0;
    +    } else {
    +      length = Integer.parseInt( m.group(2) );
    --- End diff --
    
    It would be great to add a comment that group(2) is for VARCHAR types which has field length. e.g.: VARCHAR(2147483647)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95038015
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    +          name = DEFAULT_BIT_NAME;
    +        } else {
    +          name = DEFAULT_BIT_NAME + Integer.toString(i+1);
    +        }
    +      }
    +      bits.put(name, bit);
    +    }
    +
    +    // Some operations need an allocator.
    +
    +    allocator = RootAllocatorFactory.newRoot(config);
    +
    +    // Apply system options
    +    if ( builder.systemOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.systemOptions ) {
    +        clientFixture( ).alterSystem( option.key, option.value );
    +      }
    +    }
    +    // Apply session options.
    +
    +    boolean sawMaxWidth = false;
    +    if ( builder.sessionOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.sessionOptions ) {
    +        clientFixture( ).alterSession( option.key, option.value );
    +        if ( option.key.equals( ExecConstants.MAX_WIDTH_PER_NODE_KEY ) ) {
    +          sawMaxWidth = true;
    +        }
    +      }
    +    }
    +
    +    // Set the default parallelization unless already set by the caller.
    +
    +    if ( ! sawMaxWidth ) {
    +      clientFixture( ).alterSession( ExecConstants.MAX_WIDTH_PER_NODE_KEY, MAX_WIDTH_PER_NODE );
    +    }
    +  }
    +
    +  private Properties configProperties(Properties configProps) {
    +    Properties effectiveProps = new Properties( );
    +    for (Entry<Object, Object> entry : configProps.entrySet()) {
    +      effectiveProps.put(entry.getKey(), entry.getValue().toString());
    +    }
    +    if (zkHelper != null) {
    +      effectiveProps.put(ExecConstants.ZK_CONNECTION, zkHelper.getConfig().getString(ExecConstants.ZK_CONNECTION));
    +    }
    +    return effectiveProps;
    +  }
    +
    +  public Drillbit drillbit( ) { return bits.get(DEFAULT_BIT_NAME); }
    +  public Drillbit drillbit(String name) { return bits.get(name); }
    +  public Collection<Drillbit> drillbits( ) { return bits.values(); }
    +  public RemoteServiceSet serviceSet( ) { return serviceSet; }
    +  public BufferAllocator allocator( ) { return allocator; }
    +  public DrillConfig config() { return config; }
    +
    +  public ClientFixture.ClientBuilder clientBuilder( ) {
    +    return new ClientFixture.ClientBuilder(this);
    +  }
    +
    +  public ClientFixture clientFixture() {
    +    if ( clients.isEmpty( ) ) {
    +      clientBuilder( ).build( );
    +    }
    +    return clients.get(0);
    +  }
    +
    +  public DrillClient client() {
    +    return clientFixture().client();
    +  }
    +
    +  @Override
    +  public void close() throws Exception {
    +    Exception ex = null;
    +
    +    // Close clients. Clients remove themselves from the client
    +    // list.
    +
    +    while (! clients.isEmpty( )) {
    +      ex = safeClose(clients.get(0), ex);
    +    }
    +
    +    for (Drillbit bit : drillbits()) {
    --- End diff --
    
    Why not use Autocloseables.close instead ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95053287
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.FileReader;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.regex.Matcher;
    +import java.util.regex.Pattern;
    +
    +import javax.json.Json;
    +import javax.json.JsonArray;
    +import javax.json.JsonNumber;
    +import javax.json.JsonObject;
    +import javax.json.JsonReader;
    +import javax.json.JsonValue;
    +
    +/**
    + * Parses a query profile and provides access to various bits of the profile
    + * for diagnostic purposes during tests.
    + */
    +
    +public class ProfileParser {
    +
    +  JsonObject profile;
    +  List<String> plans;
    +
    +  public ProfileParser( File file ) throws IOException {
    +    try (FileReader fileReader = new FileReader(file);
    +         JsonReader reader = Json.createReader(fileReader)) {
    +      profile = (JsonObject) reader.read();
    +    }
    +  }
    +
    +  public String getQuery( ) {
    +    return profile.get("query").toString();
    +  }
    +
    +  public String getPlan() {
    +    return profile.get("plan").toString();
    +  }
    +
    +  public List<String> getPlans() {
    +    if ( plans != null ) {
    +      return plans; }
    +    String plan = getPlan( );
    +    Pattern p = Pattern.compile( "(\\d\\d-\\d+[^\\\\]*)\\\\n", Pattern.MULTILINE );
    +    Matcher m = p.matcher(plan);
    +    plans = new ArrayList<>( );
    +    while ( m.find() ) {
    +      plans.add(m.group(1));
    +    }
    +    return plans;
    +  }
    +
    +  public String getScan( ) {
    +    int n = getPlans( ).size();
    +    Pattern p = Pattern.compile( "\\d+-\\d+\\s+(\\w+)\\(" );
    --- End diff --
    
    Yes. What we *should* do is provide the JSON version of the query profile in the UI so that customers/users can send us that form instead of the text form. With the JSON form, parsing is much simpler.
    
    Yes, this can be simplified. Originally grabbed the other information such as the numbers, but not now. Also, was being paranoid incase the word "Scan" showed up as a table or field name, but "Scan(" is probably safe.
    
    And, yes, in the general case there are multiple scans. Changed that also.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #710: DRILL-5126: Provide simplified, unified "cluster fixture" ...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on the issue:

    https://github.com/apache/drill/pull/710
  
    +1 LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95052905
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    +          name = DEFAULT_BIT_NAME;
    +        } else {
    +          name = DEFAULT_BIT_NAME + Integer.toString(i+1);
    +        }
    +      }
    +      bits.put(name, bit);
    +    }
    +
    +    // Some operations need an allocator.
    +
    +    allocator = RootAllocatorFactory.newRoot(config);
    +
    +    // Apply system options
    +    if ( builder.systemOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.systemOptions ) {
    +        clientFixture( ).alterSystem( option.key, option.value );
    +      }
    +    }
    +    // Apply session options.
    +
    +    boolean sawMaxWidth = false;
    +    if ( builder.sessionOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.sessionOptions ) {
    +        clientFixture( ).alterSession( option.key, option.value );
    +        if ( option.key.equals( ExecConstants.MAX_WIDTH_PER_NODE_KEY ) ) {
    +          sawMaxWidth = true;
    +        }
    +      }
    +    }
    +
    +    // Set the default parallelization unless already set by the caller.
    +
    +    if ( ! sawMaxWidth ) {
    +      clientFixture( ).alterSession( ExecConstants.MAX_WIDTH_PER_NODE_KEY, MAX_WIDTH_PER_NODE );
    --- End diff --
    
    Done. Didn't originally have session options in the builder. But, now that they exist we should use them...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95041561
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -0,0 +1,314 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.List;
    +
    +import org.apache.drill.PlanTestBase;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.expression.SchemaPath;
    +import org.apache.drill.exec.client.PrintingResultsListener;
    +import org.apache.drill.exec.client.QuerySubmitter.Format;
    +import org.apache.drill.exec.proto.UserBitShared.QueryId;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.proto.helper.QueryIdHelper;
    +import org.apache.drill.exec.record.RecordBatchLoader;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.rpc.RpcException;
    +import org.apache.drill.exec.rpc.user.AwaitableUserResultsListener;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.rpc.user.UserResultsListener;
    +import org.apache.drill.exec.util.VectorUtil;
    +import org.apache.drill.exec.vector.NullableVarCharVector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.test.BufferingQueryEventListener.QueryEvent;
    +
    +import com.google.common.base.Preconditions;
    +
    +/**
    + * Builder for a Drill query. Provides all types of query formats,
    + * and a variety of ways to run the query.
    + */
    +
    +public class QueryBuilder {
    +
    +  /**
    +   * Summary results of a query: records, batches, run time.
    +   */
    +
    +  public static class QuerySummary {
    +    private final QueryId queryId;
    +    private final int records;
    +    private final int batches;
    +    private final long ms;
    +
    +    public QuerySummary(QueryId queryId, int recordCount, int batchCount, long elapsed) {
    +      this.queryId = queryId;
    +      records = recordCount;
    +      batches = batchCount;
    +      ms = elapsed;
    +    }
    +
    +    public long recordCount( ) { return records; }
    +    public int batchCount( ) { return batches; }
    +    public long runTimeMs( ) { return ms; }
    +    public QueryId queryId( ) { return queryId; }
    +    public String queryIdString( ) { return QueryIdHelper.getQueryId(queryId); }
    +
    +  }
    +
    +  private final ClientFixture client;
    +  private QueryType queryType;
    +  private String queryText;
    +
    +  QueryBuilder(ClientFixture client) {
    +    this.client = client;
    +  }
    +
    +  public QueryBuilder query(QueryType type, String text) {
    +    queryType = type;
    +    queryText = text;
    +    return this;
    +  }
    +
    +  public QueryBuilder sql(String sql) {
    +    return query( QueryType.SQL, sql );
    +  }
    +
    +  public QueryBuilder sql(String query, Object... args) {
    +    return sql(String.format(query, args));
    +  }
    +
    +  public QueryBuilder physical(String plan) {
    +    return query( QueryType.PHYSICAL, plan);
    +  }
    +
    +  public QueryBuilder sqlResource(String resource) {
    +    sql(ClusterFixture.loadResource(resource));
    +    return this;
    +  }
    +
    +  public QueryBuilder sqlResource(String resource, Object... args) {
    +    sql(ClusterFixture.loadResource(resource), args);
    +    return this;
    +  }
    +
    +  public QueryBuilder physicalResource(String resource) {
    +    physical(ClusterFixture.loadResource(resource));
    +    return this;
    +  }
    +
    +  /**
    +   * Run the query returning just a summary of the results: record count,
    +   * batch count and run time. Handy when doing performance tests when the
    +   * validity of the results is verified in some other test.
    +   *
    +   * @return the query summary
    +   */
    +
    +  public QuerySummary run() {
    +    return produceSummary(withEventListener());
    +  }
    +
    +  /**
    +   * Run the query and return a list of the result batches. Use
    +   * if the batch count is small and you want to work with them.
    +   * @return a list of batches resulting from the query
    +   * @throws RpcException
    +   */
    +
    +  public List<QueryDataBatch> results() throws RpcException {
    +    Preconditions.checkNotNull(queryType, "Query not provided.");
    +    Preconditions.checkNotNull(queryText, "Query not provided.");
    +    return client.client().runQuery(queryType, queryText);
    +  }
    +
    +  /**
    +   * Run the query with the listener provided. Use when the result
    +   * count will be large, or you don't need the results.
    +   *
    +   * @param listener the Drill listener
    +   */
    +
    +  public void withListener(UserResultsListener listener) {
    +    Preconditions.checkNotNull(queryType, "Query not provided.");
    +    Preconditions.checkNotNull(queryText, "Query not provided.");
    +    client.client().runQuery(queryType, queryText, listener);
    +  }
    +
    +  /**
    +   * Run the query, return an easy-to-use event listener to process
    +   * the query results. Use when the result set is large. The listener
    +   * allows the caller to iterate over results in the test thread.
    +   * (The listener implements a producer-consumer model to hide the
    +   * details of Drill listeners.)
    +   *
    +   * @return the query event listener
    +   */
    +
    +  public BufferingQueryEventListener withEventListener( ) {
    +    BufferingQueryEventListener listener = new BufferingQueryEventListener( );
    +    withListener(listener);
    +    return listener;
    +  }
    +
    +  public long printCsv() {
    +    return print(Format.CSV);
    +  }
    +
    +  public long print( Format format ) {
    +    return print(format,20);
    +  }
    +
    +  public long print(Format format, int colWidth) {
    +    return runAndWait( new PrintingResultsListener( client.cluster().config( ), format, colWidth ) );
    +  }
    +
    +  /**
    +   * Run a query and optionally print the output in TSV format.
    +   * Similar to {@link QueryTestUtil#test} with one query. Output is printed
    +   * only if the tests are running as verbose.
    +   *
    +   * @return the number of rows returned
    +   */
    +  public long print() {
    +    DrillConfig config = client.cluster().config( );
    +
    +    // Note: verbose check disabled until that change is
    +    // committed.
    +
    +    boolean verbose = ! config.getBoolean(QueryTestUtil.TEST_QUERY_PRINTING_SILENT) /* ||
    +                      DrillTest.verbose() */;
    +    if (verbose) {
    +      return print(Format.TSV, VectorUtil.DEFAULT_COLUMN_WIDTH);
    +    } else {
    +      return run().recordCount();
    +    }
    +  }
    +
    +  public long runAndWait(UserResultsListener listener) {
    +    AwaitableUserResultsListener resultListener =
    +        new AwaitableUserResultsListener(listener);
    +    withListener( resultListener );
    +    try {
    +      return resultListener.await();
    +    } catch (Exception e) {
    +      throw new IllegalStateException(e);
    +    }
    +  }
    +
    +  /**
    +   * Submit an "EXPLAIN" statement, and return text form of the
    +   * plan.
    +   * @throws Exception if the query fails
    +   */
    +
    +  public String explainText() throws Exception {
    +    return explain(ClusterFixture.EXPLAIN_PLAN_TEXT);
    +  }
    +
    +  /**
    +   * Submit an "EXPLAIN" statement, and return the JSON form of the
    +   * plan.
    +   * @throws Exception if the query fails
    +   */
    +
    +  public String explainJson() throws Exception {
    +    return explain(ClusterFixture.EXPLAIN_PLAN_JSON);
    +  }
    +
    +  public String explain(String format) throws Exception {
    +    queryText = "EXPLAIN PLAN FOR " + queryText;
    +    return queryPlan(format);
    +  }
    +
    +  private QuerySummary produceSummary(BufferingQueryEventListener listener) {
    +    long start = System.currentTimeMillis();
    +    int recordCount = 0;
    +    int batchCount = 0;
    +    QueryId queryId = null;
    +    loop:
    +    for ( ; ; ) {
    +      QueryEvent event = listener.get();
    --- End diff --
    
    We can get NPE here since get returns  "null" in case of exception.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96927306
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -49,6 +57,107 @@
     public class QueryBuilder {
     
       /**
    +   * Listener used to retrieve the query summary (only) asynchronously
    +   * using a {@link QuerySummaryFuture}.
    +   */
    +
    +  public class SummaryOnlyQueryEventListener implements UserResultsListener {
    +
    +    private final QuerySummaryFuture future;
    +    private QueryId queryId;
    +    private int recordCount;
    +    private int batchCount;
    +    private long startTime;
    +
    +    public SummaryOnlyQueryEventListener(QuerySummaryFuture future) {
    +      this.future = future;
    +      startTime = System.currentTimeMillis();
    +    }
    +
    +    @Override
    +    public void queryIdArrived(QueryId queryId) {
    +      this.queryId = queryId;
    +    }
    +
    +    @Override
    +    public void submissionFailed(UserException ex) {
    +      future.completed(new QuerySummary(queryId, recordCount, batchCount,
    +          System.currentTimeMillis() - startTime, ex));
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95035750
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    +          name = DEFAULT_BIT_NAME;
    +        } else {
    +          name = DEFAULT_BIT_NAME + Integer.toString(i+1);
    +        }
    +      }
    +      bits.put(name, bit);
    +    }
    +
    +    // Some operations need an allocator.
    +
    +    allocator = RootAllocatorFactory.newRoot(config);
    +
    +    // Apply system options
    +    if ( builder.systemOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.systemOptions ) {
    +        clientFixture( ).alterSystem( option.key, option.value );
    +      }
    +    }
    +    // Apply session options.
    +
    +    boolean sawMaxWidth = false;
    +    if ( builder.sessionOptions != null ) {
    +      for ( FixtureBuilder.RuntimeOption option : builder.sessionOptions ) {
    +        clientFixture( ).alterSession( option.key, option.value );
    +        if ( option.key.equals( ExecConstants.MAX_WIDTH_PER_NODE_KEY ) ) {
    +          sawMaxWidth = true;
    +        }
    +      }
    +    }
    +
    +    // Set the default parallelization unless already set by the caller.
    +
    +    if ( ! sawMaxWidth ) {
    +      clientFixture( ).alterSession( ExecConstants.MAX_WIDTH_PER_NODE_KEY, MAX_WIDTH_PER_NODE );
    --- End diff --
    
    can we set this option by default in the sessionOptions List so that we don't have to explicitly check for this ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95047142
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.FileReader;
    +import java.io.IOException;
    +import java.util.ArrayList;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.regex.Matcher;
    +import java.util.regex.Pattern;
    +
    +import javax.json.Json;
    +import javax.json.JsonArray;
    +import javax.json.JsonNumber;
    +import javax.json.JsonObject;
    +import javax.json.JsonReader;
    +import javax.json.JsonValue;
    +
    +/**
    + * Parses a query profile and provides access to various bits of the profile
    + * for diagnostic purposes during tests.
    + */
    +
    +public class ProfileParser {
    +
    +  JsonObject profile;
    +  List<String> plans;
    +
    +  public ProfileParser( File file ) throws IOException {
    +    try (FileReader fileReader = new FileReader(file);
    +         JsonReader reader = Json.createReader(fileReader)) {
    +      profile = (JsonObject) reader.read();
    +    }
    +  }
    +
    +  public String getQuery( ) {
    +    return profile.get("query").toString();
    +  }
    +
    +  public String getPlan() {
    +    return profile.get("plan").toString();
    +  }
    +
    +  public List<String> getPlans() {
    +    if ( plans != null ) {
    +      return plans; }
    +    String plan = getPlan( );
    +    Pattern p = Pattern.compile( "(\\d\\d-\\d+[^\\\\]*)\\\\n", Pattern.MULTILINE );
    +    Matcher m = p.matcher(plan);
    +    plans = new ArrayList<>( );
    +    while ( m.find() ) {
    +      plans.add(m.group(1));
    +    }
    +    return plans;
    +  }
    +
    +  public String getScan( ) {
    +    int n = getPlans( ).size();
    +    Pattern p = Pattern.compile( "\\d+-\\d+\\s+(\\w+)\\(" );
    +    for ( int i = n-1; i >= 0;  i-- ) {
    +      String plan = plans.get( i );
    +      Matcher m = p.matcher( plan );
    +      if ( ! m.find() ) { continue; }
    +      if ( m.group(1).equals( "Scan" ) ) {
    +        return plan; }
    +    }
    +    return null;
    +  }
    +
    +  public List<FieldDef> getColumns( String plan ) {
    +    Pattern p = Pattern.compile( "RecordType\\((.*)\\):" );
    +    Matcher m = p.matcher(plan);
    +    if ( ! m.find() ) { return null; }
    +    String frag = m.group(1);
    +    String parts[] = frag.split( ", " );
    +    List<FieldDef> fields = new ArrayList<>( );
    +    for ( String part : parts ) {
    +      String halves[] = part.split( " " );
    +      fields.add( new FieldDef( halves[1], halves[0] ) );
    +    }
    +    return fields;
    +  }
    +
    +  public Map<Integer,String> getOperators( ) {
    +    Map<Integer,String> ops = new HashMap<>();
    +    int n = getPlans( ).size();
    +    Pattern p = Pattern.compile( "\\d+-(\\d+)\\s+(\\w+)" );
    --- End diff --
    
    Verified all the regex with a sample plan from a query profile. Looks good!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96785539
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -57,20 +166,36 @@
         private final int records;
         private final int batches;
         private final long ms;
    +    private final QueryState finalState;
    +    private final Exception error;
     
    -    public QuerySummary(QueryId queryId, int recordCount, int batchCount, long elapsed) {
    +    public QuerySummary(QueryId queryId, int recordCount, int batchCount, long elapsed, QueryState state) {
           this.queryId = queryId;
           records = recordCount;
           batches = batchCount;
           ms = elapsed;
    +      finalState = state;
    +      error = null;
         }
     
    -    public long recordCount( ) { return records; }
    -    public int batchCount( ) { return batches; }
    -    public long runTimeMs( ) { return ms; }
    -    public QueryId queryId( ) { return queryId; }
    -    public String queryIdString( ) { return QueryIdHelper.getQueryId(queryId); }
    +    public QuerySummary(QueryId queryId, int recordCount, int batchCount, long elapsed, Exception ex) {
    +      this.queryId = queryId;
    +      records = recordCount;
    +      batches = batchCount;
    +      ms = elapsed;
    +      finalState = null;
    +      error = ex;
    +    }
     
    +    public boolean failed() { return error != null; }
    +    public boolean succeeded() { return error == null; }
    --- End diff --
    
    formatting here ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r94505587
  
    --- Diff: exec/java-exec/pom.xml ---
    @@ -458,6 +458,12 @@
           <artifactId>httpdlog-parser</artifactId>
           <version>2.4</version>
         </dependency>
    +	<dependency>
    --- End diff --
    
    Please fix the indentation


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96928159
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -49,6 +57,107 @@
     public class QueryBuilder {
     
       /**
    +   * Listener used to retrieve the query summary (only) asynchronously
    +   * using a {@link QuerySummaryFuture}.
    +   */
    +
    +  public class SummaryOnlyQueryEventListener implements UserResultsListener {
    +
    +    private final QuerySummaryFuture future;
    +    private QueryId queryId;
    +    private int recordCount;
    +    private int batchCount;
    +    private long startTime;
    +
    +    public SummaryOnlyQueryEventListener(QuerySummaryFuture future) {
    +      this.future = future;
    +      startTime = System.currentTimeMillis();
    +    }
    +
    +    @Override
    +    public void queryIdArrived(QueryId queryId) {
    +      this.queryId = queryId;
    +    }
    +
    +    @Override
    +    public void submissionFailed(UserException ex) {
    +      future.completed(new QuerySummary(queryId, recordCount, batchCount,
    +          System.currentTimeMillis() - startTime, ex));
    +    }
    +
    +    @Override
    +    public void dataArrived(QueryDataBatch result, ConnectionThrottle throttle) {
    +      batchCount++;
    +      recordCount += result.getHeader().getRowCount();
    +      result.release();
    +    }
    +
    +    @Override
    +    public void queryCompleted(QueryState state) {
    +      future.completed(new QuerySummary(queryId, recordCount, batchCount,
    +                       System.currentTimeMillis() - startTime, state));
    +    }
    +  }
    +
    +  /**
    +   * The future used to wait for the completion of an async query. Returns
    +   * just the summary of the query.
    +   */
    +
    +  public class QuerySummaryFuture implements Future<QuerySummary> {
    +
    +    /**
    +     * Synchronizes the listener thread and the test thread that
    +     * launched the query.
    +     */
    +
    --- End diff --
    
    Fixed the other way: consistent white space before & after Javadoc comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96785327
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/LogFixture.java ---
    @@ -0,0 +1,255 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.test;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import org.slf4j.LoggerFactory;
    +
    +import ch.qos.logback.classic.Level;
    +import ch.qos.logback.classic.Logger;
    +import ch.qos.logback.classic.LoggerContext;
    +import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
    +import ch.qos.logback.classic.spi.ILoggingEvent;
    +import ch.qos.logback.core.ConsoleAppender;
    +
    +/**
    + * Establishes test-specific logging without having to alter the global
    + * <tt>logback-test.xml</tt> file. Allows directing output to the console
    + * (if not already configured) and setting the log level on specific loggers
    + * of interest in the test. The fixture automatically restores the original
    + * log configuration on exit.
    + * <p>
    + * Typical usage: <pre><code>
    + * {@literal @}Test
    + * public void myTest() {
    + *   LogFixtureBuilder logBuilder = LogFixture.builder()
    + *          .toConsole()
    + *          .disable() // Silence all other loggers
    + *          .logger(ExternalSortBatch.class, Level.DEBUG);
    + *   try (LogFixture logs = logBuilder.build()) {
    + *     // Test code here
    + *   }
    + * }</code></pre>
    + *  <p>
    + * You can &ndash; and should &ndash; combine the log fixtue with the
    + * cluster and client fixtures to have complete control over your test-time
    + * Drill environment.
    + */
    +
    +public class LogFixture implements AutoCloseable {
    +
    +  // Elapsed time in ms, log level, thread, logger, message.
    +
    +  public static final String DEFAULT_CONSOLE_FORMAT = "%r %level [%thread] [%logger] - %msg%n";
    +  private static final String DRILL_PACKAGE_NAME = "org.apache.drill";
    +
    +  /**
    +   * Memento for a logger name and level.
    +   */
    +  public static class LogSpec {
    +    String loggerName;
    +    Level logLevel;
    +
    +    public LogSpec(String loggerName, Level level) {
    +      this.loggerName = loggerName;
    +      this.logLevel = level;
    +    }
    +  }
    +
    +  /**
    +   * Builds the log settings to be used for a test. The log settings here
    +   * add to those specified in a <tt>logback.xml</tt> or
    +   * <tt>logback-test.xml</tt> file on your class path. In particular, if
    +   * the logging configuration already redirects the Drill logger to the
    +   * console, setting console logging here does nothing.
    +   */
    +
    +  public static class LogFixtureBuilder {
    +
    +    private String consoleFormat = DEFAULT_CONSOLE_FORMAT;
    +    private boolean logToConsole;
    +    private List<LogSpec> loggers = new ArrayList<>();
    +
    +    /**
    +     * Send all enabled logging to the console (if not already configured.) Some
    +     * Drill log configuration files send the root to the console (or file), but
    +     * the Drill loggers to Lilith. In that case, Lilith "hides" the console
    +     * logger. Using this call adds a console logger to the Drill logger so that
    +     * output does, in fact, go to the console regardless of the configuration
    +     * in the Logback configuration file.
    +     *
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder toConsole() {
    +      logToConsole = true;
    +      return this;
    +    }
    +
    +    /**
    +     * Send logging to the console using the defined format.
    +     *
    +     * @param format valid Logback log format
    +     * @return this builder
    +     */
    +
    +    public LogFixtureBuilder toConsole(String format) {
    +      consoleFormat = format;
    +      return toConsole();
    +    }
    +
    +    /**
    +     * Set a specific logger to the given level.
    +     *
    +     * @param loggerName name of the logger (typically used for package-level
    +     * loggers)
    +     * @param level the desired Logback-defined level
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder logger(String loggerName, Level level) {
    +      loggers.add(new LogSpec(loggerName, level));
    +      return this;
    +    }
    +
    +    /**
    +     * Set a specific logger to the given level.
    +     *
    +     * @param loggerClass class that defines the logger (typically used for
    +     * class-specific loggers)
    +     * @param level the desired Logback-defined level
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder logger(Class<?> loggerClass, Level level) {
    +      loggers.add(new LogSpec(loggerClass.getName(), level));
    +      return this;
    +    }
    +
    +    /**
    +     * Turns off all logging. If called first, you can set disable as your
    +     * general policy, then turn back on loggers selectively for those
    +     * of interest.
    +     * @return this builder
    +     */
    +    public LogFixtureBuilder disable() {
    +      return rootLogger(Level.OFF);
    +    }
    +
    +    /**
    +     * Set the desired log level on the root logger.
    +     * @param level the desired Logback log level
    +     * @return this builder
    +     */
    +
    +    public LogFixtureBuilder rootLogger(Level level) {
    +      loggers.add(new LogSpec(Logger.ROOT_LOGGER_NAME, level));
    +      return this;
    +    }
    +
    +    /**
    +     * Apply the log levels and output, then return a fixture to be used
    +     * in a try-with-resources block. The fixture automatically restores
    +     * the original configuration on completion of the try block.
    +     * @return the log fixture
    +     */
    +    public LogFixture build() {
    +      return new LogFixture(this);
    +    }
    +  }
    +
    +  private PatternLayoutEncoder ple;
    +  private ConsoleAppender<ILoggingEvent> appender;
    +  private List<LogSpec> loggers = new ArrayList<>();
    +  private Logger drillLogger;
    +
    +  public LogFixture(LogFixtureBuilder builder) {
    +    if (builder.logToConsole) {
    +      setupConsole(builder);
    +    }
    +    setupLoggers(builder);
    +  }
    +
    +  /**
    +   * Creates a new log fixture builder.
    +   * @return the log fixture builder
    +   */
    +
    +  public static LogFixtureBuilder builder() {
    +    return new LogFixtureBuilder();
    +  }
    +
    +  private void setupConsole(LogFixtureBuilder builder) {
    +    Logger drillLogger = (Logger)LoggerFactory.getLogger(DRILL_PACKAGE_NAME);
    +    if (drillLogger.getAppender("STDOUT") != null) {
    +      return;
    +    }
    +    LoggerContext lc = (LoggerContext) LoggerFactory.getILoggerFactory();
    +    ple = new PatternLayoutEncoder();
    +    ple.setPattern(builder.consoleFormat);
    +    ple.setContext(lc);
    +    ple.start();
    +
    +    appender = new ConsoleAppender<>( );
    +    appender.setContext(lc);
    +    appender.setName("Console");
    +    appender.setEncoder( ple );
    +    appender.start();
    +
    +    Logger root = (Logger)LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
    --- End diff --
    
    We are checking only for console appender in DrillLogger. Can't root logger have the console appender and drillLogger may not ? In which case we will overwrite the console appender of root logger and while restoring we will remove the console appender from both.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95046976
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java ---
    @@ -22,220 +22,167 @@
     
     import java.util.List;
     
    -import org.apache.drill.BaseTestQuery;
    -import org.apache.drill.common.config.DrillConfig;
     import org.apache.drill.common.expression.ExpressionPosition;
     import org.apache.drill.common.expression.SchemaPath;
    -import org.apache.drill.common.util.FileUtils;
     import org.apache.drill.common.util.TestTools;
    -import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.exception.SchemaChangeException;
    +import org.apache.drill.exec.memory.BufferAllocator;
     import org.apache.drill.exec.record.RecordBatchLoader;
     import org.apache.drill.exec.rpc.user.QueryDataBatch;
    -import org.apache.drill.exec.server.Drillbit;
    -import org.apache.drill.exec.server.RemoteServiceSet;
     import org.apache.drill.exec.vector.BigIntVector;
    +import org.apache.drill.test.ClientFixture;
    +import org.apache.drill.test.ClusterFixture;
    +import org.apache.drill.test.ClusterTest;
    +import org.apache.drill.test.DrillTest;
    +import org.apache.drill.test.FixtureBuilder;
     import org.junit.Ignore;
     import org.junit.Rule;
     import org.junit.Test;
     import org.junit.rules.TestRule;
     
    -import com.google.common.base.Charsets;
    -import com.google.common.io.Files;
    -
    -@Ignore
    -public class TestSimpleExternalSort extends BaseTestQuery {
    +public class TestSimpleExternalSort extends DrillTest {
       static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestSimpleExternalSort.class);
    -  DrillConfig c = DrillConfig.create();
    -
     
       @Rule public final TestRule TIMEOUT = TestTools.getTimeoutRule(80000);
     
       @Test
    -  public void mergeSortWithSv2() throws Exception {
    -    List<QueryDataBatch> results = testPhysicalFromFileWithResults("xsort/one_key_sort_descending_sv2.json");
    -    int count = 0;
    -    for(QueryDataBatch b : results) {
    -      if (b.getHeader().getRowCount() != 0) {
    -        count += b.getHeader().getRowCount();
    -      }
    -    }
    -    assertEquals(500000, count);
    -
    -    long previousBigInt = Long.MAX_VALUE;
    -
    -    int recordCount = 0;
    -    int batchCount = 0;
    -
    -    for (QueryDataBatch b : results) {
    -      if (b.getHeader().getRowCount() == 0) {
    -        break;
    -      }
    -      batchCount++;
    -      RecordBatchLoader loader = new RecordBatchLoader(allocator);
    -      loader.load(b.getHeader().getDef(),b.getData());
    -      BigIntVector c1 = (BigIntVector) loader.getValueAccessorById(BigIntVector.class,
    -              loader.getValueVectorId(new SchemaPath("blue", ExpressionPosition.UNKNOWN)).getFieldIds()).getValueVector();
    -
    -
    -      BigIntVector.Accessor a1 = c1.getAccessor();
    +  public void mergeSortWithSv2Legacy() throws Exception {
    +    mergeSortWithSv2(true);
    +  }
     
    -      for (int i =0; i < c1.getAccessor().getValueCount(); i++) {
    -        recordCount++;
    -        assertTrue(String.format("%d > %d", previousBigInt, a1.get(i)), previousBigInt >= a1.get(i));
    -        previousBigInt = a1.get(i);
    -      }
    -      loader.clear();
    -      b.release();
    +  /**
    +   * Tests the external sort using an in-memory sort. Relies on default memory
    +   * settings to be large enough to do the in-memory sort (there is,
    +   * unfortunately, no way to double-check that no spilling was done.)
    +   * This must be checked manually by setting a breakpoint in the in-memory
    +   * sort routine.
    +   *
    +   * @param testLegacy
    +   * @throws Exception
    +   */
    +
    +  private void mergeSortWithSv2(boolean testLegacy) throws Exception {
    +    try (ClusterFixture cluster = ClusterFixture.standardCluster( );
    +         ClientFixture client = cluster.clientFixture()) {
    +      chooseImpl(client, testLegacy);
    +      List<QueryDataBatch> results = client.queryBuilder().physicalResource("xsort/one_key_sort_descending_sv2.json").results();
    +      assertEquals(500000, client.countResults( results ));
    +      validateResults(client.allocator(), results);
         }
    +  }
     
    -    System.out.println(String.format("Sorted %,d records in %d batches.", recordCount, batchCount));
    +  private void chooseImpl(ClientFixture client, boolean testLegacy) throws Exception {
    --- End diff --
    
    Will the implementation for this be added once the new sort mechanism is checked in ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95052757
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java ---
    @@ -22,220 +22,167 @@
     
     import java.util.List;
     
    -import org.apache.drill.BaseTestQuery;
    -import org.apache.drill.common.config.DrillConfig;
     import org.apache.drill.common.expression.ExpressionPosition;
     import org.apache.drill.common.expression.SchemaPath;
    -import org.apache.drill.common.util.FileUtils;
     import org.apache.drill.common.util.TestTools;
    -import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.exception.SchemaChangeException;
    +import org.apache.drill.exec.memory.BufferAllocator;
     import org.apache.drill.exec.record.RecordBatchLoader;
     import org.apache.drill.exec.rpc.user.QueryDataBatch;
    -import org.apache.drill.exec.server.Drillbit;
    -import org.apache.drill.exec.server.RemoteServiceSet;
     import org.apache.drill.exec.vector.BigIntVector;
    +import org.apache.drill.test.ClientFixture;
    +import org.apache.drill.test.ClusterFixture;
    +import org.apache.drill.test.ClusterTest;
    +import org.apache.drill.test.DrillTest;
    +import org.apache.drill.test.FixtureBuilder;
     import org.junit.Ignore;
     import org.junit.Rule;
     import org.junit.Test;
     import org.junit.rules.TestRule;
     
    -import com.google.common.base.Charsets;
    -import com.google.common.io.Files;
    -
    -@Ignore
    -public class TestSimpleExternalSort extends BaseTestQuery {
    +public class TestSimpleExternalSort extends DrillTest {
       static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestSimpleExternalSort.class);
    -  DrillConfig c = DrillConfig.create();
    -
     
       @Rule public final TestRule TIMEOUT = TestTools.getTimeoutRule(80000);
     
       @Test
    -  public void mergeSortWithSv2() throws Exception {
    -    List<QueryDataBatch> results = testPhysicalFromFileWithResults("xsort/one_key_sort_descending_sv2.json");
    -    int count = 0;
    -    for(QueryDataBatch b : results) {
    -      if (b.getHeader().getRowCount() != 0) {
    -        count += b.getHeader().getRowCount();
    -      }
    -    }
    -    assertEquals(500000, count);
    -
    -    long previousBigInt = Long.MAX_VALUE;
    -
    -    int recordCount = 0;
    -    int batchCount = 0;
    -
    -    for (QueryDataBatch b : results) {
    -      if (b.getHeader().getRowCount() == 0) {
    -        break;
    -      }
    -      batchCount++;
    -      RecordBatchLoader loader = new RecordBatchLoader(allocator);
    -      loader.load(b.getHeader().getDef(),b.getData());
    -      BigIntVector c1 = (BigIntVector) loader.getValueAccessorById(BigIntVector.class,
    -              loader.getValueVectorId(new SchemaPath("blue", ExpressionPosition.UNKNOWN)).getFieldIds()).getValueVector();
    -
    -
    -      BigIntVector.Accessor a1 = c1.getAccessor();
    +  public void mergeSortWithSv2Legacy() throws Exception {
    +    mergeSortWithSv2(true);
    +  }
     
    -      for (int i =0; i < c1.getAccessor().getValueCount(); i++) {
    -        recordCount++;
    -        assertTrue(String.format("%d > %d", previousBigInt, a1.get(i)), previousBigInt >= a1.get(i));
    -        previousBigInt = a1.get(i);
    -      }
    -      loader.clear();
    -      b.release();
    +  /**
    +   * Tests the external sort using an in-memory sort. Relies on default memory
    +   * settings to be large enough to do the in-memory sort (there is,
    +   * unfortunately, no way to double-check that no spilling was done.)
    +   * This must be checked manually by setting a breakpoint in the in-memory
    +   * sort routine.
    +   *
    +   * @param testLegacy
    +   * @throws Exception
    +   */
    +
    +  private void mergeSortWithSv2(boolean testLegacy) throws Exception {
    +    try (ClusterFixture cluster = ClusterFixture.standardCluster( );
    +         ClientFixture client = cluster.clientFixture()) {
    +      chooseImpl(client, testLegacy);
    +      List<QueryDataBatch> results = client.queryBuilder().physicalResource("xsort/one_key_sort_descending_sv2.json").results();
    +      assertEquals(500000, client.countResults( results ));
    +      validateResults(client.allocator(), results);
         }
    +  }
     
    -    System.out.println(String.format("Sorted %,d records in %d batches.", recordCount, batchCount));
    +  private void chooseImpl(ClientFixture client, boolean testLegacy) throws Exception {
    --- End diff --
    
    Yes. This is a placeholder. Once the managed sort and this framework are both committed, then this test will be replaced with one that tests both the original and managed version. This method switches between them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95053137
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java ---
    @@ -0,0 +1,251 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Properties;
    +
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +
    +/**
    + * Build a Drillbit and client with the options provided. The simplest
    + * builder starts an embedded Drillbit, with the "dfs_test" name space,
    + * a max width (parallelization) of 2.
    + */
    +
    +public class FixtureBuilder {
    +
    +  public static class RuntimeOption {
    +    public String key;
    +    public Object value;
    +
    +    public RuntimeOption(String key, Object value) {
    +      this.key = key;
    +      this.value = value;
    +    }
    +  }
    +
    +  // Values in the drill-module.conf file for values that are customized
    +  // in the defaults.
    +
    +  public static final int DEFAULT_ZK_REFRESH = 500; // ms
    +  public static final int DEFAULT_SERVER_RPC_THREADS = 10;
    +  public static final int DEFAULT_SCAN_THREADS = 8;
    +
    +  public static Properties defaultProps() {
    +    Properties props = new Properties();
    +    props.putAll(ClusterFixture.TEST_CONFIGURATIONS);
    +    return props;
    +  }
    +
    +  String configResource;
    +  Properties configProps;
    +  boolean enableFullCache;
    +  List<RuntimeOption> sessionOptions;
    +  List<RuntimeOption> systemOptions;
    +  int bitCount = 1;
    +  String bitNames[];
    +  int zkCount;
    +  ZookeeperHelper zkHelper;
    +
    +  /**
    +   * Use the given configuration properties to start the embedded Drillbit.
    +   * @param configProps a collection of config properties
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configProps(Properties configProps) {
    +    this.configProps = configProps;
    +    return this;
    +  }
    +
    +  /**
    +   * Use the given configuration file, stored as a resource, to start the
    +   * embedded Drillbit. Note that the resource file should have the two
    +   * following settings to work as a test:
    +   * <pre><code>
    +   * drill.exec.sys.store.provider.local.write : false,
    +   * drill.exec.http.enabled : false
    +   * </code></pre>
    +   * It may be more convenient to add your settings to the default
    +   * config settings with {@link #configProperty(String, Object)}.
    +   * @param configResource path to the file that contains the
    +   * config file to be read
    +   * @return this builder
    +   * @see {@link #configProperty(String, Object)}
    +   */
    +
    +  public FixtureBuilder configResource(String configResource) {
    +
    +    // TypeSafe gets unhappy about a leading slash, but other functions
    +    // require it. Silently discard the leading slash if given to
    +    // preserve the test writer's sanity.
    +
    +    this.configResource = ClusterFixture.trimSlash(configResource);
    +    return this;
    +  }
    +
    +  /**
    +   * Add an additional boot-time property for the embedded Drillbit.
    +   * @param key config property name
    +   * @param value property value
    +   * @return this builder
    +   */
    +
    +  public FixtureBuilder configProperty(String key, Object value) {
    +    if (configProps == null) {
    +      configProps = defaultProps();
    +    }
    +    configProps.put(key, value.toString());
    +    return this;
    +  }
    +
    +   /**
    +   * Provide a session option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the session option
    +   * @param value the value of the session option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSession(String, Object)}
    +   */
    +
    +  public FixtureBuilder sessionOption(String key, Object value) {
    +    if (sessionOptions == null) {
    +      sessionOptions = new ArrayList<>();
    +    }
    +    sessionOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Provide a system option to be set once the Drillbit
    +   * is started.
    +   *
    +   * @param key the name of the system option
    +   * @param value the value of the system option
    +   * @return this builder
    +   * @see {@link ClusterFixture#alterSystem(String, Object)}
    +   */
    +
    +  public FixtureBuilder systemOption(String key, Object value) {
    +    if (systemOptions == null) {
    +      systemOptions = new ArrayList<>();
    +    }
    +    systemOptions.add(new RuntimeOption(key, value));
    +    return this;
    +  }
    +
    +  /**
    +   * Set the maximum parallelization (max width per node). Defaults
    +   * to 2.
    +   *
    +   * @param n the "max width per node" parallelization option.
    +   * @return this builder
    +   */
    +  public FixtureBuilder maxParallelization(int n) {
    +    return sessionOption(ExecConstants.MAX_WIDTH_PER_NODE_KEY, n);
    --- End diff --
    
    As it turns out, we do. `sessionOption()` is a fluent method to set the session option that returns the builder which we just pass along above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95052810
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    --- End diff --
    
    We could. But this would mean that in 99% of the tests, the name would be "drillbit1" rather than "drillbit". And, to make it easier to add a second Drillbit to an existing test, if there are two this names them "drillbit" and "drillbit2". Can't recall if I did this to be compatible with older test frameworks.
    
    Did clean up the condition and add an explanation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r96785470
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/QueryBuilder.java ---
    @@ -49,6 +57,107 @@
     public class QueryBuilder {
     
       /**
    +   * Listener used to retrieve the query summary (only) asynchronously
    +   * using a {@link QuerySummaryFuture}.
    +   */
    +
    +  public class SummaryOnlyQueryEventListener implements UserResultsListener {
    +
    +    private final QuerySummaryFuture future;
    +    private QueryId queryId;
    +    private int recordCount;
    +    private int batchCount;
    +    private long startTime;
    +
    +    public SummaryOnlyQueryEventListener(QuerySummaryFuture future) {
    +      this.future = future;
    +      startTime = System.currentTimeMillis();
    +    }
    +
    +    @Override
    +    public void queryIdArrived(QueryId queryId) {
    +      this.queryId = queryId;
    +    }
    +
    +    @Override
    +    public void submissionFailed(UserException ex) {
    +      future.completed(new QuerySummary(queryId, recordCount, batchCount,
    +          System.currentTimeMillis() - startTime, ex));
    +    }
    +
    +    @Override
    +    public void dataArrived(QueryDataBatch result, ConnectionThrottle throttle) {
    +      batchCount++;
    +      recordCount += result.getHeader().getRowCount();
    +      result.release();
    +    }
    +
    +    @Override
    +    public void queryCompleted(QueryState state) {
    +      future.completed(new QuerySummary(queryId, recordCount, batchCount,
    +                       System.currentTimeMillis() - startTime, state));
    +    }
    +  }
    +
    +  /**
    +   * The future used to wait for the completion of an async query. Returns
    +   * just the summary of the query.
    +   */
    +
    +  public class QuerySummaryFuture implements Future<QuerySummary> {
    +
    +    /**
    +     * Synchronizes the listener thread and the test thread that
    +     * launched the query.
    +     */
    +
    --- End diff --
    
    please remove extra space here and other places too. Like line 120


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #710: DRILL-5126: Provide simplified, unified "cluster fi...

Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/710#discussion_r95032760
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
    @@ -0,0 +1,405 @@
    +/*
    + * 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
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + ******************************************************************************/
    +package org.apache.drill.test;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URL;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Map.Entry;
    +import java.util.Properties;
    +
    +import org.apache.commons.io.FileUtils;
    +import org.apache.drill.DrillTestWrapper.TestServices;
    +import org.apache.drill.QueryTestUtil;
    +import org.apache.drill.common.config.DrillConfig;
    +import org.apache.drill.common.exceptions.ExecutionSetupException;
    +import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.ZookeeperHelper;
    +import org.apache.drill.exec.client.DrillClient;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.RootAllocatorFactory;
    +import org.apache.drill.exec.proto.UserBitShared.QueryType;
    +import org.apache.drill.exec.rpc.user.QueryDataBatch;
    +import org.apache.drill.exec.server.Drillbit;
    +import org.apache.drill.exec.server.RemoteServiceSet;
    +import org.apache.drill.exec.store.StoragePluginRegistry;
    +import org.apache.drill.exec.store.StoragePluginRegistryImpl;
    +import org.apache.drill.exec.store.dfs.FileSystemConfig;
    +import org.apache.drill.exec.store.dfs.FileSystemPlugin;
    +import org.apache.drill.exec.store.dfs.WorkspaceConfig;
    +import org.apache.drill.exec.store.mock.MockStorageEngine;
    +import org.apache.drill.exec.store.mock.MockStorageEngineConfig;
    +import org.apache.drill.exec.util.TestUtilities;
    +
    +import com.google.common.base.Charsets;
    +import com.google.common.base.Preconditions;
    +import com.google.common.io.Files;
    +import com.google.common.io.Resources;
    +
    +/**
    + * Test fixture to start a Drillbit with provide options, create a client,
    + * and execute queries. Can be used in JUnit tests, or in ad-hoc programs.
    + * Provides a builder to set the necessary embedded Drillbit and client
    + * options, then creates the requested Drillbit and client.
    + */
    +
    +public class ClusterFixture implements AutoCloseable {
    +//  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ClientFixture.class);
    +  public static final String ENABLE_FULL_CACHE = "drill.exec.test.use-full-cache";
    +  public static final int MAX_WIDTH_PER_NODE = 2;
    +
    +  @SuppressWarnings("serial")
    +  public static final Properties TEST_CONFIGURATIONS = new Properties() {
    +    {
    +      // Properties here mimic those in drill-root/pom.xml, Surefire plugin
    +      // configuration. They allow tests to run successfully in Eclipse.
    +
    +      put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, false);
    +      put(ExecConstants.HTTP_ENABLE, false);
    +      put(Drillbit.SYSTEM_OPTIONS_NAME, "org.apache.drill.exec.compile.ClassTransformer.scalar_replacement=on");
    +      put(QueryTestUtil.TEST_QUERY_PRINTING_SILENT, true);
    +      put("drill.catastrophic_to_standard_out", true);
    +
    +      // See Drillbit.close. The Drillbit normally waits a specified amount
    +      // of time for ZK registration to drop. But, embedded Drillbits normally
    +      // don't use ZK, so no need to wait.
    +
    +      put(ExecConstants.ZK_REFRESH, 0);
    +
    +      // This is just a test, no need to be heavy-duty on threads.
    +      // This is the number of server and client RPC threads. The
    +      // production default is DEFAULT_SERVER_RPC_THREADS.
    +
    +      put(ExecConstants.BIT_SERVER_RPC_THREADS, 2);
    +
    +      // No need for many scanners except when explicitly testing that
    +      // behavior. Production default is DEFAULT_SCAN_THREADS
    +
    +      put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
    +    }
    +  };
    +
    +
    +  public static final String DEFAULT_BIT_NAME = "drillbit";
    +
    +  private DrillConfig config;
    +  private Map<String,Drillbit> bits = new HashMap<>( );
    +  private BufferAllocator allocator;
    +  private boolean ownsZK;
    +  private ZookeeperHelper zkHelper;
    +  private RemoteServiceSet serviceSet;
    +  private String dfsTestTmpSchemaLocation;
    +  List<ClientFixture> clients = new ArrayList<>( );
    +
    +  ClusterFixture( FixtureBuilder  builder ) throws Exception {
    +
    +    // Start ZK if requested.
    +
    +    if (builder.zkHelper != null) {
    +      zkHelper = builder.zkHelper;
    +      ownsZK = false;
    +    } else if (builder.zkCount > 0) {
    +      zkHelper = new ZookeeperHelper(true);
    +      zkHelper.startZookeeper(builder.zkCount);
    +      ownsZK = true;
    +    }
    +
    +    // Create a config
    +    // Because of the way DrillConfig works, we can set the ZK
    +    // connection string only if a property set is provided.
    +
    +    if ( builder.configResource != null ) {
    +      config = DrillConfig.create(builder.configResource);
    +    } else if (builder.configProps != null) {
    +      config = DrillConfig.create(configProperties(builder.configProps));
    +    } else {
    +      config = DrillConfig.create(configProperties(TEST_CONFIGURATIONS));
    +    }
    +
    +    // Not quite sure what this is, but some tests seem to use it.
    +
    +    if (builder.enableFullCache ||
    +        (config.hasPath(ENABLE_FULL_CACHE) && config.getBoolean(ENABLE_FULL_CACHE))) {
    +      serviceSet = RemoteServiceSet.getServiceSetWithFullCache(config, allocator);
    +    } else {
    +      serviceSet = RemoteServiceSet.getLocalServiceSet();
    +    }
    +
    +    dfsTestTmpSchemaLocation = TestUtilities.createTempDir();
    +
    +    Preconditions.checkArgument(builder.bitCount > 0);
    +    int bitCount = builder.bitCount;
    +    for ( int i = 0;  i < bitCount;  i++ ) {
    +      @SuppressWarnings("resource")
    +      Drillbit bit = new Drillbit(config, serviceSet);
    +      bit.run( );
    +
    +      // Create the dfs_test name space
    +
    +      @SuppressWarnings("resource")
    +      final StoragePluginRegistry pluginRegistry = bit.getContext().getStorage();
    +      TestUtilities.updateDfsTestTmpSchemaLocation(pluginRegistry, dfsTestTmpSchemaLocation);
    +      TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
    +
    +      // Create the mock data plugin
    +      // (Disabled until DRILL-5152 is committed.)
    +
    +//      MockStorageEngineConfig config = MockStorageEngineConfig.INSTANCE;
    +//      @SuppressWarnings("resource")
    +//      MockStorageEngine plugin = new MockStorageEngine(MockStorageEngineConfig.INSTANCE, bit.getContext(), MockStorageEngineConfig.NAME);
    +//      ((StoragePluginRegistryImpl) pluginRegistry).definePlugin(MockStorageEngineConfig.NAME, config, plugin);
    +
    +      // Bit name and registration.
    +
    +      String name;
    +      if (builder.bitNames != null && i < builder.bitNames.length) {
    +        name = builder.bitNames[i];
    +      } else {
    +        if (i == 0) {
    --- End diff --
    
    we can remove (i == 0) check and always set name as "DEFAULT_BIT_NAME} + Integer.toString(i+1);


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---