impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4833: Compute precise per-host reservation size
Date Tue, 15 Aug 2017 17:26:50 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4833: Compute precise per-host reservation size
......................................................................


Patch Set 8:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7630/8/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:

PS8, Line 56: fragments
instances


Line 57:   // instance_params.
I think this comment lost some information compared to the equivalent comments in the old
code.  From the comment, it's hard to know whether this is the minimum over all instances
of the reservation, or the total of all the minimums. It would help to explain what this is
conceptually.


PS8, Line 62: initial_reservation_total_bytes
I found that having the word "claim" in the old equivalent members made it easier to keep
track of what this was, since "total" can be so vague.


http://gerrit.cloudera.org:8080/#/c/7630/8/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

PS8, Line 399: in the case all fragments are scheduled on all hosts with
             :   // max DOP
given that this patch is addressing the overestimation, why do we compute this overestimate?


http://gerrit.cloudera.org:8080/#/c/7630/8/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS8, Line 483: fragments in fragment_ctxs
is it over fragments or instances?  if fragments and not instances, how does that work out?


PS8, Line 490: initial_reservation_total_bytes
same comment


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idbcd1e9b1be14edc4017b4907e83f9d56059fbac
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message