[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
waleed.fat...@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15638 Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala 2 files changed, 16 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15638/1 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15638 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15638/1/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/15638/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@174 PS1, Line 174: thrown I think this is a typo and fails to compile. You can edit and test by using the below in the java directory: `./gradlew :kudu-backup:check` I also suspect you will need to adjust the tests slightly. Like here for example: https://github.com/apache/kudu/blob/master/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala#L698 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 03 Apr 2020 20:51:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
waleed.fat...@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15646 Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. TestKuduBackup.scala was modified where assertFalse() is used to check for failure and assertTrue() for success. Change-Id: I7014c3c4eac0f622afb0508cd97226b1d32fb904 --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 36 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/15646/1 -- To view, visit http://gerrit.cloudera.org:8080/15646 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7014c3c4eac0f622afb0508cd97226b1d32fb904 Gerrit-Change-Number: 15646 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15638 to look at the new patch set (#2). Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. TestKuduBackup.scala was modified where assertFalse() is used to check for failure and assertTrue() for success. Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 36 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15638/2 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
waleed.fat...@gmail.com has abandoned this change. ( http://gerrit.cloudera.org:8080/15646 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Abandoned Just added patch to change 15638 which has Grant's review comments. -- To view, visit http://gerrit.cloudera.org:8080/15646 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I7014c3c4eac0f622afb0508cd97226b1d32fb904 Gerrit-Change-Number: 15646 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15638 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15638/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/15638/2/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@175 PS2, Line 175: assertEquals(1, KuduBackup.run(options, ss))) Missed this one. -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 04 Apr 2020 16:48:31 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15638 to look at the new patch set (#3). Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. TestKuduBackup.scala was modified where assertFalse() is used to check for failure and assertTrue() for success. Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 37 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15638/3 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15638 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 06 Apr 2020 19:05:35 + Gerrit-HasComments: No
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15638 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Patch Set 3: Code-Review+2 Thanks for the contribution! -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 06 Apr 2020 19:06:18 + Gerrit-HasComments: No
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has removed a vote on this change. Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15638 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. TestKuduBackup.scala was modified where assertFalse() is used to check for failure and assertTrue() for success. Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Reviewed-on: http://gerrit.cloudera.org:8080/15638 Tested-by: Grant Henke Reviewed-by: Grant Henke --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 37 insertions(+), 33 deletions(-) Approvals: Grant Henke: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
