[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 12: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Oct 2019 00:30:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..

IMPALA-8997: auto fallback to mt_dop=0

Add a temporary --mt_dop_auto_fallback to allow a graceful transition to
using mt_dop for workloads. When this flag is set, DML queries and joins
that would otherwise fail with an error when run with mt_dop > 0 fall
back to running with mt_dop = 0. This means that a user can set mt_dop
for their queries and it will only take effect when supported.

The behaviour generally does not change when this flag is not set,
with a couple of exceptions:
* I made mt_dop automatic for compute stats on all file formats
* mt_dop is allowed for single node plans with inserts. The
  quirky validatePlan() logic previously disallowed this but
  allowed joins in single node plans.

The checks added by this patch can be removed safely once mt_dop is
supported by default for all queries.

This includes some cleanup:
* isDmlStmt() was stale and incorrectly implemented.
* Various TreeNode methods did not return instances of subclasses of
  the requested class, which was strange. This fix is required to
  make 'contains(JoinNode.class)' work correctly. I checked the
  callsites of the fixed functions and none of them would be affected
  by this change because they specified a terminal class without
  any subclasses.
  I didn't actually use this fix in the end (I had to write a custom
  tree traversal in hasUnsupportedMtDopJoin()), but figured I would
  leave the improvement in here.

Testing:
Add some basic functional tests ensuring that the fallback takes
effect.

Run basic join and insert tests with this flag enabled.

Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Reviewed-on: http://gerrit.cloudera.org:8080/14344
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/common/TreeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-auto-fallback.test
M tests/custom_cluster/test_mt_dop.py
M tests/query_test/test_cancellation.py
16 files changed, 177 insertions(+), 62 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 12: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Oct 2019 20:06:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 12:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5124/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Oct 2019 20:06:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-21 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 11: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Oct 2019 18:57:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 11: Code-Review+1

carry the +1


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Oct 2019 16:09:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 11:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4796/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 Oct 2019 18:10:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-15 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14344

to look at the new patch set (#11).

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..

IMPALA-8997: auto fallback to mt_dop=0

Add a temporary --mt_dop_auto_fallback to allow a graceful transition to
using mt_dop for workloads. When this flag is set, DML queries and joins
that would otherwise fail with an error when run with mt_dop > 0 fall
back to running with mt_dop = 0. This means that a user can set mt_dop
for their queries and it will only take effect when supported.

The behaviour generally does not change when this flag is not set,
with a couple of exceptions:
* I made mt_dop automatic for compute stats on all file formats
* mt_dop is allowed for single node plans with inserts. The
  quirky validatePlan() logic previously disallowed this but
  allowed joins in single node plans.

The checks added by this patch can be removed safely once mt_dop is
supported by default for all queries.

This includes some cleanup:
* isDmlStmt() was stale and incorrectly implemented.
* Various TreeNode methods did not return instances of subclasses of
  the requested class, which was strange. This fix is required to
  make 'contains(JoinNode.class)' work correctly. I checked the
  callsites of the fixed functions and none of them would be affected
  by this change because they specified a terminal class without
  any subclasses.
  I didn't actually use this fix in the end (I had to write a custom
  tree traversal in hasUnsupportedMtDopJoin()), but figured I would
  leave the improvement in here.

Testing:
Add some basic functional tests ensuring that the fallback takes
effect.

Run basic join and insert tests with this flag enabled.

Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/common/TreeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-auto-fallback.test
M tests/custom_cluster/test_mt_dop.py
M tests/query_test/test_cancellation.py
16 files changed, 177 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14344/11
--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-15 Thread Tim Armstrong (Code Review)
Tim Armstrong has removed a vote on this change.

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 10:

Hit a flake: https://issues.apache.org/jira/browse/IMPALA-9022


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 04:35:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5062/


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 03:55:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 9:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4742/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 00:16:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 8:

(3 comments)

One failure was a flaky test that I think failed because it assumed that 
queries started up within 5 seconds of submission (which is untrue with 
admission control enabled). I marked that as serial.

The planner test failure was because the output on my local system was slightly 
different, I think because I didn't have a clean data load.

http://gerrit.cloudera.org:8080/#/c/14344/8/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/14344/8/be/src/common/global-flags.cc@273
PS8, Line 273: "(Experimental) If true, fall back to non-mt_dop if mt_dop 
is set and a query "
> nit: "if mt_dop query option is set"
Done


http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/main/java/org/apache/impala/service/Frontend.java@1427
PS8, Line 1427:  on Parquet/ORC tables
> nit: update comment
Done


http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@665
PS8, Line 665:   "select * from functional_parquet.alltypes", mtDop, 
effectiveMtDop);
> nit: how about:
Done



--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 23:36:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5062/ 
DRY_RUN=true


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 23:36:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-07 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14344

to look at the new patch set (#9).

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..

IMPALA-8997: auto fallback to mt_dop=0

Add a temporary --mt_dop_auto_fallback to allow a graceful transition to
using mt_dop for workloads. When this flag is set, DML queries and joins
that would otherwise fail with an error when run with mt_dop > 0 fall
back to running with mt_dop = 0. This means that a user can set mt_dop
for their queries and it will only take effect when supported.

The behaviour generally does not change when this flag is not set,
with a couple of exceptions:
* I made mt_dop automatic for compute stats on all file formats
* mt_dop is allowed for single node plans with inserts. The
  quirky validatePlan() logic previously disallowed this but
  allowed joins in single node plans.

The checks added by this patch can be removed safely once mt_dop is
supported by default for all queries.

This includes some cleanup:
* isDmlStmt() was stale and incorrectly implemented.
* Various TreeNode methods did not return instances of subclasses of
  the requested class, which was strange. This fix is required to
  make 'contains(JoinNode.class)' work correctly. I checked the
  callsites of the fixed functions and none of them would be affected
  by this change because they specified a terminal class without
  any subclasses.
  I didn't actually use this fix in the end (I had to write a custom
  tree traversal in hasUnsupportedMtDopJoin()), but figured I would
  leave the improvement in here.

Testing:
Add some basic functional tests ensuring that the fallback takes
effect.

Run basic join and insert tests with this flag enabled.

Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/common/TreeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-auto-fallback.test
M tests/custom_cluster/test_mt_dop.py
M tests/query_test/test_cancellation.py
16 files changed, 177 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14344/9
--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-04 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 8: Code-Review+1

(3 comments)

lgtm: mostly nits. Looked briefly at the failed build, looks unrelated. You can 
assume a +2 from my side once thats confirmed/fixed

http://gerrit.cloudera.org:8080/#/c/14344/8/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/14344/8/be/src/common/global-flags.cc@273
PS8, Line 273: "(Experimental) If true, fall back to non-mt_dop if mt_dop 
is set and a query "
nit: "if mt_dop query option is set"


http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/main/java/org/apache/impala/service/Frontend.java@1427
PS8, Line 1427:  on Parquet/ORC tables
nit: update comment


http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/14344/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@665
PS8, Line 665:   "select * from functional_parquet.alltypes", mtDop, 
effectiveMtDop);
nit: how about:
// MT_DOP is set automatically for COMPUTE STATS, but can be overridden by a
// user-provided MT_DOP.
int effectiveMtDop = (mtDop != -1) ? mtDop : 4;
testEffectiveMtDop("compute stats functional_parquet.alltypes", mtDop, 
effectiveMtDop);
testEffectiveMtDop("compute stats functional.alltypes", mtDop, effectiveMtDop);
testEffectiveMtDop("compute stats functional_kudu.alltypes", mtDop, 
effectiveMtDop);



--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 04 Oct 2019 23:47:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 8: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5046/


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 20:51:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5046/ 
DRY_RUN=true


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 16:32:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4709/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 01:26:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-02 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14344 )

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/14344/7/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

http://gerrit.cloudera.org:8080/#/c/14344/7/fe/src/main/java/org/apache/impala/planner/Planner.java@130
PS7, Line 130: // specified.
Explain the singleNodeExec() bit


http://gerrit.cloudera.org:8080/#/c/14344/7/fe/src/main/java/org/apache/impala/planner/Planner.java@132
PS7, Line 132: ctx_.getQueryOptions().isSetMt_dop(
This check is not needed since it's always set at this point in planning.


http://gerrit.cloudera.org:8080/#/c/14344/7/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/14344/7/fe/src/main/java/org/apache/impala/service/Frontend.java@1169
PS7, Line 1169:
Don't need to add blank line.



--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 00:40:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8997: auto fallback to mt dop=0

2019-10-02 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14344

to look at the new patch set (#8).

Change subject: IMPALA-8997: auto fallback to mt_dop=0
..

IMPALA-8997: auto fallback to mt_dop=0

Add a temporary --mt_dop_auto_fallback to allow a graceful transition to
using mt_dop for workloads. When this flag is set, DML queries and joins
that would otherwise fail with an error when run with mt_dop > 0 fall
back to running with mt_dop = 0. This means that a user can set mt_dop
for their queries and it will only take effect when supported.

The behaviour generally does not change when this flag is not set,
with a couple of exceptions:
* I made mt_dop automatic for compute stats on all file formats
* mt_dop is allowed for single node plans with inserts. The
  quirky validatePlan() logic previously disallowed this but
  allowed joins in single node plans.

The checks added by this patch can be removed safely once mt_dop is
supported by default for all queries.

This includes some cleanup:
* isDmlStmt() was stale and incorrectly implemented.
* Various TreeNode methods did not return instances of subclasses of
  the requested class, which was strange. This fix is required to
  make 'contains(JoinNode.class)' work correctly. I checked the
  callsites of the fixed functions and none of them would be affected
  by this change because they specified a terminal class without
  any subclasses.
  I didn't actually use this fix in the end (I had to write a custom
  tree traversal in hasUnsupportedMtDopJoin()), but figured I would
  leave the improvement in here.

Testing:
Add some basic functional tests ensuring that the fallback takes
effect.

Run basic join and insert tests with this flag enabled.

Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/common/TreeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-auto-fallback.test
M tests/custom_cluster/test_mt_dop.py
15 files changed, 162 insertions(+), 44 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14344/8
--
To view, visit http://gerrit.cloudera.org:8080/14344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0d73d8744059874293697c8e104891a10dba04d
Gerrit-Change-Number: 14344
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong