Tim Armstrong 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 3: (5 comments) Looking pretty good.. http://gerrit.cloudera.org:8080/#/c/15926/1/be/src/exec/hdfs-scan-node-base.cc File be/src/exec/hdfs-scan-node-base.cc: http://gerrit.cloudera.org:8080/#/c/15926/1/be/src/exec/hdfs-scan-node-base.cc@420 PS1, Line 420: ddBytesReadCounters(); > I thought a lot about this and decided to keep this as it is. WFM 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, > not sure about the full context, but do you mean that a DCHECK(use_mt_scan_ Something like that, yeah. 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 update it to check # ranges instead of bytes in a couple of places 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 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 optimized to something without a division in the context of the loop, but you could avoid the division with something like this: idx = (idx + 1 == ranges.size()) ? 0 : idx + 1; -- 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: 3 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: Thu, 28 May 2020 20:28:42 +0000 Gerrit-HasComments: Yes
