impala-reviews mailing list archives

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

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


Patch Set 8:

(6 comments)

Good questions/comments. I think I addressed the comments, though I'm still not terribly happy
with all the names. I'll post a separate review since this already went in.

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
Done


Line 57:   // instance_params.
> I think this comment lost some information compared to the equivalent comme
Done


PS8, Line 62: initial_reservation_total_bytes
> I found that having the word "claim" in the old equivalent members made it 
Done


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 t
This is still used for the explain output. I don't think there's a way to get the 'tighter'
numbers there without going through scheduling. That said, it's pretty unfortunate that this
field exists given it gets computed in Planner.computeResourceReqs() and gets used in Planner.getExplainString().


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 doe
Done


PS8, Line 490: initial_reservation_total_bytes
> same comment
Done


-- 
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