Casey Ching has posted comments on this change.

Change subject: IMPALA-3292. Kudu scanner should not fail if KeepAlive request 
fails
......................................................................


Patch Set 2:

(1 comment)

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

Line 433:         scanner.KeepKuduScannerAlive();
> i think we want the KeepKuduScannerAlive() after the AddBatchWithTimeout() 
I couldn't figure out why the keep alive would be needed outside of the 
AddBatch loop, do you know why that was done?

The keep alive make sense inside the loop since the loop could execute for many 
seconds/minutes. If something else blocks for a long time, I'd prefer to put 
the keep alive closer to that code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I10ad2cfde3e50a4486d03f8dd763faac2eb4926a
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-HasComments: Yes

Reply via email to