Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12323 )
Change subject: KUDU-2670: Part 1: Build ScanToken by splitSizeBytes ...................................................................... Patch Set 5: (9 comments) Hi, grant. I update this patch. I broke the previous code into 2 patches. PART 1：build scanToken by splitSizeBytes PART 2: MultiKuduScanner http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java File java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java: http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java@46 PS2, Line 46: class KeyEncoder { > I don't think this needs to be public. Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KeyRange.java File java/kudu-client/src/main/java/org/apache/kudu/client/KeyRange.java: http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KeyRange.java@20 PS2, Line 20: import org.apache.yetus.audience.InterfaceAudience; > Could you add java doc? Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java: http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java@286 PS2, Line 286: private long splitSizeBytes = -1; > Could this name indicate that it will cause a tablet to be split? It would Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java@420 PS2, Line 420: UnsafeByteOperations.unsafeWrap(tablet.getPartition().getPartitionKeyStart())); > Since this is private API we can use methods to separate this logic instead Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java@460 PS2, Line 460: > I think the overall timeout needs to be considered here. Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java: http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java@a247 PS2, Line 247: > Why was this removed? Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/SplitKeyRangeRequest.java File java/kudu-client/src/main/java/org/apache/kudu/client/SplitKeyRangeRequest.java: http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/SplitKeyRangeRequest.java@30 PS2, Line 30: import org.apache.kudu.util.Pair; > Can you add javadoc? Done http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/SplitKeyRangeResponse.java File java/kudu-client/src/main/java/org/apache/kudu/client/SplitKeyRangeResponse.java: http://gerrit.cloudera.org:8080/#/c/12323/2/java/kudu-client/src/main/java/org/apache/kudu/client/SplitKeyRangeResponse.java@26 PS2, Line 26: */ > Think this is a copy paste from KuduRpcResponse. Can you update the javadoc Done http://gerrit.cloudera.org:8080/#/c/12323/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/12323/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala@51 PS2, Line 51: private val keepAlivePeriodMs = options.keepAlivePeriodMs > This configuration can be added to KuduReadOptions and set in DefaultSource Done -- To view, visit http://gerrit.cloudera.org:8080/12323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0502f5d64569e8b1d45e88de3cb36aa2e01234d0 Gerrit-Change-Number: 12323 Gerrit-PatchSet: 5 Gerrit-Owner: yangz <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu <[email protected]> Gerrit-Reviewer: yangz <[email protected]> Gerrit-Comment-Date: Fri, 01 Mar 2019 10:35:44 +0000 Gerrit-HasComments: Yes
