[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[Kunal Khatua (JIRA)]

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

Kunal Khatua commented on DRILL-5260:
-

Closing as no QA verification is required

> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>  Labels: ready-to-commit
> Fix For: 1.10.0
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[Paul Rogers (JIRA)]

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

Paul Rogers commented on DRILL-5260:


Development-only issue, no QA verification needed.

> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10.0
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>  Labels: ready-to-commit
> Fix For: 1.10.0
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user asfgit closed the pull request at:

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


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>  Labels: ready-to-commit
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user sudheeshkatkam commented on the issue:

https://github.com/apache/drill/pull/753
  
+1


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
>  Labels: ready-to-commit
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r103060090
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
+
+  p = Pattern.compile("rowType = RecordType\\((.*)\\): (rowcount .*)");
+  m = p.matcher(tail);
--- End diff --

As it turns out, different scan operators use a different syntax for this 
info. This syntax works for the scan operators used in tests thus far. We'll 
need to extend this for others.

Actually, we'll want to combine this functionality with the full-blown 
parser used earlier. This was a quick & dirty on that predated the fancier one. 
And, as noted, the real solution is to include the info in the profile in JSON 
to avoid the need to fiddle with parsing the text format.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r103052249
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
+
+  p = Pattern.compile("rowType = RecordType\\((.*)\\): (rowcount .*)");
+  m = p.matcher(tail);
--- End diff --

Discussed offline.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user sohami commented on the issue:

https://github.com/apache/drill/pull/753
  
Apart from fixing regex in ProfileParser.java, changes looks good to me.
+1


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102844435
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ExampleTest.java ---
@@ -0,0 +1,243 @@
+/*
+ * 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 static org.junit.Assert.*;
+
--- End diff --

The wild card is traditional: it allows use of any of the JUnit assertions 
without having to import them one-by-one.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102846203
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
--- End diff --

Here we are reverse engineering a format created elsewhere in Drill. Since 
that format uses a minimum of four spaces, we have to subtract that off. Then, 
that code inserts two more spaces for each indent level.

Clearly, if we change the text format at all, the parser here breaks.

A much better solution would be to include all this info in a JSON format...


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102843167
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -99,33 +111,83 @@
   // behavior. Production default is DEFAULT_SCAN_THREADS
 
   put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
+
+  // Define a useful root location for the ZK persistent
+  // storage. Profiles will go here when running in distributed
+  // mode.
+
+  
put(ZookeeperPersistentStoreProvider.DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT, 
"/tmp/drill/log");
 }
   };
 
   public static final String DEFAULT_BIT_NAME = "drillbit";
 
   private DrillConfig config;
-  private Map bits = new HashMap<>();
+  private Map bits = new HashMap<>();
   private Drillbit defaultDrillbit;
   private BufferAllocator allocator;
   private boolean ownsZK;
   private ZookeeperHelper zkHelper;
   private RemoteServiceSet serviceSet;
-  private String dfsTestTmpSchemaLocation;
+  private File dfsTestTempDir;
   protected List clients = new ArrayList<>();
+  private boolean usesZk;
+  private boolean preserveLocalFiles;
+  private boolean isLocal;
+
+  /**
+   * Temporary directories created for this test cluster.
+   * Each is removed when closing the cluster.
+   */
+
+  private List tempDirs = new ArrayList<>();
+
+  ClusterFixture(FixtureBuilder builder) {
+
+String zkConnect = configureZk(builder);
+try {
+  createConfig(builder, zkConnect);
+  startDrillbits(builder);
+  applyOptions(builder);
+} catch (Exception e) {
+  // Translate exceptions to unchecked to avoid cluttering
+  // tests. Failures will simply fail the test itself.
+
+  throw new IllegalStateException( "Cluster fixture setup failed", e );
+}
+
+// Some operations need an allocator.
 
-  protected ClusterFixture(FixtureBuilder  builder) throws Exception {
+allocator = RootAllocatorFactory.newRoot(config);
+  }
+
+  private String configureZk(FixtureBuilder builder) {
 
 // Start ZK if requested.
 
+String zkConnect = null;
 if (builder.zkHelper != null) {
+  // Case where the test itself started ZK and we're only using it.
+
   zkHelper = builder.zkHelper;
   ownsZK = false;
-} else if (builder.zkCount > 0) {
-  zkHelper = new ZookeeperHelper(true);
-  zkHelper.startZookeeper(builder.zkCount);
+} else if (builder.localZkCount > 0) {
+  // Case where we need a local ZK just for this test cluster.
+
+  zkHelper = new ZookeeperHelper("dummy");
+  zkHelper.startZookeeper(builder.localZkCount);
   ownsZK = true;
 }
+if (zkHelper != null) {
+  zkConnect = zkHelper.getConnectionString();
+  // Forced to disable this, because currently we leak memory which is 
a known issue for query cancellations.
+  // Setting this causes unittests to fail.
+  
builder.configProperty(ExecConstants.RETURN_ERROR_FOR_FAILURE_IN_CANCELLED_FRAGMENTS,
 true);
+}
+return zkConnect;
--- End diff --

Good catch. Fixed.

Lots of this code was adapted from elsewhere and I didn't always follow 
through the exact flow. Thanks for doing so.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102847232
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
+
+  p = Pattern.compile("rowType = RecordType\\((.*)\\): (rowcount .*)");
+  m = p.matcher(tail);
--- End diff --

Why is that? Spaces are literals. Colons are literal in this location, they 
have meaning only in a pattern like (?:X). What am I missing?


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102848531
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
+
+  p = Pattern.compile("rowType = RecordType\\((.*)\\): (rowcount .*)");
+  m = p.matcher(tail);
+  if ( m.matches() ) {
+columns = parseCols(m.group(1));
+tail = m.group(2);
+  }
+
+  p = Pattern.compile( "rowcount = ([\\d.E]+), cumulative cost = 
\\{([\\d.E]+) rows, ([\\d.E]+) cpu, ([\\d.E]+) io, ([\\d.E]+) network, 
([\\d.E]+) memory\\}, id = (\\d+)");
+  m = p.matcher(tail);
+  if (! m.matches()) {
+throw new IllegalStateException("Could not parse costs: " + tail );
+  }
+  estRows = Double.parseDouble(m.group(1));
+  estRowCost = Double.parseDouble(m.group(2));
+  estCpuCost = Double.parseDouble(m.group(3));
+  estIOCost = Double.parseDouble(m.group(4));
+  estNetCost = Double.parseDouble(m.group(5));
+  estMemoryCost = Double.parseDouble(m.group(6));
+  id = Integer.parseInt(m.group(7));
+}
+
+public void printTree(String indent) {
+  new TreePrinter().visit(this);
+}
+
+public OpDefInfo(int major, int id) {
+  majorId = major;
+  stepId = id;
+}
+
+@Override
+public String toString() {
+  String head = "[OpDefInfo " + majorId + "-" + stepId + ": " + name;
+  if ( isInferred ) {
+head += " (" + opName + ")";
+  }
+  return head + "]";
+}
+  }
+
+  /**
+   * Visit a tree of operator definitions to support 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102839491
  
--- Diff: common/src/test/java/org/apache/drill/test/DrillTest.java ---
@@ -69,6 +71,25 @@
 
   @Rule public TestName TEST_NAME = new TestName();
 
+  /**
+   * Option to cause tests to produce verbose output. Many tests provide
+   * detailed information to stdout when enabled. To enable:
+   * 
+   * java ... -Dtest.verbose=true ...
+   */
+  public static final String VERBOSE_OUTPUT = "test.verbose";
+
+  protected static final boolean verbose = 
Boolean.parseBoolean(System.getProperty(VERBOSE_OUTPUT));
+
+  /**
+   * Output destination for verbose tset output. Rather than using
--- End diff --

Fixed.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102845391
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -42,44 +48,207 @@
 public class ProfileParser {
 
   JsonObject profile;
+  String query;
   List plans;
+  List operations;
+  Map fragments = new HashMap<>();
+  private List topoOrder;
 
   public ProfileParser( File file ) throws IOException {
 try (FileReader fileReader = new FileReader(file);
  JsonReader reader = Json.createReader(fileReader)) {
   profile = (JsonObject) reader.read();
 }
+
+parse();
+  }
+
+  private void parse() {
+parseQuery();
+parsePlans();
+buildFrags();
+parseFragProfiles();
+mapOpProfiles();
+aggregateOpers();
+buildTree();
+  }
+
+  private void parseQuery() {
+query = profile.getString("query");
+query = query.replace("//n", "\n");
+  }
+
+  /**
+   * Parse a text version of the plan as it appears in the JSON
+   * query profile.
+   */
+
+  private static class PlanParser {
+
+List plans = new ArrayList<>();
+List operations = new ArrayList<>();
+List sorted = new ArrayList<>();
+
+public void parsePlans(String plan) {
+  plans = new ArrayList<>( );
+  String parts[] = plan.split("\n");
+  for (String part : parts) {
+plans.add(part);
+OpDefInfo opDef = new OpDefInfo( part );
+operations.add(opDef);
+  }
+  sortList();
+}
+
+public void sortList() {
+  List raw = new ArrayList<>( );
+  raw.addAll( operations );
+  Collections.sort( raw, new Comparator() {
+@Override
+public int compare(OpDefInfo o1, OpDefInfo o2) {
+  int result = Integer.compare(o1.majorId, o2.majorId);
+  if ( result == 0 ) {
+result = Integer.compare(o1.stepId, o2.stepId);
+  }
+  return result;
+}
+  });
+  int currentFrag = 0;
+  int currentStep = 0;
+  for ( OpDefInfo opDef : raw ) {
+if ( currentFrag < opDef.majorId ) {
+  currentFrag++;
+  OpDefInfo sender = new OpDefInfo( currentFrag, 0 );
+  sender.isInferred = true;
+  sender.name = "Sender";
+  sorted.add(sender);
+  currentStep = 1;
+  opDef.inferredParent = sender;
+  sender.children.add( opDef );
+}
+if ( opDef.stepId > currentStep ) {
+  OpDefInfo unknown = new OpDefInfo( currentFrag, currentStep );
+  unknown.isInferred = true;
+  unknown.name = "Unknown";
+  sorted.add(unknown);
+  opDef.inferredParent = unknown;
+  unknown.children.add( opDef );
+}
+sorted.add( opDef );
+currentStep = opDef.stepId + 1;
+  }
+}
+  }
+
+  /**
+   * Parse the plan portion of the query profile.
+   */
+
+  private void parsePlans() {
+PlanParser parser = new PlanParser();
+String plan = getPlan( );
+parser.parsePlans(plan);
+plans = parser.plans;
+topoOrder = parser.operations;
+operations = parser.sorted;
+  }
+
+  private void buildFrags() {
+for (OpDefInfo opDef : operations) {
+  FragInfo major = fragments.get(opDef.majorId);
+  if (major == null) {
+major = new FragInfo(opDef.majorId);
+fragments.put(opDef.majorId, major);
+  }
+  major.ops.add(opDef);
+}
+  }
+
+  private static List parseCols(String cols) {
+String parts[] = cols.split( ", " );
+List fields = new ArrayList<>( );
+for ( String part : parts ) {
+  String halves[] = part.split( " " );
+  fields.add( new FieldDef( halves[1], halves[0] ) );
+}
+return fields;
+  }
+
+  private void parseFragProfiles() {
+JsonArray frags = getFragmentProfile( );
+for (JsonObject fragProfile : frags.getValuesAs(JsonObject.class)) {
+  int mId = fragProfile.getInt("majorFragmentId");
+  FragInfo major = fragments.get(mId);
+  major.parse(fragProfile);
+}
+  }
+
+  private void mapOpProfiles() {
+for (FragInfo major : fragments.values()) {
+  for (MinorFragInfo minor : 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102841504
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -99,33 +111,83 @@
   // behavior. Production default is DEFAULT_SCAN_THREADS
 
   put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
+
+  // Define a useful root location for the ZK persistent
+  // storage. Profiles will go here when running in distributed
+  // mode.
+
+  
put(ZookeeperPersistentStoreProvider.DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT, 
"/tmp/drill/log");
 }
   };
 
   public static final String DEFAULT_BIT_NAME = "drillbit";
 
   private DrillConfig config;
-  private Map bits = new HashMap<>();
+  private Map bits = new HashMap<>();
   private Drillbit defaultDrillbit;
   private BufferAllocator allocator;
   private boolean ownsZK;
   private ZookeeperHelper zkHelper;
   private RemoteServiceSet serviceSet;
-  private String dfsTestTmpSchemaLocation;
+  private File dfsTestTempDir;
   protected List clients = new ArrayList<>();
+  private boolean usesZk;
+  private boolean preserveLocalFiles;
+  private boolean isLocal;
+
+  /**
+   * Temporary directories created for this test cluster.
+   * Each is removed when closing the cluster.
+   */
+
+  private List tempDirs = new ArrayList<>();
+
+  ClusterFixture(FixtureBuilder builder) {
+
+String zkConnect = configureZk(builder);
+try {
+  createConfig(builder, zkConnect);
--- End diff --

Good catch. Moved creating the allocator earlier.

The `getServiceSetWithFullCache` thing is existing code. Only used in the 
original `BaseTestQuery` and here (which I adapted from `BaseTestQuery`.) Went 
ahead and cleaned up the signature in both places.

Not only that, but `getServiceSetWithFullCache()` is the same as 
`getLocalServiceSet()`. Probably some legacy reason the "cache" version 
existed. Since none of us know that reason, went ahead and removed the "cache" 
version.

Also removed the config option that enable the (no op) cached version.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102843879
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -295,8 +396,96 @@ public void close() throws Exception {
 if (ex != null) {
   throw ex;
 }
+
+// Delete any local files, if we wrote to the local
+// persistent store. But, leave the files if the user wants
+// to review them, for debugging, say. Note that, even if the
+// files are preserved here, they will be removed when the
+// next cluster fixture starts, else the CTTAS initialization
+// will fail.
+
+if (! preserveLocalFiles) {
+try {
+  removeLocalFiles();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
+}
+
+// Remove temporary directories created for this cluster session.
+
+try {
+  removeTempDirs();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
   }
 
+  /**
+   * Removes files stored locally in the "local store provider."
+   * Required because CTTAS setup fails if these files are left from one
+   * run to the next.
+   *
+   * @throws IOException if a directory cannot be deleted
+   */
+
+  private void removeLocalFiles() throws IOException {
+
+// Don't delete if this is not a local Drillbit.
+
+if (! isLocal) {
+  return;
+}
+
+// Don't delete if we did not write.
+
+if (! 
config.getBoolean(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE)) {
+  return;
--- End diff --

Good point. The proper rule probably is that if we are using the local 
store, we need to clean it up prior to test start. Else, the new dynamic UDF 
stuff throw an error. So, for start-up, removes files is we are using the local 
store, write or not.

A separate option will leave files on exit when desired. (The options to 
leave files us usually just turned on during debugging...)


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102843336
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -295,8 +396,96 @@ public void close() throws Exception {
 if (ex != null) {
   throw ex;
--- End diff --

Fixed.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102844110
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -295,8 +396,96 @@ public void close() throws Exception {
 if (ex != null) {
   throw ex;
 }
+
+// Delete any local files, if we wrote to the local
+// persistent store. But, leave the files if the user wants
+// to review them, for debugging, say. Note that, even if the
+// files are preserved here, they will be removed when the
+// next cluster fixture starts, else the CTTAS initialization
+// will fail.
+
+if (! preserveLocalFiles) {
+try {
+  removeLocalFiles();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
+}
+
+// Remove temporary directories created for this cluster session.
+
+try {
+  removeTempDirs();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
   }
 
+  /**
+   * Removes files stored locally in the "local store provider."
+   * Required because CTTAS setup fails if these files are left from one
+   * run to the next.
+   *
+   * @throws IOException if a directory cannot be deleted
+   */
+
+  private void removeLocalFiles() throws IOException {
+
+// Don't delete if this is not a local Drillbit.
+
+if (! isLocal) {
+  return;
+}
+
+// Don't delete if we did not write.
+
+if (! 
config.getBoolean(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE)) {
+  return;
+}
+
+// Remove the local files if they exist.
+
+String localStoreLocation = 
config.getString(ExecConstants.SYS_STORE_PROVIDER_LOCAL_PATH);
+File storeDir = new File(localStoreLocation);
+if (! storeDir.exists()) {
+  return;
+}
+if (storeDir.exists()) {
--- End diff --

Fixed. It is a nuisance that `deleteDirectory` throws an exception if the 
directory is already deleted...


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/753#discussion_r102845074
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -42,44 +48,207 @@
 public class ProfileParser {
 
   JsonObject profile;
+  String query;
   List plans;
+  List operations;
+  Map fragments = new HashMap<>();
+  private List topoOrder;
 
   public ProfileParser( File file ) throws IOException {
 try (FileReader fileReader = new FileReader(file);
  JsonReader reader = Json.createReader(fileReader)) {
   profile = (JsonObject) reader.read();
 }
+
+parse();
+  }
+
+  private void parse() {
+parseQuery();
+parsePlans();
+buildFrags();
+parseFragProfiles();
+mapOpProfiles();
+aggregateOpers();
+buildTree();
+  }
+
+  private void parseQuery() {
+query = profile.getString("query");
+query = query.replace("//n", "\n");
+  }
+
+  /**
+   * Parse a text version of the plan as it appears in the JSON
+   * query profile.
+   */
+
+  private static class PlanParser {
+
+List plans = new ArrayList<>();
+List operations = new ArrayList<>();
+List sorted = new ArrayList<>();
+
+public void parsePlans(String plan) {
+  plans = new ArrayList<>( );
+  String parts[] = plan.split("\n");
+  for (String part : parts) {
+plans.add(part);
+OpDefInfo opDef = new OpDefInfo( part );
+operations.add(opDef);
+  }
+  sortList();
+}
+
+public void sortList() {
+  List raw = new ArrayList<>( );
+  raw.addAll( operations );
+  Collections.sort( raw, new Comparator() {
+@Override
+public int compare(OpDefInfo o1, OpDefInfo o2) {
+  int result = Integer.compare(o1.majorId, o2.majorId);
+  if ( result == 0 ) {
+result = Integer.compare(o1.stepId, o2.stepId);
+  }
+  return result;
+}
+  });
+  int currentFrag = 0;
+  int currentStep = 0;
+  for ( OpDefInfo opDef : raw ) {
+if ( currentFrag < opDef.majorId ) {
+  currentFrag++;
+  OpDefInfo sender = new OpDefInfo( currentFrag, 0 );
+  sender.isInferred = true;
+  sender.name = "Sender";
+  sorted.add(sender);
+  currentStep = 1;
+  opDef.inferredParent = sender;
+  sender.children.add( opDef );
+}
+if ( opDef.stepId > currentStep ) {
+  OpDefInfo unknown = new OpDefInfo( currentFrag, currentStep );
+  unknown.isInferred = true;
+  unknown.name = "Unknown";
+  sorted.add(unknown);
+  opDef.inferredParent = unknown;
+  unknown.children.add( opDef );
+}
+sorted.add( opDef );
+currentStep = opDef.stepId + 1;
+  }
+}
+  }
+
+  /**
+   * Parse the plan portion of the query profile.
+   */
+
+  private void parsePlans() {
+PlanParser parser = new PlanParser();
+String plan = getPlan( );
+parser.parsePlans(plan);
+plans = parser.plans;
+topoOrder = parser.operations;
+operations = parser.sorted;
+  }
--- End diff --

It is confusing... The query profile lists operators in topological order, 
so the original order is the topo order. We then sort the operators by operator 
ID so we can retrieve them by ID. That's what the operators list is.

Added Javadoc to explain this a bit.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102610181
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -42,44 +48,207 @@
 public class ProfileParser {
 
   JsonObject profile;
+  String query;
   List plans;
+  List operations;
+  Map fragments = new HashMap<>();
+  private List topoOrder;
 
   public ProfileParser( File file ) throws IOException {
 try (FileReader fileReader = new FileReader(file);
  JsonReader reader = Json.createReader(fileReader)) {
   profile = (JsonObject) reader.read();
 }
+
+parse();
+  }
+
+  private void parse() {
+parseQuery();
+parsePlans();
+buildFrags();
+parseFragProfiles();
+mapOpProfiles();
+aggregateOpers();
+buildTree();
+  }
+
+  private void parseQuery() {
+query = profile.getString("query");
+query = query.replace("//n", "\n");
+  }
+
+  /**
+   * Parse a text version of the plan as it appears in the JSON
+   * query profile.
+   */
+
+  private static class PlanParser {
+
+List plans = new ArrayList<>();
+List operations = new ArrayList<>();
+List sorted = new ArrayList<>();
+
+public void parsePlans(String plan) {
+  plans = new ArrayList<>( );
+  String parts[] = plan.split("\n");
+  for (String part : parts) {
+plans.add(part);
+OpDefInfo opDef = new OpDefInfo( part );
+operations.add(opDef);
+  }
+  sortList();
+}
+
+public void sortList() {
--- End diff --

can be private.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102589695
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -99,33 +111,83 @@
   // behavior. Production default is DEFAULT_SCAN_THREADS
 
   put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
+
+  // Define a useful root location for the ZK persistent
+  // storage. Profiles will go here when running in distributed
+  // mode.
+
+  
put(ZookeeperPersistentStoreProvider.DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT, 
"/tmp/drill/log");
 }
   };
 
   public static final String DEFAULT_BIT_NAME = "drillbit";
 
   private DrillConfig config;
-  private Map bits = new HashMap<>();
+  private Map bits = new HashMap<>();
   private Drillbit defaultDrillbit;
   private BufferAllocator allocator;
   private boolean ownsZK;
   private ZookeeperHelper zkHelper;
   private RemoteServiceSet serviceSet;
-  private String dfsTestTmpSchemaLocation;
+  private File dfsTestTempDir;
   protected List clients = new ArrayList<>();
+  private boolean usesZk;
+  private boolean preserveLocalFiles;
+  private boolean isLocal;
+
+  /**
+   * Temporary directories created for this test cluster.
+   * Each is removed when closing the cluster.
+   */
+
+  private List tempDirs = new ArrayList<>();
+
+  ClusterFixture(FixtureBuilder builder) {
+
+String zkConnect = configureZk(builder);
+try {
+  createConfig(builder, zkConnect);
--- End diff --

In createConfig we are calling `getServiceSetWithFullCache(config, 
allocator)`. But we are creating `allocator` later on.  We should assign 
allocator before calling above method. But allocator creation also takes in 
config so probably we have to do inside `createConfig` method. On a side note I 
don't see `getServiceSetWithFullCache(config, allocator)` using allocator or 
config anyway. Not sure why we are passing in first place.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102587851
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -99,33 +111,83 @@
   // behavior. Production default is DEFAULT_SCAN_THREADS
 
   put(ExecConstants.SCAN_THREADPOOL_SIZE, 4);
+
+  // Define a useful root location for the ZK persistent
+  // storage. Profiles will go here when running in distributed
+  // mode.
+
+  
put(ZookeeperPersistentStoreProvider.DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT, 
"/tmp/drill/log");
 }
   };
 
   public static final String DEFAULT_BIT_NAME = "drillbit";
 
   private DrillConfig config;
-  private Map bits = new HashMap<>();
+  private Map bits = new HashMap<>();
   private Drillbit defaultDrillbit;
   private BufferAllocator allocator;
   private boolean ownsZK;
   private ZookeeperHelper zkHelper;
   private RemoteServiceSet serviceSet;
-  private String dfsTestTmpSchemaLocation;
+  private File dfsTestTempDir;
   protected List clients = new ArrayList<>();
+  private boolean usesZk;
+  private boolean preserveLocalFiles;
+  private boolean isLocal;
+
+  /**
+   * Temporary directories created for this test cluster.
+   * Each is removed when closing the cluster.
+   */
+
+  private List tempDirs = new ArrayList<>();
+
+  ClusterFixture(FixtureBuilder builder) {
+
+String zkConnect = configureZk(builder);
+try {
+  createConfig(builder, zkConnect);
+  startDrillbits(builder);
+  applyOptions(builder);
+} catch (Exception e) {
+  // Translate exceptions to unchecked to avoid cluttering
+  // tests. Failures will simply fail the test itself.
+
+  throw new IllegalStateException( "Cluster fixture setup failed", e );
+}
+
+// Some operations need an allocator.
 
-  protected ClusterFixture(FixtureBuilder  builder) throws Exception {
+allocator = RootAllocatorFactory.newRoot(config);
+  }
+
+  private String configureZk(FixtureBuilder builder) {
 
 // Start ZK if requested.
 
+String zkConnect = null;
 if (builder.zkHelper != null) {
+  // Case where the test itself started ZK and we're only using it.
+
   zkHelper = builder.zkHelper;
   ownsZK = false;
-} else if (builder.zkCount > 0) {
-  zkHelper = new ZookeeperHelper(true);
-  zkHelper.startZookeeper(builder.zkCount);
+} else if (builder.localZkCount > 0) {
+  // Case where we need a local ZK just for this test cluster.
+
+  zkHelper = new ZookeeperHelper("dummy");
+  zkHelper.startZookeeper(builder.localZkCount);
   ownsZK = true;
 }
+if (zkHelper != null) {
+  zkConnect = zkHelper.getConnectionString();
+  // Forced to disable this, because currently we leak memory which is 
a known issue for query cancellations.
+  // Setting this causes unittests to fail.
+  
builder.configProperty(ExecConstants.RETURN_ERROR_FOR_FAILURE_IN_CANCELLED_FRAGMENTS,
 true);
+}
+return zkConnect;
--- End diff --

We can set the `ZK_CONNECTION` property inside configProperty here itself. 
That way we don't have to get the connectString from here and pass to 
createConfig where we are doing `null` check for `zkConnect` string.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102585253
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -295,8 +396,96 @@ public void close() throws Exception {
 if (ex != null) {
   throw ex;
 }
+
+// Delete any local files, if we wrote to the local
+// persistent store. But, leave the files if the user wants
+// to review them, for debugging, say. Note that, even if the
+// files are preserved here, they will be removed when the
+// next cluster fixture starts, else the CTTAS initialization
+// will fail.
+
+if (! preserveLocalFiles) {
+try {
+  removeLocalFiles();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
+}
+
+// Remove temporary directories created for this cluster session.
+
+try {
+  removeTempDirs();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
   }
 
+  /**
+   * Removes files stored locally in the "local store provider."
+   * Required because CTTAS setup fails if these files are left from one
+   * run to the next.
+   *
+   * @throws IOException if a directory cannot be deleted
+   */
+
+  private void removeLocalFiles() throws IOException {
+
+// Don't delete if this is not a local Drillbit.
+
+if (! isLocal) {
+  return;
+}
+
+// Don't delete if we did not write.
+
+if (! 
config.getBoolean(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE)) {
+  return;
+}
+
+// Remove the local files if they exist.
+
+String localStoreLocation = 
config.getString(ExecConstants.SYS_STORE_PROVIDER_LOCAL_PATH);
+File storeDir = new File(localStoreLocation);
+if (! storeDir.exists()) {
+  return;
+}
+if (storeDir.exists()) {
--- End diff --

Check not needed since we are already checking above. Also we can directly 
use the `removeDir` method below.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102600191
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ExampleTest.java ---
@@ -0,0 +1,243 @@
+/*
+ * 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 static org.junit.Assert.*;
+
--- End diff --

please remove wildcard import since we are only using `assertEquals`


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102369640
  
--- Diff: common/src/test/java/org/apache/drill/test/DrillTest.java ---
@@ -69,6 +71,25 @@
 
   @Rule public TestName TEST_NAME = new TestName();
 
+  /**
+   * Option to cause tests to produce verbose output. Many tests provide
+   * detailed information to stdout when enabled. To enable:
+   * 
+   * java ... -Dtest.verbose=true ...
+   */
+  public static final String VERBOSE_OUTPUT = "test.verbose";
+
+  protected static final boolean verbose = 
Boolean.parseBoolean(System.getProperty(VERBOSE_OUTPUT));
+
+  /**
+   * Output destination for verbose tset output. Rather than using
--- End diff --

typo `test`


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102603020
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -42,44 +48,207 @@
 public class ProfileParser {
 
   JsonObject profile;
+  String query;
   List plans;
+  List operations;
+  Map fragments = new HashMap<>();
+  private List topoOrder;
 
   public ProfileParser( File file ) throws IOException {
 try (FileReader fileReader = new FileReader(file);
  JsonReader reader = Json.createReader(fileReader)) {
   profile = (JsonObject) reader.read();
 }
+
+parse();
+  }
+
+  private void parse() {
+parseQuery();
+parsePlans();
+buildFrags();
+parseFragProfiles();
+mapOpProfiles();
+aggregateOpers();
+buildTree();
+  }
+
+  private void parseQuery() {
+query = profile.getString("query");
+query = query.replace("//n", "\n");
+  }
+
+  /**
+   * Parse a text version of the plan as it appears in the JSON
+   * query profile.
+   */
+
+  private static class PlanParser {
+
+List plans = new ArrayList<>();
+List operations = new ArrayList<>();
+List sorted = new ArrayList<>();
+
+public void parsePlans(String plan) {
+  plans = new ArrayList<>( );
+  String parts[] = plan.split("\n");
+  for (String part : parts) {
+plans.add(part);
+OpDefInfo opDef = new OpDefInfo( part );
+operations.add(opDef);
+  }
+  sortList();
+}
+
+public void sortList() {
+  List raw = new ArrayList<>( );
+  raw.addAll( operations );
+  Collections.sort( raw, new Comparator() {
+@Override
+public int compare(OpDefInfo o1, OpDefInfo o2) {
+  int result = Integer.compare(o1.majorId, o2.majorId);
+  if ( result == 0 ) {
+result = Integer.compare(o1.stepId, o2.stepId);
+  }
+  return result;
+}
+  });
+  int currentFrag = 0;
+  int currentStep = 0;
+  for ( OpDefInfo opDef : raw ) {
+if ( currentFrag < opDef.majorId ) {
+  currentFrag++;
+  OpDefInfo sender = new OpDefInfo( currentFrag, 0 );
+  sender.isInferred = true;
+  sender.name = "Sender";
+  sorted.add(sender);
+  currentStep = 1;
+  opDef.inferredParent = sender;
+  sender.children.add( opDef );
+}
+if ( opDef.stepId > currentStep ) {
+  OpDefInfo unknown = new OpDefInfo( currentFrag, currentStep );
+  unknown.isInferred = true;
+  unknown.name = "Unknown";
+  sorted.add(unknown);
+  opDef.inferredParent = unknown;
+  unknown.children.add( opDef );
+}
+sorted.add( opDef );
+currentStep = opDef.stepId + 1;
+  }
+}
+  }
+
+  /**
+   * Parse the plan portion of the query profile.
+   */
+
+  private void parsePlans() {
+PlanParser parser = new PlanParser();
+String plan = getPlan( );
+parser.parsePlans(plan);
+plans = parser.plans;
+topoOrder = parser.operations;
+operations = parser.sorted;
+  }
--- End diff --

wrong assignment ?` topoOrder = parser.sorted` and `operations = 
parser.operations` ?


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102584427
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -295,8 +396,96 @@ public void close() throws Exception {
 if (ex != null) {
   throw ex;
 }
+
+// Delete any local files, if we wrote to the local
+// persistent store. But, leave the files if the user wants
+// to review them, for debugging, say. Note that, even if the
+// files are preserved here, they will be removed when the
+// next cluster fixture starts, else the CTTAS initialization
+// will fail.
+
+if (! preserveLocalFiles) {
+try {
+  removeLocalFiles();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
+}
+
+// Remove temporary directories created for this cluster session.
+
+try {
+  removeTempDirs();
+} catch (Exception e) {
+  ex = ex == null ? e : ex;
+}
   }
 
+  /**
+   * Removes files stored locally in the "local store provider."
+   * Required because CTTAS setup fails if these files are left from one
+   * run to the next.
+   *
+   * @throws IOException if a directory cannot be deleted
+   */
+
+  private void removeLocalFiles() throws IOException {
+
+// Don't delete if this is not a local Drillbit.
+
+if (! isLocal) {
+  return;
+}
+
+// Don't delete if we did not write.
+
+if (! 
config.getBoolean(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE)) {
+  return;
--- End diff --

If the previous test wants to preserve the file then we will not delete 
upon cluster shutdown. But then if next test starts and doesn't want to save 
profiles this option will be `false` by default and on cluster startup we will 
still not delete the old files ? which is the intention in `startDrillbits()`


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102612813
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -42,44 +48,207 @@
 public class ProfileParser {
 
   JsonObject profile;
+  String query;
   List plans;
+  List operations;
+  Map fragments = new HashMap<>();
+  private List topoOrder;
 
   public ProfileParser( File file ) throws IOException {
 try (FileReader fileReader = new FileReader(file);
  JsonReader reader = Json.createReader(fileReader)) {
   profile = (JsonObject) reader.read();
 }
+
+parse();
+  }
+
+  private void parse() {
+parseQuery();
+parsePlans();
+buildFrags();
+parseFragProfiles();
+mapOpProfiles();
+aggregateOpers();
+buildTree();
+  }
+
+  private void parseQuery() {
+query = profile.getString("query");
+query = query.replace("//n", "\n");
+  }
+
+  /**
+   * Parse a text version of the plan as it appears in the JSON
+   * query profile.
+   */
+
+  private static class PlanParser {
+
+List plans = new ArrayList<>();
+List operations = new ArrayList<>();
+List sorted = new ArrayList<>();
+
+public void parsePlans(String plan) {
+  plans = new ArrayList<>( );
+  String parts[] = plan.split("\n");
+  for (String part : parts) {
+plans.add(part);
+OpDefInfo opDef = new OpDefInfo( part );
+operations.add(opDef);
+  }
+  sortList();
+}
+
+public void sortList() {
+  List raw = new ArrayList<>( );
+  raw.addAll( operations );
+  Collections.sort( raw, new Comparator() {
+@Override
+public int compare(OpDefInfo o1, OpDefInfo o2) {
+  int result = Integer.compare(o1.majorId, o2.majorId);
+  if ( result == 0 ) {
+result = Integer.compare(o1.stepId, o2.stepId);
+  }
+  return result;
+}
+  });
+  int currentFrag = 0;
+  int currentStep = 0;
+  for ( OpDefInfo opDef : raw ) {
+if ( currentFrag < opDef.majorId ) {
+  currentFrag++;
+  OpDefInfo sender = new OpDefInfo( currentFrag, 0 );
+  sender.isInferred = true;
+  sender.name = "Sender";
+  sorted.add(sender);
+  currentStep = 1;
+  opDef.inferredParent = sender;
+  sender.children.add( opDef );
+}
+if ( opDef.stepId > currentStep ) {
+  OpDefInfo unknown = new OpDefInfo( currentFrag, currentStep );
+  unknown.isInferred = true;
+  unknown.name = "Unknown";
+  sorted.add(unknown);
+  opDef.inferredParent = unknown;
+  unknown.children.add( opDef );
+}
+sorted.add( opDef );
+currentStep = opDef.stepId + 1;
+  }
+}
+  }
+
+  /**
+   * Parse the plan portion of the query profile.
+   */
+
+  private void parsePlans() {
+PlanParser parser = new PlanParser();
+String plan = getPlan( );
+parser.parsePlans(plan);
+plans = parser.plans;
+topoOrder = parser.operations;
+operations = parser.sorted;
+  }
+
+  private void buildFrags() {
+for (OpDefInfo opDef : operations) {
+  FragInfo major = fragments.get(opDef.majorId);
+  if (major == null) {
+major = new FragInfo(opDef.majorId);
+fragments.put(opDef.majorId, major);
+  }
+  major.ops.add(opDef);
+}
+  }
+
+  private static List parseCols(String cols) {
+String parts[] = cols.split( ", " );
+List fields = new ArrayList<>( );
+for ( String part : parts ) {
+  String halves[] = part.split( " " );
+  fields.add( new FieldDef( halves[1], halves[0] ) );
+}
+return fields;
+  }
+
+  private void parseFragProfiles() {
+JsonArray frags = getFragmentProfile( );
+for (JsonObject fragProfile : frags.getValuesAs(JsonObject.class)) {
+  int mId = fragProfile.getInt("majorFragmentId");
+  FragInfo major = fragments.get(mId);
+  major.parse(fragProfile);
+}
+  }
+
+  private void mapOpProfiles() {
+for (FragInfo major : fragments.values()) {
+  for (MinorFragInfo minor : 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102581476
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -295,8 +396,96 @@ public void close() throws Exception {
 if (ex != null) {
   throw ex;
--- End diff --

It looks like we are doing best effort in release or closing all the 
resources. So this condition should be in the end after removing local files 
and temp dirs.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102618884
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
+
+  p = Pattern.compile("rowType = RecordType\\((.*)\\): (rowcount .*)");
+  m = p.matcher(tail);
+  if ( m.matches() ) {
+columns = parseCols(m.group(1));
+tail = m.group(2);
+  }
+
+  p = Pattern.compile( "rowcount = ([\\d.E]+), cumulative cost = 
\\{([\\d.E]+) rows, ([\\d.E]+) cpu, ([\\d.E]+) io, ([\\d.E]+) network, 
([\\d.E]+) memory\\}, id = (\\d+)");
+  m = p.matcher(tail);
+  if (! m.matches()) {
+throw new IllegalStateException("Could not parse costs: " + tail );
+  }
+  estRows = Double.parseDouble(m.group(1));
+  estRowCost = Double.parseDouble(m.group(2));
+  estCpuCost = Double.parseDouble(m.group(3));
+  estIOCost = Double.parseDouble(m.group(4));
+  estNetCost = Double.parseDouble(m.group(5));
+  estMemoryCost = Double.parseDouble(m.group(6));
+  id = Integer.parseInt(m.group(7));
+}
+
+public void printTree(String indent) {
+  new TreePrinter().visit(this);
+}
+
+public OpDefInfo(int major, int id) {
+  majorId = major;
+  stepId = id;
+}
+
+@Override
+public String toString() {
+  String head = "[OpDefInfo " + majorId + "-" + stepId + ": " + name;
+  if ( isInferred ) {
+head += " (" + opName + ")";
+  }
+  return head + "]";
+}
+  }
+
+  /**
+   * Visit a tree of operator definitions to support 

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102613259
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
--- End diff --

Group 3 is looking for empty spaces. Can indent.length() be < 4 (like empty 
string) ? In which case globalLevel will be -ve which will cause issue in 
`buildTree()`. Looks like globalLevel needs to be atleast 1. Better to have a 
check here ?


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/drill/pull/753#discussion_r102607033
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/test/ProfileParser.java ---
@@ -138,9 +414,208 @@ public long getMetric(int id) {
 }
   }
 
-  public Map getOpInfo( ) {
+  /**
+   * Information about an operator definition: the plan-time information
+   * that appears in the plan portion of the profile. Also holds the
+   * "actuals" from the minor fragment portion of the profile.
+   * Allows integrating the "planned" vs. "actual" performance of the
+   * query.
+   */
+
+  public static class OpDefInfo {
+public String opName;
+public boolean isInferred;
+public int majorId;
+public int stepId;
+public String args;
+public List columns;
+public int globalLevel;
+public int localLevel;
+public int id;
+public int branchId;
+public boolean isBranchRoot;
+public double estMemoryCost;
+public double estNetCost;
+public double estIOCost;
+public double estCpuCost;
+public double estRowCost;
+public double estRows;
+public String name;
+public long actualMemory;
+public int actualBatches;
+public long actualRows;
+public OpDefInfo inferredParent;
+public List opExecs = new ArrayList<>( );
+public List children = new ArrayList<>( );
+
+// 00-00Screen : rowType = RecordType(VARCHAR(10) Year, 
VARCHAR(65536) Month, VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) 
LOB, CHAR(10) Gateway, BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) 
Week_end_date, BIGINT Usage_Cnt): \
+// rowcount = 100.0, cumulative cost = {7.42124276972414E9 rows, 
7.663067406383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129302
+//
+// 00-01  Project(Year=[$0], Month=[$1], Devices=[$2], Tier=[$3], 
LOB=[$4], Gateway=[$5], Day=[$6], Hour=[$7], Week=[$8], Week_end_date=[$9], 
Usage_Cnt=[$10]) :
+// rowType = RecordType(VARCHAR(10) Year, VARCHAR(65536) Month, 
VARCHAR(100) Devices, VARCHAR(100) Tier, VARCHAR(100) LOB, CHAR(10) Gateway, 
BIGINT Day, BIGINT Hour, INTEGER Week, VARCHAR(100) Week_end_date, BIGINT 
Usage_Cnt): rowcount = 100.0, cumulative cost = {7.42124275972414E9 rows, 
7.663067405383167E10 cpu, 0.0 io, 2.24645048816E10 network, 2.692766612982188E8 
memory}, id = 129301
+
+public OpDefInfo(String plan) {
+  Pattern p = Pattern.compile( 
"^(\\d+)-(\\d+)(\\s+)(\\w+)(?:\\((.*)\\))?\\s*:\\s*(.*)$" );
+  Matcher m = p.matcher(plan);
+  if (!m.matches()) {
+throw new IllegalStateException( "Could not parse plan: " + plan );
+  }
+  majorId = Integer.parseInt(m.group(1));
+  stepId = Integer.parseInt(m.group(2));
+  name = m.group(4);
+  args = m.group(5);
+  String tail = m.group(6);
+  String indent = m.group(3);
+  globalLevel = (indent.length() - 4) / 2;
+
+  p = Pattern.compile("rowType = RecordType\\((.*)\\): (rowcount .*)");
+  m = p.matcher(tail);
--- End diff --

Looks like this regex will be `rowType = 
RecordType\((.*)\):(\s*)\\?(\s*)(rowcount .*)` with extra `(\s*)\\?(\s*)` in 
between.


> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-5260) Refinements to new "Cluster Fixture" test framework

[ASF GitHub Bot (JIRA)]

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

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

GitHub user paul-rogers opened a pull request:

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

DRILL-5260: Extend "Cluster Fixture" test framework

- Config option to suppress printing of CSV and other output. (Allows
printing for single tests, not printing when running from Maven.)

- Parsing of query profiles to extract plan and run time information.

- Fix bug in log fixture when enabling logging for a package.

- Improved ZK support.

- Set up the new CTTAS default temporary workspace for tests.

- Clean up persistent storage files on disk to avoid CTTAS startup
failures.

- Provides a set of examples for how to use the cluster fixture.

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

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

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

https://github.com/apache/drill/pull/753.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 #753


commit 40080d663545c6f0a5a413cf18866611a2bc5b3f
Author: Paul Rogers 
Date:   2017-02-18T01:39:20Z

DRILL-5260: Extend "Cluster Fixture" test framework

- Config option to suppress printing of CSV and other output. (Allows
printing for single tests, not printing when running from Maven.)

- Parsing of query profiles to extract plan and run time information.

- Fix bug in log fixture when enabling logging for a package.

- Improved ZK support.

- Set up the new CTTAS default temporary workspace for tests.

- Clean up persistent storage files on disk to avoid CTTAS startup
failures.

- Provides a set of examples for how to use the cluster fixture.




> Refinements to new "Cluster Fixture" test framework
> ---
>
> Key: DRILL-5260
> URL: https://issues.apache.org/jira/browse/DRILL-5260
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.10
>Reporter: Paul Rogers
>Assignee: Paul Rogers
>Priority: Minor
> Fix For: 1.10
>
>
> Roll-up of a number of enhancements to the cluster fixture framework.
> * Config option to suppress printing of CSV and other output. (Allows 
> printing for single tests, not printing when running from Maven.)
> * Parsing of query profiles to extract plan and run time information.
> * Fix bug in log fixture when enabling logging for a package.
> * Improved ZK support.
> * Set up the new CTTAS default temporary workspace for tests.
> * Revise TestDrillbitResiliance to use the new framework.
> * Revise TestWindowFrame to to use the new framework.
> * Revise TestMergeJoinWithSchemaChanges to use the new framework.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)