Dan Hecht has posted comments on this change. Change subject: IMPALA-2831: Bound the number of scanner threads per scan node. ......................................................................
Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4174/2/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: Line 741: } if we do it this way, let's factor the set_max_quota() out so it's clear that that we're now always doing that: if (query option set) { num_scanner_threads = query-option } else { num_scanner_threads = CpuInfo::num_cores(); } set_max_quota(num_scanner_threads + 1); but this makes me a little nervous still due to the side-effect on join thread. This isn't all that different than changing FLAGS_num_threads_per_core to be 1, right? The alternative is to move this logic into ThreadTokenAvailableCb to not take a token if we've already spun up enough threads. Not saying we should change this approach necessarily, but wondering why you chose this approach over those two alternatives. -- To view, visit http://gerrit.cloudera.org:8080/4174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I191988ad18d6b4caf892fc967258823edcf9681f Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
