Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-5147: Add the ability to exclude hosts from query 
execution
......................................................................


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/6628/4/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

Line 202:     // If this is not an executor, don't add it to the new backend 
config.
> this comment just repeats what the code is doing, but doesn't explain why. 
Done


http://gerrit.cloudera.org:8080/#/c/6628/4/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

PS4, Line 269: backend_config_
> as mentioned earlier, this would be easier to follow if this were renamed t
Done


PS4, Line 283:   /// Map from unique backend id to TBackendDescriptor. Used to 
track the known executors
             :   /// from the statestore. It's important to track both the 
backend ID as well as the
             :   /// TBackendDescriptor so we know what is being removed in a 
given update.
> this comment was very unhelpful at understanding the code because it doesn'
Done


http://gerrit.cloudera.org:8080/#/c/6628/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

Line 388: void ImpalaServer::BackendsUrlCallback(
> I think this should go in impala-http-handler.cc
Done


http://gerrit.cloudera.org:8080/#/c/6628/4/be/src/service/impala-server.h
File be/src/service/impala-server.h:

PS4, Line 284: GetServerRole
> Why deal with the presentation (i.e. string formatting) here, rather than l
Done


http://gerrit.cloudera.org:8080/#/c/6628/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

PS4, Line 41: only_coordinate
> when i read this name I thought that it would start a cluster of only coord
Done


PS4, Line 42: \
> don't think you need these inside ( )
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d2ff7f341c9d2b0649e4d14561077e166ad7c4d
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to