Dan Hecht has posted comments on this change. Change subject: IMPALA-2831: Bound the number of scanner threads per scan node. ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4174/1//COMMIT_MSG Commit Message: Line 20: with 8 logical cores reduces from 287MB to 101MB. could you add a little more explanation about why the mem usage goes down (disk-io buffers), so that we have that recorded somewhere for future reference? 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 + 1 for the main thread. Line 740: runtime_state_->resource_pool()->set_max_quota(CpuInfo::num_cores() + 1); > Yes, I agree that we probably should have the + 1 for the query option too. hmm doesn't this change the number of threads available for the join side hack? i thought we were trying to avoid impacting that. On the other hand, this is the same logic as NUM_SCANNER_THREADS. Line 906: if (!ranges.empty()) ThreadTokenAvailableCb(runtime_state_->resource_pool()); seems correct, but it's hard to reason about unexpected side-effects. is this needed by the capping portion of this change for some reason? -- 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
