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/25 17:48:46 UTC
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13120
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
KUDU-2790: Enhance the backup and restore tests
This patch enhances the backup and restore tests and
restructures the utility methods to be more flexible as
more tests are added.
Along with improvments to existing test the following
test methods were added:
- testForceFullBackup
- testRestoreWithNoBackup
- testBackupMissingTable
- testBackupAndRestoreWithNoRows
- testSimpleIncrementalBackupAndRestore
Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/BackupGraph.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/SessionIO.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestBackupGraph.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
5 files changed, 268 insertions(+), 152 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/13120/1
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
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/13120
to look at the new patch set (#2).
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
KUDU-2790: Enhance the backup and restore tests
This patch enhances the backup and restore tests and
restructures the utility methods to be more flexible as
more tests are added.
Along with improvments to existing test the following
test methods were added:
- testForceFullBackup
- testRestoreWithNoBackup
- testBackupMissingTable
- testBackupAndRestoreWithNoRows
- testSimpleIncrementalBackupAndRestore
Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/BackupGraph.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/SessionIO.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestBackupGraph.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
5 files changed, 271 insertions(+), 151 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/13120/2
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
Gerrit-PatchSet: 2
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] KUDU-2790: Enhance the backup and restore tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@48
PS2, Line 48: // Verify there is a table to backup.
: if (!context.syncClient.tableExists(tableName)) {
: throw new RuntimeException(s"Table does not exist: $tableName")
: }
> I looked into this further and I agree that openTable() doesn't do a great
I opted to remove the tableExists() call and use the less attractive but still decent error message.
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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, 25 Apr 2019 21:08:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
KUDU-2790: Enhance the backup and restore tests
This patch enhances the backup and restore tests and
restructures the utility methods to be more flexible as
more tests are added.
Along with improvements to existing test the following
test methods were added:
- testForceFullBackup
- testRestoreWithNoBackup
- testBackupMissingTable
- testBackupAndRestoreWithNoRows
- testSimpleIncrementalBackupAndRestore
Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Reviewed-on: http://gerrit.cloudera.org:8080/13120
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Grant Henke <gr...@apache.org>
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/BackupGraph.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/SessionIO.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestBackupGraph.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
4 files changed, 265 insertions(+), 151 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Grant Henke: Verified
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
Gerrit-PatchSet: 5
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] KUDU-2790: Enhance the backup and restore tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13120/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13120/2//COMMIT_MSG@13
PS2, Line 13: Along with improvments to existing test the following
> improvements
Done
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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, 25 Apr 2019 20:11:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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>
Gerrit-Comment-Date: Thu, 25 Apr 2019 21:58:42 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@48
PS2, Line 48: // Verify there is a table to backup.
: if (!context.syncClient.tableExists(tableName)) {
: throw new RuntimeException(s"Table does not exist: $tableName")
: }
> Won't openTable() already do this?
It will be this allows me to explicitly control the error message and avoid a large stacktrace deep into the client. More validation is going to be put in this job shortly too. So it will mesh better.
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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, 25 Apr 2019 20:04:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
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/13120
to look at the new patch set (#3).
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
KUDU-2790: Enhance the backup and restore tests
This patch enhances the backup and restore tests and
restructures the utility methods to be more flexible as
more tests are added.
Along with improvements to existing test the following
test methods were added:
- testForceFullBackup
- testRestoreWithNoBackup
- testBackupMissingTable
- testBackupAndRestoreWithNoRows
- testSimpleIncrementalBackupAndRestore
Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/BackupGraph.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/SessionIO.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestBackupGraph.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
5 files changed, 271 insertions(+), 151 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/13120/3
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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] KUDU-2790: Enhance the backup and restore tests
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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] KUDU-2790: Enhance the backup and restore tests
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/13120
to look at the new patch set (#4).
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
KUDU-2790: Enhance the backup and restore tests
This patch enhances the backup and restore tests and
restructures the utility methods to be more flexible as
more tests are added.
Along with improvements to existing test the following
test methods were added:
- testForceFullBackup
- testRestoreWithNoBackup
- testBackupMissingTable
- testBackupAndRestoreWithNoRows
- testSimpleIncrementalBackupAndRestore
Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/BackupGraph.scala
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/SessionIO.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestBackupGraph.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
4 files changed, 265 insertions(+), 151 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/13120/4
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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] KUDU-2790: Enhance the backup and restore tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 2:
(2 comments)
I didn't review the new tests super closely but the rest looks fine.
http://gerrit.cloudera.org:8080/#/c/13120/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13120/2//COMMIT_MSG@13
PS2, Line 13: Along with improvments to existing test the following
improvements
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@48
PS2, Line 48: // Verify there is a table to backup.
: if (!context.syncClient.tableExists(tableName)) {
: throw new RuntimeException(s"Table does not exist: $tableName")
: }
Won't openTable() already do this?
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
Gerrit-PatchSet: 2
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>
Gerrit-Comment-Date: Thu, 25 Apr 2019 20:01:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/13120/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@48
PS2, Line 48: // Verify there is a table to backup.
: if (!context.syncClient.tableExists(tableName)) {
: throw new RuntimeException(s"Table does not exist: $tableName")
: }
> It will be this allows me to explicitly control the error message and avoid
I looked into this further and I agree that openTable() doesn't do a great job of surfacing the underlying MasterErrorPB: unlike the C++ client which at least appends some textual representation of TABLE_NOT_FOUND to the Status, the Java client just swallows it up. You'd be forced to look for something like NotFound in the exception's status which is a little more abstract.
That said, tableExists() is a bad idea if Sentry integration is enabled. That's because tableExists() results in a ListTables RPC, and ListTables is going to perform really poorly with a cold Sentry cache: it'll send an RPC to Sentry for every table in the catalog. Coupled with the loop here (for each table being backed up) and it's bad news all around.
So maybe call getTableList() just once outside of the per-table loop, and then use that list to decide whether to throw here? Or drop the tableExists() call and make it work with openTable()'s exception?
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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, 25 Apr 2019 20:37:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2790: Enhance the backup and restore tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13120 )
Change subject: KUDU-2790: Enhance the backup and restore tests
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7300469887656d3f995ef063e915ca7255fbf0bb
Gerrit-Change-Number: 13120
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>
Gerrit-Comment-Date: Thu, 25 Apr 2019 21:11:03 +0000
Gerrit-HasComments: No