David Ribeiro Alves has posted comments on this change.

Change subject: IMPALA-3857: KuduScanNode race on returning "optional" threads
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3637/1/be/src/exec/kudu-scan-node.cc
File be/src/exec/kudu-scan-node.cc:

Line 480:   if (!optional_threads_exceeded) --num_active_scanners_;
> What refactoring did you have in mind? Maybe I can do it now.
state checks and changes should be atomic, so basically we'd move out the done: 
block to its own method, to something like:
bool CheckIfDoneAndFinish()
in this method we acquire the lock, check the state (and change if necessary). 
If the method returns true we then return from ScannerThread.

It's probably more code but it would avoid this race (and the extra bool)


-- 
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

Reply via email to