Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11571 )

Change subject: KUDU-2563: [spark] Use the scanner keepAlive API
......................................................................


Patch Set 2:

(3 comments)

It would be good to double check that KuduScanner.keepAlive() is well behaved 
when the scan is "in between" two tablets. Ideally that would do nothing, but 
at the very least it shouldn't try to send a keep alive RPC into the ether and 
then throw an exception when that fails.

http://gerrit.cloudera.org:8080/#/c/11571/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala:

http://gerrit.cloudera.org:8080/#/c/11571/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala@19
PS2, Line 19: import java.util.concurrent.Executors
Can you double check that you still need all of these new imports?


http://gerrit.cloudera.org:8080/#/c/11571/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala@146
PS2, Line 146:       LOG.error(s"Calling keepAlive on ${scanner.currentTablet}")
For debugging only?


http://gerrit.cloudera.org:8080/#/c/11571/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala@147
PS2, Line 147:       scanner.keepAlive()
Is it OK if this fails and throws a KuduException?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7f26d6ab8deb24982055d247938a11e188c35db
Gerrit-Change-Number: 11571
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Comment-Date: Thu, 04 Oct 2018 22:41:57 +0000
Gerrit-HasComments: Yes

Reply via email to