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, let's reevaluate the scheduling strategy based on more extensive test could you add a todo to that effect to the .h? Line 440: } else if (memory_distance == min_distance) { > what does that have to do w/ the backend config? ? Line 509: // (the root fragment doesn't have a destination) is that not true? PS24, Line 1089: total_assignments_-> > Yes, I used rand() here to get deterministic randomness. It will return ran why does moving the seed generator into the main scheduler class cause a problem? that should give you a deterministic sequence of seeds in the single-threaded/test case. Line 1090: if (!remote_read) total_local_assignments_->Increment(1); > Do you think this will be prohibitively slow and we should perform a benchm let's do it as a follow-on (but let's do that right after this gets in). http://gerrit.cloudera.org:8080/#/c/2200/27/be/src/scheduling/simple-scheduler.cc File be/src/scheduling/simple-scheduler.cc: Line 916: BackendList* be_descs = &backend_map_[be_desc.ip_address]; i'm not a fan of using [] for reading, because that has the potential also to do an implicit update. find() circumvents that. Line 1100: // Explicitly set the optional fields. why? http://gerrit.cloudera.org:8080/#/c/2200/25/be/src/scheduling/simple-scheduler.h File be/src/scheduling/simple-scheduler.h: Line 327: is this the local backend? local_backend_descriptor_? -- 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 <l...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: anujphadke <apha...@cloudera.com> Gerrit-HasComments: Yes
