Alex Behm has posted comments on this change.

Change subject: IMPALA-4654: KuduScanner must return when ReachedLimit()
......................................................................


Patch Set 2: Code-Review+1

Change looks good to me.

As discussed, I think there is still one remaining bug with plans like 
scan+topn where the coordinator may not necessarily cancel fragments containing 
scans. In that case Close() is called on the plan tree during tear down, but 
unlike other scan nodes the Kudu scan node does not use done_ to terminate the 
scanner threads. For non-Kudu scan nodes setting done_ triggers teardown of all 
scanner threads. The scanner threads detect this indirectly via 
ScannerContext::cancelled() which is checked on a per-row-batch basis.

Todd has a good point. Our limit check is broken. Agree to deal with that 
everywhere in s separate change.

-- 
To view, visit http://gerrit.cloudera.org:8080/5493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaddd51111a1b2647995d68e6d37d0500b3a322de
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: No

Reply via email to