You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/04/30 15:35:10 UTC

[kudu-CR] [backup] Support partition alterations between Kudu backups

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13191


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

[backup] Support partition alterations between Kudu backups

This patch enables partitions to be added and dropped
between backups.

It does this by adding tablet and partition details to the
metadata which can be used to detect if a partition
was dropped. It filters out partitions that are no longer
valid and adds the remaining partitions to a
KuduPartitioner. That KuduPartitioner can then be used
to filter out non-covered rows.

This does require some prevously package/private classes
and constructors to be marked as public. In those cases
I have tagged them as Private and Unstable using the
interface annotations.

Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
---
M java/kudu-backup/src/main/protobuf/backup.proto
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/NonCoveredRangeException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Partition.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionSchema.java
8 files changed, 231 insertions(+), 58 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/13191/1
-- 
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: newchange
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.

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


Removed Verified-1 by Kudu Jenkins (120)
-- 
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: deleteVote
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 01 May 2019 21:35:00 +0000
Gerrit-HasComments: Yes

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 2: Verified+1


-- 
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: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 12:44:22 +0000
Gerrit-HasComments: No

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.

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


Removed Verified-1 by Kudu Jenkins (120)
-- 
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: deleteVote
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/13191/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13191/1//COMMIT_MSG@19
PS1, Line 19: prevously
previously


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 expensive given it's on each row.

If we're already poking holes in KuduPartitioner, perhaps we can create a cheaper way to check if a row is covered or not?


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

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@330
PS1, Line 330:   def getPartitionSchema(metadata: TableMetadataPB): PartitionSchema = {
Method name is kinda confusing given KuduTable.getPartitionSchema.


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@55
PS1, Line 55:                   Map<String, Partition> tabletIdToPartition) {
Nit: indentation


http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java@61
PS1, Line 61:     partitionByStartKey.put(EMPTY, NON_COVERED_RANGE_INDEX);
Why did you move this into the constructor? I think it'd be easier to follow if all of the work was done in one place rather than split between build() and the constructor.

Oh, now I get it: it's because the restore code creates a custom KuduPartitioner with its own metadata. That's somewhat icky, but I can't think of a good alternative.


http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java@194
PS1, Line 194:         String tabletId = new String(tablet.getTabletId(), UTF_8);
Could we avoid the conversion and retain the tablet ID as a byte array? Seems like that might be doable if we treat it as such in the protobuf.


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

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionSchema.java@44
PS1, Line 44: @InterfaceAudience.LimitedPrivate("Impala")
            : @InterfaceStability.Unstable
Do these annotations automatically apply to nested classes too?



-- 
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 <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 01 May 2019 04:26:41 +0000
Gerrit-HasComments: Yes

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
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 2: Code-Review+2


-- 
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: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 05:43:01 +0000
Gerrit-HasComments: No

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13191 )

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

[backup] Support partition alterations between Kudu backups

This patch enables partitions to be added and dropped
between backups.

It does this by adding tablet and partition details to the
metadata which can be used to detect if a partition
was dropped. It filters out partitions that are no longer
valid and adds the remaining partitions to a
KuduPartitioner. That KuduPartitioner can then be used
to filter out non-covered rows.

This does require some previously package/private
classes and constructors to be marked as public.
In those cases I have tagged them as Private and Unstable
using the interface annotations.

Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Reviewed-on: http://gerrit.cloudera.org:8080/13191
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Grant Henke <gr...@apache.org>
---
M java/kudu-backup/src/main/protobuf/backup.proto
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/NonCoveredRangeException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Partition.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionSchema.java
8 files changed, 256 insertions(+), 63 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved; Verified

-- 
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: merged
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/13191/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13191/1//COMMIT_MSG@19
PS1, Line 19: prevously
> previously
Done


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
             :             }
> +1
I went back on forth with this. I will make a change.


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

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@330
PS1, Line 330:   def getPartitionSchema(metadata: TableMetadataPB): PartitionSchema = {
> Method name is kinda confusing given KuduTable.getPartitionSchema.
It returns the same thing. One from the metadata and one from the table.


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@55
PS1, Line 55:                   Map<String, Partition> tabletIdToPartition) {
> Nit: indentation
Done


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)
Done


http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java@61
PS1, Line 61:     partitionByStartKey.put(EMPTY, NON_COVERED_RANGE_INDEX);
> Why did you move this into the constructor? I think it'd be easier to follo
yeah, it wasn't great but I decided I was okay with this since there we no remote calls in the constructor.


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 
It's not strictly required, but maintains old and convenient behavior. I will add a detailed comment.


http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java@194
PS1, Line 194:         String tabletId = new String(tablet.getTabletId(), UTF_8);
> Could we avoid the conversion and retain the tablet ID as a byte array? See
It gets messy because I want to use it in the Map but I can't use a byte array as a key in a map. I could convert it to a BytesKey, but I did't really want to expose that. 

The API is already inconsistent and often exposes tableId as a string, so I didn't feel bad about doing it here too.


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

http://gerrit.cloudera.org:8080/#/c/13191/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionSchema.java@44
PS1, Line 44: @InterfaceAudience.LimitedPrivate("Impala")
            : @InterfaceStability.Unstable
> Do these annotations automatically apply to nested classes too?
Yes



-- 
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 <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 01 May 2019 23:07:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 3: Verified+1


-- 
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: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 13:35:05 +0000
Gerrit-HasComments: No

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.

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


Removed Verified-1 by Kudu Jenkins (120)
-- 
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: deleteVote
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 1: Verified+1


-- 
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 <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 01 May 2019 00:40:35 +0000
Gerrit-HasComments: No

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 3: Code-Review+2

Carrying +2 through rebase.


-- 
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: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 12:48:23 +0000
Gerrit-HasComments: No

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13191 )

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


Patch Set 2: Code-Review+1


-- 
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: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 00:49:32 +0000
Gerrit-HasComments: No

[kudu-CR] [backup] Support partition alterations between Kudu backups

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13191

to look at the new patch set (#2).

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

[backup] Support partition alterations between Kudu backups

This patch enables partitions to be added and dropped
between backups.

It does this by adding tablet and partition details to the
metadata which can be used to detect if a partition
was dropped. It filters out partitions that are no longer
valid and adds the remaining partitions to a
KuduPartitioner. That KuduPartitioner can then be used
to filter out non-covered rows.

This does require some previously package/private
classes and constructors to be marked as public.
In those cases I have tagged them as Private and Unstable
using the interface annotations.

Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
---
M java/kudu-backup/src/main/protobuf/backup.proto
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/TableMetadata.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/NonCoveredRangeException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Partition.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionSchema.java
8 files changed, 256 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/13191/2
-- 
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: newpatchset
Gerrit-Change-Id: I31e0eb27f163c38840e5466ff85d0b4a44d4ec0a
Gerrit-Change-Number: 13191
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>