Michael Ho has posted comments on this change. Change subject: IMPALA-2831: Bound the number of scanner threads per scan node. ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4174/1/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: PS1, Line 738: The scanner thread will yield after completing a scan range so the main thread : // also gets to run. > I don't understand what this is trying to say, given that you also have the What I was trying to convey was that the number of scanner threads being the same as the number of logical cores may starve the main thread. While it's true that OS' preemption will ensure some chances for the main thread to run, this comment is to point out that the scanner thread also active yields the cpu so other threads get to run too. Let me rephrase it (or remove it). Line 740: runtime_state_->resource_pool()->set_max_quota(CpuInfo::num_cores() + 1); > hmm doesn't this change the number of threads available for the join side h Yes, good point. So, the way we implemented 'NUM_SCANNER_THREADS' query option is broken all along. Let's not touch the max_quota at all for this query option and check it in another way. Please wait for the next update of this patch. -- 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: 1 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
