[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Bikramjeet Vig has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. IMPALA-10970: Fix criterion for classifying coordinator only query This patch fixes a bug in the criterion which decided whether a query can be considered as a coordinator only query. It did not consider the possibility of parallel plans and ended up mis-classifying some queries as coordinator only queries. This classification was used during scheduling when dedicated coordinators and executor groups are used and allowed coordinator queries to be scheduled only on the coordinator even in the absence of healthy executor groups. As a result of this bug, queries classified wrongly ended up with error code: NO_REGISTERED_BACKENDS. Testing: - Add new mt_dop test case for functional_query and pass - Ran and passed custom_cluster/test_coordinators, test_executor_groups Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Reviewed-on: http://gerrit.cloudera.org:8080/17937 Tested-by: Impala Public Jenkins Reviewed-by: Bikramjeet Vig --- M be/src/scheduling/scheduler.cc M common/thrift/Query.thrift M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test 3 files changed, 129 insertions(+), 2 deletions(-) Approvals: Impala Public Jenkins: Verified Bikramjeet Vig: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 5 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 4 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Tue, 30 Nov 2021 20:12:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 4 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Thu, 25 Nov 2021 18:36:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7675/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 4 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Thu, 25 Nov 2021 12:13:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
guojingfeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 4: Thank you for review this patch, I submit update and passed the preview-test-check. -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 4 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Tue, 23 Nov 2021 06:55:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9809/ : 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/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 4 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Fri, 19 Nov 2021 03:33:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
guojingfeng has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. IMPALA-10970: Fix criterion for classifying coordinator only query This patch fixes a bug in the criterion which decided whether a query can be considered as a coordinator only query. It did not consider the possibility of parallel plans and ended up mis-classifying some queries as coordinator only queries. This classification was used during scheduling when dedicated coordinators and executor groups are used and allowed coordinator queries to be scheduled only on the coordinator even in the absence of healthy executor groups. As a result of this bug, queries classified wrongly ended up with error code: NO_REGISTERED_BACKENDS. Testing: - Add new mt_dop test case for functional_query and pass - Ran and passed custom_cluster/test_coordinators, test_executor_groups Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 --- M be/src/scheduling/scheduler.cc M common/thrift/Query.thrift M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test 3 files changed, 129 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/17937/4 -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 4 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
guojingfeng has posted comments on this change. (
http://gerrit.cloudera.org:8080/17937 )
Change subject: IMPALA-10970: Fix criterion for classifying coordinator only
query
..
Patch Set 3:
(2 comments)
Thanks for your suggestions, i will submit update soon later.
http://gerrit.cloudera.org:8080/#/c/17937/3/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/17937/3/be/src/scheduling/scheduler.cc@824
PS3, Line 824: if (exec_request.plan_exec_info.size() > 1) {
: /// There are intermediate join build side plan that
co-located with join node,
: /// don't take the query as coordinator only.
: return false;
: }
> we can instead do something like,
Agree, provide detail description in struct definition is better. I will update
this later.
http://gerrit.cloudera.org:8080/#/c/17937/3/testdata/workloads/functional-query/queries/QueryTest/mt-dop.test
File testdata/workloads/functional-query/queries/QueryTest/mt-dop.test:
http://gerrit.cloudera.org:8080/#/c/17937/3/testdata/workloads/functional-query/queries/QueryTest/mt-dop.test@30
PS3, Line 30: # TIMPALA-128: Coordinator only query determination error.
: # From IMPALMA-4224 we execute join build in a separate
fragments, the root
: # plan contains two subplan, one of it contains join node while
the other
: # contains join builder as its root, the separate join build
fragment is
: # co-located with join node.
: # This test case will fail due to outdated coordinator only
determination
: # logic which only take only the first subplan for concern.
: # To ensure the inline left table placed at join probe side, we
add
: # STRAIGHT_JOIN hint to disable join inversion optimization.
> How about?
I think your description is more clear.
--
To view, visit http://gerrit.cloudera.org:8080/17937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700
Gerrit-Change-Number: 17937
Gerrit-PatchSet: 3
Gerrit-Owner: guojingfeng
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: guojingfeng
Gerrit-Comment-Date: Fri, 19 Nov 2021 02:59:55 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/17937 )
Change subject: IMPALA-10970: Fix criterion for classifying coordinator only
query
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17937/3/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:
http://gerrit.cloudera.org:8080/#/c/17937/3/be/src/scheduling/scheduler.h@77
PS3, Line 77: /// If there are only one TPlanExecInfo, single unpartitioned
fragment is considered
: /// coordinator only.
: /// If there are multiple TPlanExecInfo, we should check all of
them is co-located in
: /// coordinator node. If all other TPlanExecInfo has only one
fragments and it's
: /// partition type is unpartitioned, we should also take this
Query is a coordinator
: /// only. (e.g. JOIN BUILD sink (MT_DOP>0) is co-located with
its pre order ancestor
: /// : JOIN_NODE) Check IMPALA-4224 for more detail.
nit: I think the original comment is good enough, this is adding extra context
for an edge case which is an implementation detail and does not need to me
mentioned in the method comment
http://gerrit.cloudera.org:8080/#/c/17937/3/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/17937/3/be/src/scheduling/scheduler.cc@824
PS3, Line 824: if (exec_request.plan_exec_info.size() > 1) {
: /// There are intermediate join build side plan that
co-located with join node,
: /// don't take the query as coordinator only.
: return false;
: }
we can instead do something like,
num_parallel_plans = exec_request.plan_exec_info.size();
then at the end check for num_parallel_plans == 1, this way it makes the code
self explanatory and helps us avoid adding comments. In addition to this, to
further improve readability, you can add a better description to
'plan_exec_info' member in struct TQueryExecRequest in Query.thrift file as
follows:
'The first plan in the list materializes the query result and subsequent plans
materialize the build sides of joins. Each plan appears before its dependencies
in the list.'
http://gerrit.cloudera.org:8080/#/c/17937/3/testdata/workloads/functional-query/queries/QueryTest/mt-dop.test
File testdata/workloads/functional-query/queries/QueryTest/mt-dop.test:
http://gerrit.cloudera.org:8080/#/c/17937/3/testdata/workloads/functional-query/queries/QueryTest/mt-dop.test@30
PS3, Line 30: # TIMPALA-128: Coordinator only query determination error.
: # From IMPALMA-4224 we execute join build in a separate
fragments, the root
: # plan contains two subplan, one of it contains join node while
the other
: # contains join builder as its root, the separate join build
fragment is
: # co-located with join node.
: # This test case will fail due to outdated coordinator only
determination
: # logic which only take only the first subplan for concern.
: # To ensure the inline left table placed at join probe side, we
add
: # STRAIGHT_JOIN hint to disable join inversion optimization.
How about?
IMPALA-10970: Verify that a query plan containing 2 parallel plans with the
first one having a single unpartitioned fragment is correctly classified as not
being a coordinator only query. This query create a plan with the
aforementioned condition and has a join build fragment that will be co-located
with the root sink fragment(IMPALMA-4224). This ensures that the query fails to
schedule and eventually times out in the queue if incorrectly classified as a
coordinator only query.
To ensure the inline left table placed at join probe side, we add STRAIGHT_JOIN
hint to disable join inversion optimization.
--
To view, visit http://gerrit.cloudera.org:8080/17937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700
Gerrit-Change-Number: 17937
Gerrit-PatchSet: 3
Gerrit-Owner: guojingfeng
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: guojingfeng
Gerrit-Comment-Date: Fri, 19 Nov 2021 00:43:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9803/ : 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/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 3 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Thu, 18 Nov 2021 17:17:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9802/ : 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/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 2 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Thu, 18 Nov 2021 17:00:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
guojingfeng has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. IMPALA-10970: Fix criterion for classifying coordinator only query This patch fixes a bug in the criterion which decided whether a query can be considered as a coordinator only query. It did not consider the possibility of parallel plans and ended up mis-classifying some queries as coordinator only queries. This classification was used during scheduling when dedicated coordinators and executor groups are used and allowed coordinator queries to be scheduled only on the coordinator even in the absence of healthy executor groups. As a result of this bug, queries classified wrongly ended up with error code: NO_REGISTERED_BACKENDS. Testing: - Add new mt_dop test case for functional_query and pass - Ran and passed custom_cluster/test_coordinators, test_executor_groups Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 --- M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test 3 files changed, 135 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/17937/3 -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 3 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
guojingfeng has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. IMPALA-10970: Fix criterion for classifying coordinator only query This patch fixes a bug in the criterion which decided whether a query can be considered as a coordinator only query. It did not consider the possibility of parallel plans and ended up mis-classifying some queries as coordinator only queries. This classification was used during scheduling when dedicated coordinators and executor groups are used and allowed coordinator queries to be scheduled only on the coordinator even in the absence of healthy executor groups. As a result of this bug, queries classified wrongly ended up with error code: NO_REGISTERED_BACKENDS. Testing: - Add new mt_dop test case for functional_query and pass - Ran and passed custom_cluster/test_coordinators, test_executor_groups Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 --- M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M testdata/workloads/functional-query/queries/QueryTest/mt-dop.test 3 files changed, 136 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/17937/2 -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 2 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng
[Impala-ASF-CR] IMPALA-10970: Fix criterion for classifying coordinator only query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17937 ) Change subject: IMPALA-10970: Fix criterion for classifying coordinator only query .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/17937/2/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/17937/2/be/src/scheduling/scheduler.cc@825 PS2, Line 825: /// There are intermediate join build side plan that co-located with join node, line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/17937/2/be/src/scheduling/scheduler.cc@829 PS2, Line 829: line has trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/17937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaaf1f1ba7a976122b4d37bd675e6d8181dc8700 Gerrit-Change-Number: 17937 Gerrit-PatchSet: 2 Gerrit-Owner: guojingfeng Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: guojingfeng Gerrit-Comment-Date: Thu, 18 Nov 2021 16:40:23 + Gerrit-HasComments: Yes
