Matthew Jacobs 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? I'll take a look, though I'm not too worried about it because it repros constantly when I run our regular tpch tests which we will be added shortly (Dimitris is working on that as we speak). PS1, Line 450: return > rephrase "return this thread" Done Line 480: if (!optional_threads_exceeded) --num_active_scanners_; > do we even need this bool? seems like the only case when the condition is f I don't think it's so easy. The issue is that if we don't check the state and update num_active_scanners_ within the while() loop above, there may be 2 remaining threads that both think they are optional threads, and thus will goto done. But if they do, then there are no more threads left. I don't love the bool, but I thought about how to do this cleanly and I don't see a better way. Maybe I'm missing your point though. -- 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-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
