Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12341 )

Change subject: KUDU-2672: Spark write to kudu, too many machines write to one 
tserver.
......................................................................


Patch Set 1:

(3 comments)

Thank you for the contribution! I added some high level requests and feedback 
below.

http://gerrit.cloudera.org:8080/#/c/12341/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

http://gerrit.cloudera.org:8080/#/c/12341/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@76
PS1, Line 76:   val retryTimes = sc.getConf.getInt("spark.kudu.flush.retry", 3)
Do you mind breaking the flush changes into a separate patch? The two features 
are related to writing but I think we can have clearer reviews and feedback if 
they are separate.


http://gerrit.cloudera.org:8080/#/c/12341/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@333
PS1, Line 333:   private[kudu] def writeRows(
Instead of the "spark.kudu.batch.repartition" config, can we add an option to 
writeOptions?

The config can then be added to DefaultSource and mapped to be set in the 
options similar to "kudu.ignoreNull"


http://gerrit.cloudera.org:8080/#/c/12341/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@346
PS1, Line 346:       val partitioner = new PartitionerByTablet(tabletNum)
Could we implement the Spark partition based on the KuduPartitioner in this 
patch?:
https://gerrit.cloudera.org/#/c/12275/

This makes it so we don't need to expose and use non-public client APIs that 
are meant to be internal to the client.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6e0df2d43e93dc60d0b5c68daf91eb75b8d3bc7
Gerrit-Change-Number: 12341
Gerrit-PatchSet: 1
Gerrit-Owner: yangz <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 06 Feb 2019 20:06:54 +0000
Gerrit-HasComments: Yes

Reply via email to