[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. More precisely, if the cluster is running with a global admission controller, the max parallelism for the trivial query is three per resource pool, but if there is no global admission controller, each coordinator would admit the trivial queries based on its own local variable, therefore, the max parallelism would be three per coordinator per resource pool in this case. As the first patch, we try to keep the trivial query as simple as possible, and it could be extended in future. Added query option enable_trivial_query_for_admission to control whether the trivial query policy is enabled. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Reviewed-on: http://gerrit.cloudera.org:8080/19214 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 16 files changed, 444 insertions(+), 32 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 9 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 8 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Fri, 27 Jan 2023 01:14:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8990/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 8 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Thu, 26 Jan 2023 20:14:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 8 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Thu, 26 Jan 2023 20:14:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 7 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Thu, 26 Jan 2023 01:55:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has removed a vote on this change. Change subject: IMPALA-7969: Always admit trivial queries immediately .. Removed Verified+1 by Yida Wu -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 7 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 7 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 24 Jan 2023 14:06:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1/ : 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/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 7 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 24 Jan 2023 01:00:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/19214/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19214/5//COMMIT_MSG@33 PS5, Line 33: runni > using -> running Done http://gerrit.cloudera.org:8080/#/c/19214/5//COMMIT_MSG@42 PS5, Line 42: could be extended in fu > 'might be able to extend in future' => Done http://gerrit.cloudera.org:8080/#/c/19214/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19214/6//COMMIT_MSG@31 PS6, Line 31: If the maximum parallelism is reached, : the admission controller would try to admit the trivial query : via normal process. > Would be good to add a test case for this scenario, if we don't have one al test_trivial_query_multi_runs in the ee test should cover the case, and the testcase ensures successful runs when max parallelism is reached. Added test_trivial_query_multi_runs_fallback to create an error case that fallback and blocked by a long query then timeout. http://gerrit.cloudera.org:8080/#/c/19214/5/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/19214/5/be/src/scheduling/admission-controller.h@666 PS5, Line 666: in > on -> in Done http://gerrit.cloudera.org:8080/#/c/19214/5/be/src/scheduling/admission-controller.h@979 PS5, Line 979: the quer > 'schedule' => 'query' Done -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 7 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 24 Jan 2023 00:44:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. More precisely, if the cluster is running with a global admission controller, the max parallelism for the trivial query is three per resource pool, but if there is no global admission controller, each coordinator would admit the trivial queries based on its own local variable, therefore, the max parallelism would be three per coordinator per resource pool in this case. As the first patch, we try to keep the trivial query as simple as possible, and it could be extended in future. Added query option enable_trivial_query_for_admission to control whether the trivial query policy is enabled. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 16 files changed, 444 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/19214/7 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 7 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 6: Rebased to fix the conflicts. -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 6 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 23 Jan 2023 04:34:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12217/ : 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/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 6 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 23 Jan 2023 03:40:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. More precisely, if the cluster is using with a global admission controller, the max parallelism for the trivial query is three per resource pool, but if there is no global admission controller, each coordinator would admit the trivial queries based on its own local variable, therefore, the max parallelism would be three per coordinator per resource pool in this case. As the first patch, we try to keep the trivial query as simple as possible, and it might be able to extend in future. Added query option enable_trivial_query_for_admission to control whether the trivial query policy is enabled. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 16 files changed, 409 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/19214/6 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 6 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 5: Code-Review+1 Thanks for the changes! -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 10 Jan 2023 09:31:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 5: (3 comments) Added a testcase for the multi-threaded case. http://gerrit.cloudera.org:8080/#/c/19214/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19214/4//COMMIT_MSG@29 PS4, Line 29: Also, we restrict the parallelism of execution of the trivial : query, each resource pool can execute no more than three trivial : queries at the same time. If the maximum parallelism is reached, : the admission controller would try to admit the trivial query : via normal process. > What happens when you have multiple coordinators? Since the trivial queries Added comments. Discussed with Abhishek, if working with a global admission controller, no matter the coordinator's number, the max concurrent trivial query is limited to three per pool, but if there is no global admission controller, each coordinator would admit the query based on local variable, so it could be X*N trivial queries per resource pool. Maybe we can keep it this way in this patch to keep it simple, as long as the trivial queries can be restricted to a reasonable parallelism. http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java: http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@66 PS3, Line 66: HasFunctionSleep > Hmm, I meant that a new predicate could be created here that filters specif I added a function in Expr.java, should be simpler now. http://gerrit.cloudera.org:8080/#/c/19214/4/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java: http://gerrit.cloudera.org:8080/#/c/19214/4/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@71 PS4, Line 71: IS_FN_SLEEP, slee > This looks strange, as sleep is not an aggregate function The function was added long ago back to 2014 in https://github.com/apache/impala/commit/335c46a2060f4e76f9a689a1a285af7a024353cf, and it is no longer used in the code as far as I can see, maybe at that time we only support aggregate function. But the implementation is actually to find out all the build-in functions. -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 10 Jan 2023 00:51:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12133/ : 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/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 10 Jan 2023 00:54:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. More precisely, if the cluster is using with a global admission controller, the max parallelism for the trivial query is three per resource pool, but if there is no global admission controller, each coordinator would admit the trivial queries based on its own local variable, therefore, the max parallelism would be three per coordinator per resource pool in this case. As the first patch, we try to keep the trivial query as simple as possible, and it might be able to extend in future. Added query option enable_trivial_query_for_admission to control whether the trivial query policy is enabled. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 16 files changed, 410 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/19214/5 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java: http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@66 PS3, Line 66: HasFunctionSleep > This recursive function could be simplifed by using expr.collectAll() + a P Hmm, I meant that a new predicate could be created here that filters specifically for sleep. My guess is that it would make the code simpler, but it ok to keep it as it is. http://gerrit.cloudera.org:8080/#/c/19214/4/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java: http://gerrit.cloudera.org:8080/#/c/19214/4/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@71 PS4, Line 71: IS_BUILTIN_AGG_FN This looks strange, as sleep is not an aggregate function -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 4 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Sun, 08 Jan 2023 11:45:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/19214/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19214/4//COMMIT_MSG@29 PS4, Line 29: Also, we restrict the parallelism of execution of the trivial : query, each resource pool can execute no more than three trivial : queries at the same time. If the maximum parallelism is reached, : the admission controller would try to admit the trivial query : via normal process. What happens when you have multiple coordinators? Since the trivial queries are a subset of coordinator only queries should we also include #coordinators in the calculation. So basically something like X trivial queries per coordinator per resource pool and total X*N trivial queries per resource pool? We will also have to enforce it so that each coordinator can only execute X trivial queries per resource pool. -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 4 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Sat, 07 Jan 2023 01:43:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12120/ : 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/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 4 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Fri, 06 Jan 2023 18:57:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. As the first patch, we try to keep the trivial query as simple as possible, and it might be able to extend in future. Added query option enable_trivial_query_for_admission to control whether the trivial query policy is enabled. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 15 files changed, 352 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/19214/4 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 4 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java: http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@66 PS3, Line 66: HasFunctionSleep This recursive function could be simplifed by using expr.collectAll() + a Predicate. There are several predicates like this, e.g. https://github.com/apache/impala/blob/5b349cb5492827e7584412de3922d263ac441d26/fe/src/main/java/org/apache/impala/analysis/Expr.java#L104 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Thu, 05 Jan 2023 10:49:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/19214 )
Change subject: IMPALA-7969: Always admit trivial queries immediately
..
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@340
PS2, Line 340: ss << " local_host(local_mem_admitted=" <<
PrintBytes(local_mem_admitted_) << ", ";
> local_trivial_running_ could be added to the debug string
Done
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@784
PS2, Line 784: pools_for_updates_.insert(running_query.reque
> Is this log line intentional?
Thanks pointing it out. Removed.
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@1914
PS2, Line 1914:<< " max_mem=" << PrintBytes(max_mem)
> Maybe add state->GetIsTrivialQuery() to the log?
Done
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@1921
PS2, Line 1921: tor grou
> typo
Done
http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java
File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java:
http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@22
PS2, Line 22:
> nit: usually there is a blank line betwwen java. and org. imports
Done
http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@82
PS2, Line 82:* Check whether the query meets the special requirements of a
trivial query.
> nit: we usually add braces to the block when breaking a line
Done
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_admission_controller.py@1216
PS2, Line 1216: "select * from functional_parquet.alltypes limit
1", False)
> Can you check what happens when the sleep is after an empty set? e.g.
Added the testcases. Thanks, I think I understand the issue now, changed a way
to detect the sleep function in TrivialQueryChecker.java, not sure if there is
any other case, but it is working with the testcases now.
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_session_expiration.py
File tests/custom_cluster/test_session_expiration.py:
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_session_expiration.py@107
PS2, Line 107: # trivial query.
> I think the test would be clearer with keeping select 1, but setting enable
Done
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_shell_interactive.py
File tests/custom_cluster/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_shell_interactive.py@51
PS2, Line 51: roc.sendline("set live_summary=t
> I think the test would be clearer with keeping select 1, but setting enable
Done
--
To view, visit http://gerrit.cloudera.org:8080/19214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91
Gerrit-Change-Number: 19214
Gerrit-PatchSet: 3
Gerrit-Owner: Yida Wu
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Wed, 04 Jan 2023 17:47:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12097/ : 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/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Wed, 04 Jan 2023 17:42:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. As the first patch, we try to keep the trivial query as simple as possible, and it might be able to extend in future. Added query option enable_trivial_query_for_admission to control whether the trivial query policy is enabled. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 15 files changed, 350 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/19214/3 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/19214 )
Change subject: IMPALA-7969: Always admit trivial queries immediately
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@340
PS2, Line 340: ss << " local_host(local_mem_admitted=" <<
PrintBytes(local_mem_admitted_) << ", ";
local_trivial_running_ could be added to the debug string
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@784
PS2, Line 784: LOG(INFO) << "Release query id " << query_id;
Is this log line intentional?
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@1914
PS2, Line 1914:<< " max_mem=" << PrintBytes(max_mem);
Maybe add state->GetIsTrivialQuery() to the log?
http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@1921
PS2, Line 1921: Admiited
typo
http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java
File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java:
http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@22
PS2, Line 22: import org.apache.impala.analysis.Expr;
nit: usually there is a blank line betwwen java. and org. imports
http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@82
PS2, Line 82: if (!queryOptions.isEnable_trivial_query_for_admission() ||
!isQueryStmt)
nit: we usually add braces to the block when breaking a line
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_admission_controller.py@1216
PS2, Line 1216: "select 1 union all select sleep(1)", False)
Can you check what happens when the sleep is after an empty set? e.g.
select 1 from functional.alltypes limit 0 union all select sleep(1)
another query I am curious about is select a from (select 1 a, sleep(1)) s;
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_session_expiration.py
File tests/custom_cluster/test_session_expiration.py:
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_session_expiration.py@107
PS2, Line 107: queued_handle = client.execute_async("select sleep(1)")
I think the test would be clearer with keeping select 1, but setting
enable_trivial_query_for_admission to false.
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_shell_interactive.py
File tests/custom_cluster/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_shell_interactive.py@51
PS2, Line 51: roc.sendline("select sleep(1);")
I think the test would be clearer with keeping select 1, but setting
enable_trivial_query_for_admission to false. Currently it is not evident why
sleep is used.
--
To view, visit http://gerrit.cloudera.org:8080/19214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91
Gerrit-Change-Number: 19214
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Mon, 02 Jan 2023 12:45:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/19214 )
Change subject: IMPALA-7969: Always admit trivial queries immediately
..
Patch Set 1:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@10
PS1, Line 10: immediate admission
> Do you know if these trivial queries consume some global resource we can ru
Added a limit of 3 for the concurrent running trivial queries per resource
pool. I think the target in this patch is to allow immediate admission without
causing too many troubles, so adding restrictions should be okay.
http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@14
PS1, Line 14: UNION
> This could be extended with the union being all const.
Added a restriction that only zero or one row can be returned.
http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@18
PS1, Line 18: - select * from table limit 0;
> is this also a valid example?
Added a restriction that the max number of rows is 1. So "select 1 union all
select 2" won't be trivial in this patch.
http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@21
PS1, Line 21: Need some thinking on the definition of tirvial query
: - Any other cases can be trivial queries?
: - Any case that we don't want but allows for immediate
admission in
: this patch?
> I think probably okay to create a separate query option for admitting coord
I implement the trivial query as a subset of coord-only query in this patch,
because would like to keep it simple in the first version, might extend later.
http://gerrit.cloudera.org:8080/#/c/19214/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/19214/1/be/src/scheduling/admission-controller.cc@1889
PS1, Line 1889: if (state->GetIsTrivialQuery()) {
: VLOG_QUERY << "Admiited by trivial query policy.";
: queue_node->admitted_schedule =
std::move(group_state.state);
: return true;
: }
> This will always add trivial queries to the first executor group. Can this
I think in the first version we can treat the trivial query as a subset of
coord-only query to keep it simple. Added a DCHECK to ensure it is a coord-only
query.
http://gerrit.cloudera.org:8080/#/c/19214/1/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java
File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java:
http://gerrit.cloudera.org:8080/#/c/19214/1/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@44
PS1, Line 44: String displayLabel = planRoot.getDisplayLabel();
: if (displayLabel == null) return false;
: if (!displayLabel.equals("00:EMPTYSET") &&
!displayLabel.equals("00:UNION")) {
: return false;
: }
> Can the root node have child nodes? My guess is no, as empty set has no chi
Added a restriction that trivial query only allow to return 0 or 1 row.
http://gerrit.cloudera.org:8080/#/c/19214/1/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@60
PS1, Line 60: sinkRoot.collectExprs(exprs);
> Will this detect a sleep in the source nodes e.g. select 1 union all select
Make a restriction that no more than 1 row can be returned, so "select 1 union
all select sleep(1000)" won't be accepted as a trivial query in this patch.
Also added it as a negative case in the ee test.
http://gerrit.cloudera.org:8080/#/c/19214/1/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/19214/1/tests/custom_cluster/test_admission_controller.py@1202
PS1, Line 1202: "Admission result:
Admitted immediately
> It would be nice to able to check somehow whether a query was handled as tr
Added a string AdmissionController::PROFILE_INFO_VAL_ADMIT_TRIVIAL: "Admitted
as a trivial query".
http://gerrit.cloudera.org:8080/#/c/19214/1/tests/custom_cluster/test_admission_controller.py@1208
PS1, Line 1208: select 1,3
> Can you add a test for a more complex constant query, e.g select 1 union al
Added a testcase "select 1 union all select 2" as a negative case. Because
restrict trivial query to have no more than 1 row.
--
To view, visit http://gerrit.cloudera.org:8080/19214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91
Gerrit-Change-Number: 19214
Gerrit-PatchSet: 1
Gerrit-Owner: Yida Wu
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Thu, 22 Dec 2022
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12061/ : 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/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 22 Dec 2022 18:02:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7969: Always admit trivial queries immediately
Yida Wu has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/19214 ) Change subject: IMPALA-7969: Always admit trivial queries immediately .. IMPALA-7969: Always admit trivial queries immediately The idea of trivial query is to allow certain queries to bypass the admission control, and therefore accelerating the query execution even when the server resource is at capacity. It could benefit the queries that require a fast response while consuming the minimum resources. This patch adds support for the trivial query detection and allows an immediate admission for the trivial query. We define the trivial query as a subset of the coordinator-only query, and returns no more than one row. The definition is as below: - Must have PLAN ROOT SINK as the root - Can contain UNION and EMPTYSET nodes only - Results can not be over one row Examples of a trivial query: - select 1; - select * from table limit 0; - select * from table limit 0 union all select 1; - select 1, (2 + 3); Also, we restrict the parallelism of execution of the trivial query, each resource pool can execute no more than three trivial queries at the same time. If the maximum parallelism is reached, the admission controller would try to admit the trivial query via normal process. As the first patch, we try to keep the trivial query as simple as possible, and it might be able to extend in future. Tests: Passed exhaustive tests. Added test_trivial_query and test_trivial_query_low_mem. Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/schedule-state.cc M be/src/scheduling/schedule-state.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/planner/Planner.java A fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java M tests/common/resource_pool_config.py M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_shell_interactive.py 15 files changed, 302 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/19214/2 -- To view, visit http://gerrit.cloudera.org:8080/19214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91 Gerrit-Change-Number: 19214 Gerrit-PatchSet: 2 Gerrit-Owner: Yida Wu Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
