mirror of
https://github.com/google/nomulus.git
synced 2025-05-13 07:57:13 +02:00
Remove the reduntant 'afterFinalFailure' from Retrier
'afterFinalFailure' is called just before rethrowing a non-retrying error from the retrier. This can happen either because the exception shouldn't be retried, or because we exceeded the maximum number of retries. The same thing can be done by catching that thrown error outside of the retrier: retrier.callWithRetry( callable, new FailureReporter() { @Override void afterFinalFailure(Throwable thrown, int failures) { // do something with thrown } }, RetriableException.class); is (almost) the same as: try { retrier.callWithRetry(callable, RetriableException.class); } catch (Throwable thrown) { // do something with thrown throw thrown; } ("almost" because the retrier might wrap the Throwable in a RuntimeException, so you might need to getCause or getRootCause. Also - there is the "beforeRetry" I ignored for the example) Removing "afterFinalFailure" also makes the FailureReporter in line with Java 8 functional interface - meaning we can more easily create it when we do need to override "beforeRetry". ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=189972101
This commit is contained in:
parent
63785e5149
commit
552940a816
6 changed files with 135 additions and 177 deletions
|
@ -138,19 +138,9 @@ class CommitLogManifestReader extends InputReader<Key<CommitLogManifest>> {
|
|||
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 {
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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<String> manifestedFilesBuilder = new ImmutableList.Builder<>();
|
||||
for (ReportType reportType : reportTypes) {
|
||||
manifestedFilesBuilder.addAll(stager.stageReports(reportType));
|
||||
}
|
||||
ImmutableList<String> manifestedFiles = manifestedFilesBuilder.build();
|
||||
stager.createAndUploadManifest(manifestedFiles);
|
||||
try {
|
||||
retrier.callWithRetry(
|
||||
() -> {
|
||||
ImmutableList.Builder<String> manifestedFilesBuilder = new ImmutableList.Builder<>();
|
||||
for (ReportType reportType : reportTypes) {
|
||||
manifestedFilesBuilder.addAll(stager.stageReports(reportType));
|
||||
}
|
||||
ImmutableList<String> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -49,16 +49,6 @@ public class Retrier implements Serializable {
|
|||
* <p>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.
|
||||
*
|
||||
* <p>Called either after the final failure, or if the Throwable thrown isn't "a retriable
|
||||
* error". The retrier throws right after calling this function.
|
||||
*
|
||||
* <p>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);
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue