-
Notifications
You must be signed in to change notification settings - Fork 301
Description
The Execution and AsyncExecution APIs are meant to make it easy to integrate with third party libraries or APIs where some portion of retry or execution logic is built into that library, and Failsafe just needs to do the rest. That said, they are a bit odd, having been created in the early days of Failsafe, and these APIs are very specific to retries as opposed to executions in general.
Execution
Current example:
// Standalone Execution
Execution execution = new Execution(retryPolicy);
if (execution.canRetryOn(someFailure))
service.scheduleRetry(execution.getWaitTime().toNanos(), TimeUnit.MILLISECONDS);
A better idea might be something that focuses on "execution" semantics rather than retries, since retries are just one reason an execution might be needed or not, given whatever policies are in use. So a better idea might be:
execution.recordFailure(someFailure);
if (execution.canPerform())
service.scheduleRetry(execution.getWaitTime().toNanos(), TimeUnit.MILLISECONDS);
AsyncExecution
Current example:
// Async execution
Failsafe.with(retryPolicy)
.getAsyncExecution(execution -> service.connect().whenComplete((result, failure) -> {
if (execution.complete(result, failure))
log.info("Connected");
else if (execution.retry())
log.info("Retrying connection")
else
log.error("Connection attempts failed", failure);
}));
Whether we're scheduling a retry or not should not really be a concern here. We should just attempt to handle the execution, and a retry either occurs or not based on whatever policies are configured:
Failsafe.with(retryPolicy)
.getAsyncExecution(execution -> service.connect().whenComplete((result, failure) -> {
execution.handle(result, failure); // May trigger a retry, or not
}));
While the current AsyncExecution
API allows for logging when an execution completes or is retried:
if (execution.complete(result, failure))
log.info("Connected");
else if (execution.retry())
log.info("Retrying connection")
else
log.error("Connection attempts failed", failure);
...Failsafe's event listeners already support this. So we could change the original example to something like:
retryPolicy.onRetry(e -> "Retrying connection");
Failsafe.with(retryPolicy)
.onSuccess(e -> log.info("Connected"))
.onFailure(e -> log.error("Connection attempts failed", e.getFailure()))
.getAsyncExecution(execution -> service.connect().whenComplete((result, failure) -> {
execution.handle(result, failure);
}));
And we're not losing any capabilities, just moving them around somewhat.
Thoughts?