diff --git a/java/google/registry/mapreduce/inputs/CommitLogManifestReader.java b/java/google/registry/mapreduce/inputs/CommitLogManifestReader.java index 434c9fd56..67a4c5577 100644 --- a/java/google/registry/mapreduce/inputs/CommitLogManifestReader.java +++ b/java/google/registry/mapreduce/inputs/CommitLogManifestReader.java @@ -138,19 +138,9 @@ class CommitLogManifestReader extends InputReader> { try { return retrier.callWithRetry( () -> queryIterator.next(), - new Retrier.FailureReporter() { - @Override - public void beforeRetry(Throwable thrown, int failures, int maxAttempts) { - checkNotNull(currentCursor, "Can't retry because cursor is null. Giving up."); - queryIterator = query().startAt(currentCursor).keys().iterator(); - } - - @Override - public void afterFinalFailure(Throwable thrown, int failures) { - logger.severefmt( - "Max retry attempts reached trying to read item %d/%d. Giving up.", - loaded, total); - } + (thrown, failures, maxAttempts) -> { + checkNotNull(currentCursor, "Can't retry because cursor is null. Giving up."); + queryIterator = query().startAt(currentCursor).keys().iterator(); }, DatastoreTimeoutException.class); } finally { diff --git a/java/google/registry/reporting/billing/BillingEmailUtils.java b/java/google/registry/reporting/billing/BillingEmailUtils.java index 91d467f0d..32382d277 100644 --- a/java/google/registry/reporting/billing/BillingEmailUtils.java +++ b/java/google/registry/reporting/billing/BillingEmailUtils.java @@ -14,6 +14,7 @@ package google.registry.reporting.billing; +import static com.google.common.base.Throwables.getRootCause; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.appengine.tools.cloudstorage.GcsFilename; @@ -78,71 +79,68 @@ class BillingEmailUtils { /** Sends an e-mail to all expected recipients with an attached overall invoice from GCS. */ void emailOverallInvoice() { - retrier.callWithRetry( - () -> { - String invoiceFile = - String.format( - "%s-%s.csv", BillingModule.OVERALL_INVOICE_PREFIX, yearMonth.toString()); - GcsFilename invoiceFilename = - new GcsFilename(billingBucket, invoiceDirectoryPrefix + invoiceFile); - try (InputStream in = gcsUtils.openInputStream(invoiceFilename)) { - Message msg = emailService.createMessage(); - msg.setFrom(new InternetAddress(alertSenderAddress)); - for (String recipient : invoiceEmailRecipients) { - msg.addRecipient(RecipientType.TO, new InternetAddress(recipient)); - } - msg.setSubject(String.format("Domain Registry invoice data %s", yearMonth.toString())); - Multipart multipart = new MimeMultipart(); - BodyPart textPart = new MimeBodyPart(); - textPart.setText( + try { + retrier.callWithRetry( + () -> { + String invoiceFile = String.format( - "Attached is the %s invoice for the domain registry.", yearMonth.toString())); - multipart.addBodyPart(textPart); - BodyPart invoicePart = new MimeBodyPart(); - String invoiceData = CharStreams.toString(new InputStreamReader(in, UTF_8)); - invoicePart.setContent(invoiceData, "text/csv; charset=utf-8"); - invoicePart.setFileName(invoiceFile); - multipart.addBodyPart(invoicePart); - msg.setContent(multipart); - msg.saveChanges(); - emailService.sendMessage(msg); - } - }, - new Retrier.FailureReporter() { - @Override - public void beforeRetry(Throwable thrown, int failures, int maxAttempts) {} - - @Override - public void afterFinalFailure(Throwable thrown, int failures) { - sendAlertEmail( - String.format("Emailing invoice failed due to %s", thrown.getMessage())); - } - }, - IOException.class, - MessagingException.class); + "%s-%s.csv", BillingModule.OVERALL_INVOICE_PREFIX, yearMonth.toString()); + GcsFilename invoiceFilename = + new GcsFilename(billingBucket, invoiceDirectoryPrefix + invoiceFile); + try (InputStream in = gcsUtils.openInputStream(invoiceFilename)) { + Message msg = emailService.createMessage(); + msg.setFrom(new InternetAddress(alertSenderAddress)); + for (String recipient : invoiceEmailRecipients) { + msg.addRecipient(RecipientType.TO, new InternetAddress(recipient)); + } + msg.setSubject( + String.format("Domain Registry invoice data %s", yearMonth.toString())); + Multipart multipart = new MimeMultipart(); + BodyPart textPart = new MimeBodyPart(); + textPart.setText( + String.format( + "Attached is the %s invoice for the domain registry.", yearMonth.toString())); + multipart.addBodyPart(textPart); + BodyPart invoicePart = new MimeBodyPart(); + String invoiceData = CharStreams.toString(new InputStreamReader(in, UTF_8)); + invoicePart.setContent(invoiceData, "text/csv; charset=utf-8"); + invoicePart.setFileName(invoiceFile); + multipart.addBodyPart(invoicePart); + msg.setContent(multipart); + msg.saveChanges(); + emailService.sendMessage(msg); + } + }, + IOException.class, + MessagingException.class); + } catch (Throwable e) { + logger.severe(e, "Emailing invoice failed"); + // Strip one layer, because callWithRetry wraps in a RuntimeException + sendAlertEmail( + String.format( + "Emailing invoice failed due to %s", + getRootCause(e).getMessage())); + throw e; + } } /** Sends an e-mail to the provided alert e-mail address indicating a billing failure. */ void sendAlertEmail(String body) { - retrier.callWithRetry( - () -> { - Message msg = emailService.createMessage(); - msg.setFrom(new InternetAddress(alertSenderAddress)); - msg.addRecipient(RecipientType.TO, new InternetAddress(alertRecipientAddress)); - msg.setSubject(String.format("Billing Pipeline Alert: %s", yearMonth.toString())); - msg.setText(body); - emailService.sendMessage(msg); - return null; - }, - new Retrier.FailureReporter() { - @Override - public void beforeRetry(Throwable thrown, int failures, int maxAttempts) {} - - @Override - public void afterFinalFailure(Throwable thrown, int failures) { - logger.severe(thrown, "The alert e-mail system failed."); - } - }, - MessagingException.class); + try { + retrier.callWithRetry( + () -> { + Message msg = emailService.createMessage(); + msg.setFrom(new InternetAddress(alertSenderAddress)); + msg.addRecipient(RecipientType.TO, new InternetAddress(alertRecipientAddress)); + msg.setSubject(String.format("Billing Pipeline Alert: %s", yearMonth.toString())); + msg.setText(body); + emailService.sendMessage(msg); + return null; + }, + MessagingException.class); + } catch (Throwable e) { + logger.severe(e, "The alert e-mail system failed."); + throw e; + } } } diff --git a/java/google/registry/reporting/billing/CopyDetailReportsAction.java b/java/google/registry/reporting/billing/CopyDetailReportsAction.java index dff1e959a..89ab3fdf2 100644 --- a/java/google/registry/reporting/billing/CopyDetailReportsAction.java +++ b/java/google/registry/reporting/billing/CopyDetailReportsAction.java @@ -14,6 +14,7 @@ package google.registry.reporting.billing; +import static com.google.common.base.Throwables.getRootCause; import static google.registry.request.Action.Method.POST; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -104,34 +105,30 @@ public final class CopyDetailReportsAction implements Runnable { continue; } // Attempt to copy each detail report to its associated registrar's drive folder. - retrier.callWithRetry( - () -> { - try (InputStream input = - gcsUtils.openInputStream( - new GcsFilename(billingBucket, invoiceDirectoryPrefix + detailReportName))) { - driveConnection.createFile( - detailReportName, - MediaType.CSV_UTF_8, - driveFolderId, - ByteStreams.toByteArray(input)); - logger.infofmt( - "Published detail report for %s to folder %s using GCS file gs://%s/%s.", - registrarId, driveFolderId, billingBucket, detailReportName); - } - }, - new Retrier.FailureReporter() { - @Override - public void beforeRetry(Throwable thrown, int failures, int maxAttempts) {} - - @Override - public void afterFinalFailure(Throwable thrown, int failures) { - emailUtils.sendAlertEmail( - String.format( - "Warning: CopyDetailReportsAction failed.\nEncountered: %s on file: %s", - thrown.getMessage(), detailReportName)); - } - }, - IOException.class); + try { + retrier.callWithRetry( + () -> { + try (InputStream input = + gcsUtils.openInputStream( + new GcsFilename(billingBucket, invoiceDirectoryPrefix + detailReportName))) { + driveConnection.createFile( + detailReportName, + MediaType.CSV_UTF_8, + driveFolderId, + ByteStreams.toByteArray(input)); + logger.infofmt( + "Published detail report for %s to folder %s using GCS file gs://%s/%s.", + registrarId, driveFolderId, billingBucket, detailReportName); + } + }, + IOException.class); + } catch (Throwable e) { + emailUtils.sendAlertEmail( + String.format( + "Warning: CopyDetailReportsAction failed.\nEncountered: %s on file: %s", + getRootCause(e).getMessage(), detailReportName)); + throw e; + } } response.setStatus(SC_OK); response.setContentType(MediaType.PLAIN_TEXT_UTF_8); diff --git a/java/google/registry/reporting/icann/IcannReportingStagingAction.java b/java/google/registry/reporting/icann/IcannReportingStagingAction.java index b334eae43..e452fecf3 100644 --- a/java/google/registry/reporting/icann/IcannReportingStagingAction.java +++ b/java/google/registry/reporting/icann/IcannReportingStagingAction.java @@ -14,6 +14,7 @@ package google.registry.reporting.icann; +import static com.google.common.base.Throwables.getRootCause; import static google.registry.request.Action.Method.POST; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -73,50 +74,51 @@ public final class IcannReportingStagingAction implements Runnable { @Override public void run() { - retrier.callWithRetry( - () -> { - ImmutableList.Builder manifestedFilesBuilder = new ImmutableList.Builder<>(); - for (ReportType reportType : reportTypes) { - manifestedFilesBuilder.addAll(stager.stageReports(reportType)); - } - ImmutableList manifestedFiles = manifestedFilesBuilder.build(); - stager.createAndUploadManifest(manifestedFiles); + try { + retrier.callWithRetry( + () -> { + ImmutableList.Builder manifestedFilesBuilder = new ImmutableList.Builder<>(); + for (ReportType reportType : reportTypes) { + manifestedFilesBuilder.addAll(stager.stageReports(reportType)); + } + ImmutableList manifestedFiles = manifestedFilesBuilder.build(); + stager.createAndUploadManifest(manifestedFiles); - logger.infofmt("Completed staging %d report files.", manifestedFiles.size()); - emailUtils.emailResults( - "ICANN Monthly report staging summary [SUCCESS]", - String.format( - "Completed staging the following %d ICANN reports:\n%s", - manifestedFiles.size(), Joiner.on('\n').join(manifestedFiles))); - - response.setStatus(SC_OK); - response.setContentType(MediaType.PLAIN_TEXT_UTF_8); - response.setPayload("Completed staging action."); - - logger.infofmt("Enqueueing report upload :"); - TaskOptions uploadTask = TaskOptions.Builder.withUrl(IcannReportingUploadAction.PATH) - .method(Method.POST) - .countdownMillis(Duration.standardMinutes(2).getMillis()) - .param(IcannReportingModule.PARAM_SUBDIR, subdir); - QueueFactory.getQueue(CRON_QUEUE).add(uploadTask); - return null; - }, - new Retrier.FailureReporter() { - @Override - public void beforeRetry(Throwable thrown, int failures, int maxAttempts) {} - - @Override - public void afterFinalFailure(Throwable thrown, int failures) { + logger.infofmt("Completed staging %d report files.", manifestedFiles.size()); emailUtils.emailResults( - "ICANN Monthly report staging summary [FAILURE]", + "ICANN Monthly report staging summary [SUCCESS]", String.format( - "Staging failed due to %s, check logs for more details.", thrown.toString())); - logger.severe(thrown, "Staging action failed."); - response.setStatus(SC_INTERNAL_SERVER_ERROR); + "Completed staging the following %d ICANN reports:\n%s", + manifestedFiles.size(), Joiner.on('\n').join(manifestedFiles))); + + response.setStatus(SC_OK); response.setContentType(MediaType.PLAIN_TEXT_UTF_8); - response.setPayload(String.format("Staging failed due to %s", thrown.toString())); - } - }, - BigqueryJobFailureException.class); + response.setPayload("Completed staging action."); + + logger.infofmt("Enqueueing report upload :"); + TaskOptions uploadTask = + TaskOptions.Builder.withUrl(IcannReportingUploadAction.PATH) + .method(Method.POST) + .countdownMillis(Duration.standardMinutes(2).getMillis()) + .param(IcannReportingModule.PARAM_SUBDIR, subdir); + QueueFactory.getQueue(CRON_QUEUE).add(uploadTask); + return null; + }, + BigqueryJobFailureException.class); + } catch (Throwable e) { + emailUtils.emailResults( + "ICANN Monthly report staging summary [FAILURE]", + String.format( + "Staging failed due to %s, check logs for more details.", + getRootCause(e).toString())); + logger.severe(e, "Staging action failed."); + response.setStatus(SC_INTERNAL_SERVER_ERROR); + response.setContentType(MediaType.PLAIN_TEXT_UTF_8); + response.setPayload( + String.format( + "Staging failed due to %s", + getRootCause(e).toString())); + throw e; + } } } diff --git a/java/google/registry/util/Retrier.java b/java/google/registry/util/Retrier.java index 2f45ba4af..69553b440 100644 --- a/java/google/registry/util/Retrier.java +++ b/java/google/registry/util/Retrier.java @@ -49,16 +49,6 @@ public class Retrier implements Serializable { *

Not called at all if the retrier succeeded on its first attempt. */ void beforeRetry(Throwable thrown, int failures, int maxAttempts); - - /** - * Called after a a non-retriable error. - * - *

Called either after the final failure, or if the Throwable thrown isn't "a retriable - * error". The retrier throws right after calling this function. - * - *

Not called at all if the retrier succeeds. - */ - void afterFinalFailure(Throwable thrown, int failures); } @Inject @@ -89,7 +79,6 @@ public class Retrier implements Serializable { return callable.call(); } catch (Throwable e) { if (++failures == attempts || !isRetryable.test(e)) { - failureReporter.afterFinalFailure(e, failures); throwIfUnchecked(e); throw new RuntimeException(e); } @@ -180,13 +169,6 @@ public class Retrier implements Serializable { } private static final FailureReporter LOGGING_FAILURE_REPORTER = - new FailureReporter() { - @Override - public void beforeRetry(Throwable thrown, int failures, int maxAttempts) { - logger.infofmt(thrown, "Retrying transient error, attempt %d", failures); - } - - @Override - public void afterFinalFailure(Throwable thrown, int failures) {} - }; + (thrown, failures, maxAttempts) -> + logger.infofmt(thrown, "Retrying transient error, attempt %d/%d", failures, maxAttempts); } diff --git a/javatests/google/registry/util/RetrierTest.java b/javatests/google/registry/util/RetrierTest.java index 2e44d02ea..ab955d539 100644 --- a/javatests/google/registry/util/RetrierTest.java +++ b/javatests/google/registry/util/RetrierTest.java @@ -61,23 +61,12 @@ public class RetrierTest { static class TestReporter implements FailureReporter { int numBeforeRetry = 0; - int numOnFinalFailure = 0; @Override public void beforeRetry(Throwable e, int failures, int maxAttempts) { numBeforeRetry++; assertThat(failures).isEqualTo(numBeforeRetry); } - - @Override - public void afterFinalFailure(Throwable e, int failures) { - numOnFinalFailure++; - } - - void assertNumbers(int expectedBeforeRetry, int expectedOnFinalFailure) { - assertThat(numBeforeRetry).isEqualTo(expectedBeforeRetry); - assertThat(numOnFinalFailure).isEqualTo(expectedOnFinalFailure); - } } @Test @@ -114,7 +103,7 @@ public class RetrierTest { try { retrier.callWithRetry(new CountingThrower(3), reporter, CountingException.class); } catch (CountingException expected) { - reporter.assertNumbers(2, 1); + assertThat(reporter.numBeforeRetry).isEqualTo(2); throw expected; } }); @@ -126,7 +115,7 @@ public class RetrierTest { TestReporter reporter = new TestReporter(); assertThat(retrier.callWithRetry(new CountingThrower(2), reporter, CountingException.class)) .isEqualTo(2); - reporter.assertNumbers(2, 0); + assertThat(reporter.numBeforeRetry).isEqualTo(2); } @Test @@ -134,6 +123,6 @@ public class RetrierTest { TestReporter reporter = new TestReporter(); assertThat(retrier.callWithRetry(new CountingThrower(0), reporter, CountingException.class)) .isEqualTo(0); - reporter.assertNumbers(0, 0); + assertThat(reporter.numBeforeRetry).isEqualTo(0); } }