Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3902: Scheduler improvements for running multiple 
fragment instances on a single backend
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4054/3/fe/src/main/java/com/cloudera/impala/service/Frontend.java
File fe/src/main/java/com/cloudera/impala/service/Frontend.java:

Line 964:       planner.computeResourceReqs(fragments, true, queryExecRequest);
> This function assumes that the entire query is captured by the given fragme
anything llama-related is going to be removed for the next release. i'll leave 
this alone for now.


Line 1042:     for (int idx = 0; idx < fragments.size(); ++idx) {
> factor our the common code between createExecRequest() and createPlanExecIn
all of the non-mt paths (meaning the functions for which this patch creates a 
"Mt-" equivalent) are going to be retired in the not-too-distant future. i 
didn't feel that factoring out common code between the two alternative paths 
has value, because a) this code has been fairly static in the past and doesn't 
see many changes, b) some of this may get ripped out later anyway.

in other words, you should consider the "Mt-" functions the end state, and look 
at them from the perspective of 'is that what the code should look like?' 
rather than 'if we have both the Mt- and the other path, what should the code 
look like?'.

i'll augment the commit msg.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I240445199e22f009f4e72fdb8754eb8d77e3d680
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-HasComments: Yes

Reply via email to