You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2023/04/04 01:45:33 UTC

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19688


Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web UI always shows default resource pool even if a different resource
pool is used by the query. The Planner could set request_pool in
TQueryCtx, but the backend does not check the request_pool in TQueryCtx
returned from the frontend when getting resource pools for Web UI.
This patch fixs the issue in ClientRequestState::request_pool(). It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default to
   verify resource pools in Web queries site and Web admission site.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
4 files changed, 27 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_executor_groups.py@931
PS4, Line 931:     # Check resource pools on the Web queries site and admission site
             :     self._verify_query_num_for_resource_pool("root.small", 2)
             :     self._verify_query_num_for_resource_pool("root.tiny", 1)
             :     self._verify_query_num_for_resource_pool("root.large", 2)
             :     self._verify_total_admitted_queries("root.small", 2)
             :     self._verify_total_admitted_queries("root.tiny", 1)
             :     self._verify_total_admitted_queries("root.large", 2)
> These tests check that number of queries admitted to the pool. Have to chec
That is right, thanks for the clarification.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 00:43:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 3:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/12744/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 17:50:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19688/6/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/6/tests/custom_cluster/test_admission_controller.py@307
PS6, Line 307: 
> flake8: E502 the backslash is redundant between brackets
Done


http://gerrit.cloudera.org:8080/#/c/19688/6/tests/custom_cluster/test_admission_controller.py@312
PS6, Line 312: 
> flake8: E502 the backslash is redundant between brackets
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 00:35:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/3/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/3/tests/custom_cluster/test_admission_controller.py@307
PS3, Line 307: '
> flake8: W605 invalid escape sequence '\S'
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 17:59:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 04:40:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12749/ : 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/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 00:50:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web queries site shows no resource pool unless it is specified with
query option. The Planner could set TQueryCtx.request_pool in
TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixes the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest. It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Updated expected error message in
   TestAdmissionController::test_set_request_pool.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
6 files changed, 59 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/7
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/5/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/5/tests/custom_cluster/test_admission_controller.py@307
PS5, Line 307: \
flake8: W605 invalid escape sequence '\S'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:53:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12746/ : 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/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 19:13:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12750/ : 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/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 00:56:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12740/ : 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/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 02:07:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py@307
PS4, Line 307: \
flake8: W605 invalid escape sequence '\S'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:00:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py@307
PS4, Line 307: S
> same as previous code, not an issue. passed the test.
The warning suggests making this a r"..." (raw literal) string. https://www.flake8rules.com/rules/W605.html



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:53:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/3/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/3/tests/custom_cluster/test_admission_controller.py@307
PS3, Line 307: \
flake8: W605 invalid escape sequence '\S'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 17:38:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web UI always shows default resource pool even if a different resource
pool is used by the query. The Planner could set TQueryCtx.request_pool
in TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixs the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest. It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Updated expected error message in
   TestAdmissionController::test_set_request_pool.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
5 files changed, 45 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/4
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web UI always shows default resource pool even if a different resource
pool is used by the query. The Planner could set TQueryCtx.request_pool
in TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixs the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest. It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Updated expected error message in
   TestAdmissionController::test_set_request_pool.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
5 files changed, 44 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/3
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 09:51:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py@307
PS4, Line 307: \
> flake8: W605 invalid escape sequence '\S'
same as previous code, not an issue. passed the test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:20:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web UI always shows default resource pool even if a different resource
pool is used by the query. The Planner could set TQueryCtx.request_pool
in TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixs the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/service/client-request-state.h
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
3 files changed, 42 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12743/ : 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/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 17:07:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19688/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19688/5//COMMIT_MSG@9
PS5, Line 9: Web UI always shows default resource pool even if a different resource
Are you referring to the Admission WebUI? With unit tests, /queries Web UI shows no resource pool unless it was specified via client option.


http://gerrit.cloudera.org:8080/#/c/19688/5//COMMIT_MSG@14
PS5, Line 14: fixs
fixes


http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h@255
PS5, Line 255:     if (exec_request_->query_exec_request.query_ctx.__isset.request_pool) {
I don't think that this is actually thread-safe since exec_request_ may not be filled in at this point. Whether an invalid pointer could be read might depend on how thrift assignments with optional strings are implemented.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 19:15:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h@255
PS5, Line 255:     if (exec_request_->query_exec_request.query_ctx.__isset.request_pool) {
> Looking at thrift code, the is_set flag is updated after assignments for bo
I will add atomic_bool is_planning_done_. It will be set as true after planning is done. We check exec_request_->query_exec_request.query_ctx only is_planning_done_ is done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 21:08:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19688/6/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/6/tests/custom_cluster/test_admission_controller.py@307
PS6, Line 307: \
flake8: E502 the backslash is redundant between brackets


http://gerrit.cloudera.org:8080/#/c/19688/6/tests/custom_cluster/test_admission_controller.py@312
PS6, Line 312: \
flake8: E502 the backslash is redundant between brackets



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 00:30:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web queries site shows no resource pool unless it is specified with
query option. The Planner could set TQueryCtx.request_pool in
TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixes the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest. It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Updated expected error message in
   TestAdmissionController::test_set_request_pool.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
6 files changed, 59 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/6
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19688/2/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/19688/2/tests/custom_cluster/test_executor_groups.py@925
PS2, Line 925:  
flake8: E203 whitespace before ','


http://gerrit.cloudera.org:8080/#/c/19688/2/tests/custom_cluster/test_executor_groups.py@925
PS2, Line 925: ,
flake8: E231 missing whitespace after ','



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 16:48:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/scheduling/request-pool-service.cc
File be/src/scheduling/request-pool-service.cc:

http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/scheduling/request-pool-service.cc@a181
PS4, Line 181: 
> Does this constant still being used somewhere? Can it be removed?
Done


http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/service/client-request-state.h@256
PS4, Line 256:   // If the request pool has been updated by Planner, return the updated request
             :       // pool.
             :       return exec_request_->query_exec_request.query_ctx.request_pool;
> nit: Can we change this to if-else and comments for better readability?
Done


http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_executor_groups.py@931
PS4, Line 931:     # Check resource pools on the Web queries site and admission site
             :     self._verify_query_num_for_resource_pool("root.small", 2)
             :     self._verify_query_num_for_resource_pool("root.tiny", 1)
             :     self._verify_query_num_for_resource_pool("root.large", 2)
             :     self._verify_total_admitted_queries("root.small", 2)
             :     self._verify_total_admitted_queries("root.tiny", 1)
             :     self._verify_total_admitted_queries("root.large", 2)
> nit: move this right after _setup_three_exec_group_cluster in line 891. If 
These tests check that number of queries admitted to the pool. Have to check after queries are executed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:52:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h@255
PS5, Line 255:     if (exec_request_->query_exec_request.query_ctx.__isset.request_pool) {
> I don't think that this is actually thread-safe since exec_request_ may not
Looking at thrift code, the is_set flag is updated after assignments for both __set_request_pool and TQueryCtx::operator= but there are no explicit memory barriers. Since this should be updated from the planner thrift, it should be using TQueryCtx::operator= and most likely is safe enough since there are many other members assigned before the is_set flags.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 19:34:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 15:34:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/scheduling/request-pool-service.cc
File be/src/scheduling/request-pool-service.cc:

http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/scheduling/request-pool-service.cc@a181
PS4, Line 181: 
Does this constant still being used somewhere? Can it be removed?

Also, does removing this condition change any behavior? Any test that is changing?

What happen now if result.resolved_pool.empty() is true? Does the method documentation in request-pool-service.h need update as well?


http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/19688/4/be/src/service/client-request-state.h@256
PS4, Line 256: return exec_request_->query_exec_request.query_ctx.__isset.request_pool ?
             :         exec_request_->query_exec_request.query_ctx.request_pool :
             :             (query_ctx_.__isset.request_pool ? query_ctx_.request_pool : "");
nit: Can we change this to if-else and comments for better readability?


http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_executor_groups.py@931
PS4, Line 931:     # Check resource pools on the Web queries site and admission site
             :     self._verify_query_num_for_resource_pool("root.small", 2)
             :     self._verify_query_num_for_resource_pool("root.tiny", 1)
             :     self._verify_query_num_for_resource_pool("root.large", 2)
             :     self._verify_total_admitted_queries("root.small", 2)
             :     self._verify_total_admitted_queries("root.tiny", 1)
             :     self._verify_total_admitted_queries("root.large", 2)
nit: move this right after _setup_three_exec_group_cluster in line 891. If verify fail, then no testcase should be run.
Similarly for the other 2 below.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:31:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web UI always shows default resource pool even if a different resource
pool is used by the query. The Planner could set TQueryCtx.request_pool
in TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixs the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest. It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Updated expected error message in
   TestAdmissionController::test_set_request_pool.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
5 files changed, 48 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/19688/5
-- 
To view, visit http://gerrit.cloudera.org:8080/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/19688/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19688/5//COMMIT_MSG@9
PS5, Line 9: Web queries site shows no resource pool unless it is specified with
> Are you referring to the Admission WebUI? With unit tests, /queries Web UI 
Done


http://gerrit.cloudera.org:8080/#/c/19688/5//COMMIT_MSG@14
PS5, Line 14: fixe
> fixes
Done


http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/19688/5/be/src/service/client-request-state.h@255
PS5, Line 255:         && exec_request_->query_exec_request.query_ctx.__isset.request_pool) {
> I will add atomic_bool is_planning_done_. It will be set as true after plan
Done


http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/4/tests/custom_cluster/test_admission_controller.py@307
PS4, Line 307:  
> The warning suggests making this a r"..." (raw literal) string. https://www
Thanks, will fix it.


http://gerrit.cloudera.org:8080/#/c/19688/5/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/19688/5/tests/custom_cluster/test_admission_controller.py@307
PS5, Line 307: t
> flake8: W605 invalid escape sequence '\S'
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 00:29:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12745/ : 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/19688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 18:20:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................


Patch Set 7: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Apr 2023 12:51:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12036: Fix Web UI to show right resource pools

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19688 )

Change subject: IMPALA-12036: Fix Web UI to show right resource pools
......................................................................

IMPALA-12036: Fix Web UI to show right resource pools

Web queries site shows no resource pool unless it is specified with
query option. The Planner could set TQueryCtx.request_pool in
TQueryExecRequest when auto scaling is enabled. But the backend
ignores the TQueryCtx.request_pool in TQueryExecRequest when getting
resource pools for Web UI.
This patch fixes the issue in ClientRequestState::request_pool() by
checking TQueryCtx.request_pool in TQueryExecRequest. It also
removes the error path in RequestPoolService::ResolveRequestPool() if
requested_pool is empty string.

Testing:
 - Updated TestExecutorGroups::test_query_cpu_count_divisor_default,
   TestExecutorGroups::test_query_cpu_count_divisor_two, and
   TestExecutorGroups::test_query_cpu_count_divisor_fraction to
   verify resource pools on Web queries site and Web admission site.
 - Updated expected error message in
   TestAdmissionController::test_set_request_pool.
 - Passed core test.

Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Reviewed-on: http://gerrit.cloudera.org:8080/19688
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Kurt Deschler <kd...@cloudera.com>
Reviewed-by: Abhishek Rawat <ar...@cloudera.com>
---
M be/src/scheduling/request-pool-service.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/custom_cluster/test_web_pages.py
6 files changed, 59 insertions(+), 12 deletions(-)

Approvals:
  Riza Suminto: Looks good to me, but someone else must approve
  Impala Public Jenkins: Verified
  Kurt Deschler: Looks good to me, but someone else must approve
  Abhishek Rawat: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iceacb3a8ec3bd15a8029ba05d064bbbb81e3a766
Gerrit-Change-Number: 19688
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>