impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2979: Fix scheduling on remote hosts
Date Thu, 12 May 2016 05:40:32 GMT
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-2979: Fix scheduling on remote hosts
......................................................................


Patch Set 22:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/2200/22/be/src/scheduling/simple-scheduler.h
File be/src/scheduling/simple-scheduler.h:

Line 123:   typedef boost::heap::binomial_heap<BackendAssignmentInfo,
does this really require anything from boost (and not std)? we're trying to get rid of boost
dependencies.


Line 210:   /// data from the given data_loation.
what is data_location?

does this do load balancing with node_assignments?


Line 216:   /// node_assignments:     Used to track per-query and per-host assignment information.
which backends does this track? is this an output param? the global state?


Line 254:   Status ComputeScanRangeAssignmentForQuery(const TQueryExecRequest& exec_request,
isn't the 'for query' implied?


Line 262:   /// First we assemble a list of candidate backends, from which the scan range
can be
no ,


Line 263:   /// read. Backends are classified by their memory distance to the data, and the
closest
briefly explain memory distance, it's not a standard term


Line 266:   /// if they were further away. Only backends running on one of the replicas of
the scan
i don't think you should describe the steps of the algorithm here (you could read the code
for that, plus it might change anyway). instead, describe the outcome/semantic properties
of the computed assignment (and be specific there; for instance, list the query option that
influence replica preference).


Line 273:   /// list, resulting in a local read. First a search is performed to find the candidate
yep, this is way too detailed.


Line 307:   Status ComputeScanRangeAssignmentForPlanNode(PlanNodeId node_id,
same here, the parameters differentiate it from 'for query'.


Line 350:   /// Lookup the IP address of 'hostname' and return whether the lookup was successful.
If
lookup is a noun, to look up is the verb.


Line 358:   Status HostnameToIpAddr(const Hostname& hostname, IpAddr* ip);
on the face of it this looks like LookUpBackendIp. how is it different?


Line 376:       bool global_backend_order, AssignmentInfo* context);
is this the global assignment info?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I044f83806fcde820fcb38047cf6b8e780d803858
Gerrit-PatchSet: 22
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: anujphadke <aphadke@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message