You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Abhishek Rawat (Code Review)" <ge...@cloudera.org> on 2023/08/18 17:53:39 UTC

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

Abhishek Rawat has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20378


Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 89 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 21:33:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 21:33:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 90 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 8: Code-Review+2

Updated test case to have mem limit for admission on executors/coordinators to be higher than the value set by `mem_limit_executors` and `mem_limit_coordinators` query options. The default behavior is to cap mem_limit for a query at the max allowed mem limit for admission.

Third time's a charm!

Carry forward +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 21:32:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20378/2/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

http://gerrit.cloudera.org:8080/#/c/20378/2/common/thrift/ImpalaService.thrift@839
PS2, Line 839: unspecified or a limit of 0
nit: Unspecified or a limit of 0 or negative value



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Aug 2023 23:09:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 23:40:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 13:39:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

Posted by "Abhishek Rawat (Code Review)" <ge...@cloudera.org>.
Abhishek Rawat has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/20378 )

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 93 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20378/3/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/20378/3/be/src/service/query-options.h@308
PS3, Line 308:   
> nit: right align this backslash?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 20:54:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20378/1/be/src/scheduling/schedule-state.cc
File be/src/scheduling/schedule-state.cc:

http://gerrit.cloudera.org:8080/#/c/20378/1/be/src/scheduling/schedule-state.cc@319
PS1, Line 319:   const bool is_mem_limit_coordinators_set = query_options().__isset.mem_limit_coordinators
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Aug 2023 17:54:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 90 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 7: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 18:08:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20378/2/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

http://gerrit.cloudera.org:8080/#/c/20378/2/common/thrift/ImpalaService.thrift@839
PS2, Line 839: unspecified or a limit of 0
> nit: Unspecified or a limit of 0 or negative value
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 15:37:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 16:01:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Aug 2023 01:54:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 90 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 13:39:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20378/3/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/20378/3/be/src/service/query-options.h@308
PS3, Line 308:  \
nit: right align this backslash?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 16:21:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 6: Code-Review+2

Setting proper mem_limit_coordinators as the original value was too small for the testcase.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 04:25:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20378 )

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Reviewed-on: http://gerrit.cloudera.org:8080/20378
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 93 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/382/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Aug 2023 01:59:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 1:

(1 comment)

Make lint happy.

http://gerrit.cloudera.org:8080/#/c/20378/1/be/src/scheduling/schedule-state.cc
File be/src/scheduling/schedule-state.cc:

http://gerrit.cloudera.org:8080/#/c/20378/1/be/src/scheduling/schedule-state.cc@319
PS1, Line 319:   const bool is_mem_limit_coordinators_set = query_options().__isset.mem_limit_coordinators
> line too long (91 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Aug 2023 20:14:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................

IMPALA-10860: Allow setting mem_limit for coordinators

Added support for MEM_LIMIT_COORDINATORS query option. This is
similar to exisiting MEM_LIMIT_EXECUTORS, but applies to coordinators.
There are cases where Planner generates inaccurate estimates for
coordinator fragments and would be good to be able to set mem limit
just for the coordinator, since a query's memory requirement on
coordinator tends to be much lower compared to that on executors.

If MEM_LIMIT is set, then MEM_LIMIT_COORDINATORS is ignored.

Also updated the documentation for the new query option.

Testing:
- Added new custom cluster tests which validates MEM_LIMIT_COORDINATORS
applies only on coordinator. The test also validates that both
MEM_LIMIT_EXECUTORS and MEM_LIMIT_COORDINATORS can be set together.
- Built docs and made sure that the new changes have proper formatting.

Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
---
M be/src/scheduling/schedule-state.cc
M be/src/service/query-options-test.cc
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 docs/topics/impala_mem_limit.xml
M tests/custom_cluster/test_admission_controller.py
8 files changed, 90 insertions(+), 4 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 4: Code-Review+2

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 21:00:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Aug 2023 23:40:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10860: Allow setting mem limit for coordinators

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

Change subject: IMPALA-10860: Allow setting mem_limit for coordinators
......................................................................


Patch Set 5: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfc9a735e82dce2fd903bdaf6bc2e46e982ef8c
Gerrit-Change-Number: 20378
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 03:50:34 +0000
Gerrit-HasComments: No