Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13416 )
Change subject: [backup] KUDU-2787 Pt 2 Allow single table failures for restore ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/13416/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13416/2//COMMIT_MSG@7 PS2, Line 7: [backup] KUDU-2787 Pt 2 Allow single table failures for restore > s/backup and //; consider noting as part 2 of KUDU-2787 Done http://gerrit.cloudera.org:8080/#/c/13416/2/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/13416/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@164 PS2, Line 164: TODO > mind filing a jira for this? Done http://gerrit.cloudera.org:8080/#/c/13416/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@166 PS2, Line 166: changed. We ought to try to clean up the mess when a failure > Are you thinking we should drop the table if we couldn't successfully resto Yeah, so a retry of the job wouldn't auto-fail. http://gerrit.cloudera.org:8080/#/c/13416/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@187 PS2, Line 187: if (restoreResults.exists(_._2.isFailure)) > See my comments on the backup patch. See my responses there. http://gerrit.cloudera.org:8080/#/c/13416/2/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@299 PS2, Line 299: System.exit(run(options, session)) > See my comments on the backup patch. Ditto. http://gerrit.cloudera.org:8080/#/c/13416/2/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/13416/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@659 PS2, Line 659: runRestor > nit: can we rename this to runRestore() to avoid confusion with the method Done -- To view, visit http://gerrit.cloudera.org:8080/13416 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9dcc0be78e637b47f35e494dce4a6df274c8d559 Gerrit-Change-Number: 13416 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Wed, 29 May 2019 18:05:32 +0000 Gerrit-HasComments: Yes
