Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/21277 )
Change subject: IMPALA-12988: Calculate an unbounded version of CpuAsk ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/21277/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21277/1//COMMIT_MSG@16 PS1, Line 16: MAX_FRAGMENT_INSTANCES_PER_NODE options accordingly. If these bounds don't make sense to apply with multiple executor groups, then we should reconsider their naming or whether these bounds should be applied at all by the planner or instead moved to the admission control. http://gerrit.cloudera.org:8080/#/c/21277/1/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/21277/1/common/thrift/Query.thrift@1035 PS1, Line 1035: no specifi need to clarify what this means and why we have this concept. Is one core sufficient? http://gerrit.cloudera.org:8080/#/c/21277/1/fe/src/main/java/org/apache/impala/planner/CostingSegment.java File fe/src/main/java/org/apache/impala/planner/CostingSegment.java: http://gerrit.cloudera.org:8080/#/c/21277/1/fe/src/main/java/org/apache/impala/planner/CostingSegment.java@80 PS1, Line 80: private PlanFragment getFragment() { Can you make this getTopNode() then call this by both createCoreCount() and create UnboundedCoreCount()? http://gerrit.cloudera.org:8080/#/c/21277/1/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/21277/1/fe/src/main/java/org/apache/impala/planner/Planner.java@590 PS1, Line 590: int coresRequiredUnbounded = boundedCores.total(); Move this assignment to the else branch for clarity. I'm not really clear why this is using boundedCores and not an unbounded Number.. http://gerrit.cloudera.org:8080/#/c/21277/1/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/21277/1/fe/src/main/java/org/apache/impala/service/Frontend.java@313 PS1, Line 313: protected int cores_required_unbounded_ = -1; Can this default to 1 instead? -- To view, visit http://gerrit.cloudera.org:8080/21277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5441e31088f90761062af35862be4ce09d116923 Gerrit-Change-Number: 21277 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: David Rorke <dro...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Tue, 09 Apr 2024 22:20:00 +0000 Gerrit-HasComments: Yes
