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 <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: anujphadke <[email protected]> Gerrit-HasComments: Yes
