Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13191 )

Change subject: [backup] Support partition alterations between Kudu backups
......................................................................


Patch Set 1:

(3 comments)

Nice, looks good

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala:

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@119
PS1, Line 119: catch {
             :               // Passthrough on NonCoveredRangeException. These 
are the expected way of
             :               // detecting a partition which was dropped between 
backups and filtering
             :               // out the rows from that dropped partition.
             :               case ncr: NonCoveredRangeException => Unit
             :             }
> Throwing and catching an exception isn't cheap, so this seems particularly
+1


http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java:

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java@56
PS1, Line 56: TODO
nit: TODO(ghenke)


http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java@180
PS1, Line 180:       // Use a LinkedHashMap to maintain partition order.
Why do we care about order? That doesn't appear to be part of the contract for 
the KuduPartitioner when we pass this map into it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Comment-Date: Wed, 01 May 2019 21:35:00 +0000
Gerrit-HasComments: Yes

Reply via email to