Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19033 )

Change subject: IMPALA-11604 Planner changes for CPU usage
......................................................................


Patch Set 46:

(12 comments)

Thank you Qifan for continuing shepherding this patch.
I add some more edits at commit message in patch set 46 and put some examples.
Hope it is clearer now.

http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@144
PS44, Line 144: subtree of PlanNodes/DataSink in the
> Each blocking segment is a subtree of the PlanNodes in the fragment with a
Done


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@146
PS44, Line 146: PlanNodes or DataSink that belong to the same segment will have 
their
> nit the
This is rephrased in ps46.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@154
PS44, Line 154:
> nit missing 's'
Done


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@158
PS44, Line 158:
> May need to further explain, say with the concept of a blocking fragment.
The whole section III  is rephrased in ps46.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@161
PS44, Line 161:
> nit decides
This is rephrased in ps46.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@177
PS44, Line 177: fragment's O
> nit "At the blocking fragment" is better here as it 'blocking fragment' is
Rephrased in ps46.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@177
PS44, Line 177: ngCost is be
> same. use of blocking segment is better.
Rephrased.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@178
PS44, Line 178: of the parent f
> It is better to define CoreRequirement first before use. It is not clear wh
Added definition and examples in ps46.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@188
PS44, Line 188: rallelism) of that fragment
> Old name.
Done


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@195
PS44, Line 195: This
> query_cpu_requirement_scale or its new form should be listed here as well.
Done


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@200
PS44, Line 200: lle
> Do we need a min_threads control?  If not, please add a comment here to exp
The adjustment can go lower is Consumer fragment is deemed faster than Producer 
fragment in terms of ProcessingCost. Added clarification in ps46.


http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@212
PS44, Line 212:
              : |
> for completeness, better to list them here as like the three FE ones.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If32dc770dfffcdd0be2b5555a789a7720952c68a
Gerrit-Change-Number: 19033
Gerrit-PatchSet: 46
Gerrit-Owner: Qifan Chen <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Qifan Chen <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 10 Feb 2023 21:41:44 +0000
Gerrit-HasComments: Yes

Reply via email to