Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2979: Fix scheduling on remote hosts ......................................................................
Patch Set 27: (8 comments) http://gerrit.cloudera.org:8080/#/c/2200/24/be/src/scheduling/simple-scheduler.cc File be/src/scheduling/simple-scheduler.cc: Line 439: backend_candidates.push_back(backend_ip); > Yes, this follows the original approach of the scheduler to only track assi yes, let's reevaluate the scheduling strategy based on more extensive tests. let's try to get those done before the code freeze, though, so we have time to tweak. Line 440: } else if (memory_distance == min_distance) { > See previous comment. what does that have to do w/ the backend config? http://gerrit.cloudera.org:8080/#/c/2200/25/be/src/scheduling/simple-scheduler.h File be/src/scheduling/simple-scheduler.h: Line 174: /// AssignmentHeap it can be used to look up heap elements by their IP address and > I had added this to address this review comment: "where is it used, who cre i guess it's good to leave here. http://gerrit.cloudera.org:8080/#/c/2200/27/be/src/scheduling/simple-scheduler.h File be/src/scheduling/simple-scheduler.h: Line 140: /// Internal structure to track scan range assignments for a backend. This struct is ah, so this is really for a backend host, not just backend. Line 141: /// used as the heap element in AddressableAssignmentHeap. "heap element in and maintained by "? (or is this updated by anyone else?) (to address your question about how to comment on the fields. it's good to give the reader context on who updates something.) Line 148: int random_rank; you can also make this const int and then provide a c'tor that sets the rank. that way it's clear that this doesn't change once the struct has been created. Line 247: /// member in AssignmentCtx. streamline comment Line 428: /// Finally scan ranges are considered which do not have an impalad backend running on sorry, i think it's actually "finally, ..." -- 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: 27 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: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: anujphadke <[email protected]> Gerrit-HasComments: Yes
