impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4654: KuduScanner must return when ReachedLimit()
Date Wed, 14 Dec 2016 05:42:08 GMT
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 <mj@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <todd@apache.org>
Gerrit-HasComments: No

Mime
View raw message