impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <>
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:

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

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
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.
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I044f83806fcde820fcb38047cf6b8e780d803858
Gerrit-PatchSet: 22
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Mostafa Mokhtar <>
Gerrit-Reviewer: anujphadke <>
Gerrit-HasComments: Yes

View raw message