[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. IMPALA-6957: calc thread resource requirement in planner This only factors in fragment execution threads. E.g. this does *not* try to account for the number of threads on the old Thrift RPC code path if that is enabled. This is loosely related to the old VCores estimate, but is different in that it: * Directly ties into the notion of required threads in ThreadResourceMgr. * Is a strict upper bound on the number of such threads, rather than an estimate. Does not include "optional" threads. ThreadResourceMgr in the backend bounds the number of "optional" threads per impalad, so the number of execution threads on a backend is limited by sum(required threads per query) + CpuInfo::num_cores() * FLAGS_num_threads_per_core DCHECKS in the backend enforce that the calculation is correct. They were actually hit in KuduScanNode because of some races in thread management leading to multiple "required" threads running. Now the first thread in the multithreaded scans never exits, which means that it's always safe for any of the other threads to exit early, which simplifies the logic a lot. Testing: Updated planner tests. Ran core tests. Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Reviewed-on: http://gerrit.cloudera.org:8080/10256 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/10398 --- M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scan-node.cc M be/src/exec/kudu-scan-node.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.cc M be/src/runtime/thread-resource-mgr.cc M be/src/runtime/thread-resource-mgr.h M be/src/scheduling/admission-controller.cc M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/Planner.thrift M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.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/PlanFragment.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/ResourceProfile.java M fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test M testdata/workloads/functional-planner/queries/PlannerTest/union.test M testdata/workloads/functional-query/queries/QueryTest/explain-level0.test M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test 43 files changed, 1,973 insertions(+), 1,927 deletions(-) Approvals: Impala Public Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: merged Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 21:03:19 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 19:24:19 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2474/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 15:47:00 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2475/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 15:47:06 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2474/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 15:39:55 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2473/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 09:51:38 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2473/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 15 May 2018 06:15:45 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 1: Code-Review+2 Fix one minor planner tests difference. -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 May 2018 06:15:55 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10398 to look at the new patch set (#2). Change subject: IMPALA-6957: calc thread resource requirement in planner .. IMPALA-6957: calc thread resource requirement in planner This only factors in fragment execution threads. E.g. this does *not* try to account for the number of threads on the old Thrift RPC code path if that is enabled. This is loosely related to the old VCores estimate, but is different in that it: * Directly ties into the notion of required threads in ThreadResourceMgr. * Is a strict upper bound on the number of such threads, rather than an estimate. Does not include "optional" threads. ThreadResourceMgr in the backend bounds the number of "optional" threads per impalad, so the number of execution threads on a backend is limited by sum(required threads per query) + CpuInfo::num_cores() * FLAGS_num_threads_per_core DCHECKS in the backend enforce that the calculation is correct. They were actually hit in KuduScanNode because of some races in thread management leading to multiple "required" threads running. Now the first thread in the multithreaded scans never exits, which means that it's always safe for any of the other threads to exit early, which simplifies the logic a lot. Testing: Updated planner tests. Ran core tests. Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Reviewed-on: http://gerrit.cloudera.org:8080/10256 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scan-node.cc M be/src/exec/kudu-scan-node.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.cc M be/src/runtime/thread-resource-mgr.cc M be/src/runtime/thread-resource-mgr.h M be/src/scheduling/admission-controller.cc M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/Planner.thrift M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.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/PlanFragment.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/ResourceProfile.java M fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test M testdata/workloads/functional-planner/queries/PlannerTest/union.test M testdata/workloads/functional-query/queries/QueryTest/explain-level0.test M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test 43 files changed, 1,973 insertions(+), 1,927 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/10398/2 -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2469/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 15 May 2018 02:20:19 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10398 ) Change subject: IMPALA-6957: calc thread resource requirement in planner .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2469/ -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 14 May 2018 22:35:16 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner
Hello Impala Public Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10398 to review the following change. Change subject: IMPALA-6957: calc thread resource requirement in planner .. IMPALA-6957: calc thread resource requirement in planner This only factors in fragment execution threads. E.g. this does *not* try to account for the number of threads on the old Thrift RPC code path if that is enabled. This is loosely related to the old VCores estimate, but is different in that it: * Directly ties into the notion of required threads in ThreadResourceMgr. * Is a strict upper bound on the number of such threads, rather than an estimate. Does not include "optional" threads. ThreadResourceMgr in the backend bounds the number of "optional" threads per impalad, so the number of execution threads on a backend is limited by sum(required threads per query) + CpuInfo::num_cores() * FLAGS_num_threads_per_core DCHECKS in the backend enforce that the calculation is correct. They were actually hit in KuduScanNode because of some races in thread management leading to multiple "required" threads running. Now the first thread in the multithreaded scans never exits, which means that it's always safe for any of the other threads to exit early, which simplifies the logic a lot. Testing: Updated planner tests. Ran core tests. Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Reviewed-on: http://gerrit.cloudera.org:8080/10256 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scan-node.cc M be/src/exec/kudu-scan-node.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.cc M be/src/runtime/thread-resource-mgr.cc M be/src/runtime/thread-resource-mgr.h M be/src/scheduling/admission-controller.cc M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/Planner.thrift M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.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/PlanFragment.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/ResourceProfile.java M fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test M testdata/workloads/functional-planner/queries/PlannerTest/union.test M testdata/workloads/functional-query/queries/QueryTest/explain-level0.test M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test 43 files changed, 1,973 insertions(+), 1,927 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/10398/1 -- To view, visit http://gerrit.cloudera.org:8080/10398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: newchange Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140 Gerrit-Change-Number: 10398 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
