David Ribeiro Alves has posted comments on this change. Change subject: IMPALA-3857: KuduScanNode race on returning "optional" threads ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3637/1/be/src/exec/kudu-scan-node.cc File be/src/exec/kudu-scan-node.cc: Line 421: bool optional_threads_exceeded = false; anyway to add a simple directed test to kudu-scan-node-test.cc for this? PS1, Line 450: return rephrase "return this thread" Line 480: if (!optional_threads_exceeded) --num_active_scanners_; do we even need this bool? seems like the only case when the condition is false is when we've decremented above, in which case isn't it the same thing to do it here? I mean, couldn't you just have, above: { unique_lock<mutex> l(lock_); if (num_active_scanners_ > 1 && runtime_state_->resource_pool()->optional_exceeded()) { goto done; } and then not have this change at all? -- To view, visit http://gerrit.cloudera.org:8080/3637 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I22adf2109b43b1b37d9a597de85e063431dff155 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Matthew Jacobs <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-HasComments: Yes
