Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/15926 )
Change subject: IMPALA-9655: Dynamic intra-node load balancing for HDFS scans ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/15926/1/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/15926/1/be/src/exec/hdfs-scan-node.cc@73 PS1, Line 73: HdfsScanNode::HdfsScanNode(ObjectPool* pool, const HdfsScanPlanNode& pnode, > Something like that, yeah. added this check to ProcessScanRangesAndInitSharedState http://gerrit.cloudera.org:8080/#/c/15926/1/be/src/scheduling/scheduler-test.cc File be/src/scheduling/scheduler-test.cc: http://gerrit.cloudera.org:8080/#/c/15926/1/be/src/scheduling/scheduler-test.cc@725 PS1, Line 725: > I think a lot of this test is still relevant, I think we just need to updat I agree the tests definitely make sense, the only reason I removed them was that I felt that now the code was rudimentary enough that adding a unit test seemed like an overkill as opposed to its predecessor (LPT algo) that had a fair bit of complexity to it. Let me know if you feel strongly about it and I'll add it in. http://gerrit.cloudera.org:8080/#/c/15926/3/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/15926/3/be/src/scheduling/scheduler.cc@459 PS3, Line 459: (ranges) > redundant parens Done http://gerrit.cloudera.org:8080/#/c/15926/3/be/src/scheduling/scheduler.cc@459 PS3, Line 459: per_instance_ranges[i % num_instances].push_back((ranges)[i]); > % here could be a bottleneck if there were a lot of ranges. Maybe it is opt Done -- To view, visit http://gerrit.cloudera.org:8080/15926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a101d0d98dff6e3779f85bc466e4c0bdb38094b Gerrit-Change-Number: 15926 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 29 May 2020 00:05:24 +0000 Gerrit-HasComments: Yes
