You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org> on 2023/06/16 04:43:10 UTC

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20084


Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
4 files changed, 108 insertions(+), 27 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/1
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 11: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Sun, 25 Jun 2023 07:39:06 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M src/kudu/common/schema.cc
5 files changed, 112 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/7
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20084/2/java/kudu-backup-common/src/main/protobuf/backup.proto
File java/kudu-backup-common/src/main/protobuf/backup.proto:

http://gerrit.cloudera.org:8080/#/c/20084/2/java/kudu-backup-common/src/main/protobuf/backup.proto@52
PS2, Line 52:   bool is_auto_incrementing = 11;
BTW, what happens when a backup of a table with auto-incrementing column is used to restore the table when using backup tool of prior revision?  Is everything works as expected in terms of:
  * data integrity (i.e. absence of corruption)
  * return code (or visible result when running the restore app) compared with what actually happened under the hood



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 16 Jun 2023 18:44:03 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20084/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:

http://gerrit.cloudera.org:8080/#/c/20084/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@135
PS1, Line 135: "auto_incrementing_id"
> nit: Schema.getAutoIncrementingColumnName()
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 16 Jun 2023 18:05:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Without this patch any attempt to
Backup a table with auto-incrementing column will fail with an error
message "auto_incrementing_id is a reserved column name" from
schema.cc.
Restoring a table with auto-incrementing column will result in the
same behavior as with the backup.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Reviewed-on: http://gerrit.cloudera.org:8080/20084
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Kudu Jenkins
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M src/kudu/common/schema.cc
5 files changed, 113 insertions(+), 28 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 13
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

PS6: 
> The intention of having it here is to have it close to where it was discove
All right, it seems that's a long separate story.  Then I'm fine to keep this as-is in this patch -- addressing that long-standing piece of tech debt is out of this patch's scope, of course.


http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@314
PS6, Line 314: ColumnSchema.getStrippedColumnSchema() would trigger the exception
             :     // otherwise
... ColumnSchema.getStrippedColumnSchema() in the Java client would throw an exception otherwise



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Jun 2023 22:53:03 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M src/kudu/common/schema.cc
5 files changed, 113 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/4
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20084/2/java/kudu-backup-common/src/main/protobuf/backup.proto
File java/kudu-backup-common/src/main/protobuf/backup.proto:

http://gerrit.cloudera.org:8080/#/c/20084/2/java/kudu-backup-common/src/main/protobuf/backup.proto@52
PS2, Line 52:   bool is_auto_incrementing = 11;
> BTW, what happens when a backup of a table with auto-incrementing column is
Without this change the backup of table cannot be taken as it as en exception is thrown from ColumnSchema.getStrippedColumnSchema() when generating the column schema.

Even if a backup is taken using this change, and a restore is attempted from an older backup-restore jar, an error is thrown during in ColumnSchema.ColumnSchemaBuilder() as we have a column with the reserved name - "auto_incrementing_column". So there is no data corruption but the return message doesn't explicitly mention to use a newer version of the jar. Since it is all a part of the Kudu java client, probably we can document the error message?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Jun 2023 00:57:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Without this patch any attempt to
Backup a table with auto-incrementing column will fail with an error
message "auto_incrementing_id is a reserved column name" from
schema.cc.
Restoring a table with auto-incrementing column will result in the
same behavior as with the backup.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M src/kudu/common/schema.cc
5 files changed, 113 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/12
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 01:07:21 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 12: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 04:28:54 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
4 files changed, 108 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/2
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M src/kudu/common/schema.cc
5 files changed, 113 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/3
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 11:

The only question is what behavior to expect when trying to use old version of the tools (that don't understand the auto-incrementing column)  (a) the backup tool against a table that have an auto-incrementing column (b) the restore tool that tries to restore data from the data that have been created for a table with auto-incrementing column by a tool that understands auto-incrementing columns.


It would be great if you could clarify on that.

Could be done separately, but if including that into the commit message, that would be even better.  But this patch is good as is, overall.

Thanks!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 01:11:36 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@313
PS6, Line 313: for number
> for the number
Done


http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@313
PS6, Line 313: the check
> to check
Done


http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@317
PS6, Line 317: !=
> nit: add spaces around '!=' operation
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Jun 2023 22:30:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................

KUDU-1945 Backup/restore for tables with auto incrementing columns

This patch adds the ability to backup and restore tables containing
auto-incrementing columns. A simple test case is also added to test
the functionaility.

Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
---
M java/kudu-backup-common/src/main/protobuf/backup.proto
M java/kudu-backup-common/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-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala
M src/kudu/common/schema.cc
5 files changed, 113 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/20084/11
-- 
To view, visit http://gerrit.cloudera.org:8080/20084
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@314
PS6, Line 314: ColumnSchema.getStrippedColumnSchema() would trigger the exception
             :     // otherwise
> ... ColumnSchema.getStrippedColumnSchema() in the Java client would throw a
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Sat, 24 Jun 2023 00:10:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20084/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:

http://gerrit.cloudera.org:8080/#/c/20084/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@135
PS1, Line 135: "auto_incrementing_id"
nit: Schema.getAutoIncrementingColumnName()



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 16 Jun 2023 16:49:07 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 11:

Updated the commit message with the behavior of backup and restore without this restore.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 03:44:41 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 12:

> Updated the commit message with the behavior of backup and restore
 > without this restore.

Thank you very much for the update!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Jun 2023 04:29:24 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

PS6: 
> Shouldn't this be a part of other patch in this stack of patches, with corr
The intention of having it here is to have it close to where it was discovered that it was needed for future references. I can add it to the client side patches with a. test coverage, sure.

The stripping was introduced to overcome a limitation in Java client:
https://gerrit.cloudera.org/#/c/3015/
There was a note worthy discussion here:
https://gerrit.cloudera.org/#/c/12689/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@323

I guess the technical debt (TODO) was never addressed and should be addressed to iron out the implementation.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Jun 2023 22:27:35 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1945 Backup/restore for tables with auto incrementing columns

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

Change subject: KUDU-1945 Backup/restore for tables with auto incrementing columns
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

PS6: 
Shouldn't this be a part of other patch in this stack of patches, with corresponding test coverage?  Doesn't C++ client choke in similar case?  If not, why similar check isn't present in the C++ client and does it make sense to add similar check in there as well?


http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@313
PS6, Line 313: the check
to check


http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@313
PS6, Line 313: for number
for the number


http://gerrit.cloudera.org:8080/#/c/20084/6/src/kudu/common/schema.cc@317
PS6, Line 317: !=
nit: add spaces around '!=' operation



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I929d54d22c1c938ee67fdda9f4c2bb68c028b4ec
Gerrit-Change-Number: 20084
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Fri, 23 Jun 2023 06:53:58 +0000
Gerrit-HasComments: Yes