David Ribeiro Alves has posted comments on this change. Change subject: IMPALA-3857: KuduScanNode race on returning "optional" threads ......................................................................
Patch Set 1: (2 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; > I'll take a look, though I'm not too worried about it because it repros con yeah only if it's easy (not sure how involved is to emulate runtime_state_->resource_pool()->optional_exceeded()) but would be good to have some coverage that doesn't require running tpch under load. Line 480: if (!optional_threads_exceeded) --num_active_scanners_; > I don't think it's so easy. The issue is that if we don't check the state a oh, I see the race you are talking about. I'm regretting not having changed the goto's from the original implementation and making this more vanilla. Can and move mentioning the race to where you decrement num_active_scanners_ in the block above and leave a TODO to refactor this logic? this is totally something that future me would forget -- 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
