Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2979: Fix scheduling on remote hosts ......................................................................
Patch Set 28: Code-Review+2 (3 comments) let's get this in, and then think about additional test coverage. http://gerrit.cloudera.org:8080/#/c/2200/28/be/src/scheduling/simple-scheduler-test.cc File be/src/scheduling/simple-scheduler-test.cc: Line 765: false, plan_.scan_range_locations(), plan_.referenced_datanodes(), exec_at_coord, are there no tests for the random_replica flag? http://gerrit.cloudera.org:8080/#/c/2200/27/be/src/scheduling/simple-scheduler.cc File be/src/scheduling/simple-scheduler.cc: Line 1100: } > The comment didn't make sense, I removed it. well, you are explicitly setting the optional fields. you could also leave that out and stick with the defaults from the .thrift file. http://gerrit.cloudera.org:8080/#/c/2200/28/be/src/scheduling/simple-scheduler.h File be/src/scheduling/simple-scheduler.h: Line 61: /// TODO: Benchmark the performance of std::shuffle to create the random backend order. what we need is a microbenchmark for the performance of the scheduler itself, not so much std::shuffle. (that way, we can detect perf regressions easily.) -- 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: 28 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
