Using ScheduledExecutorService instead of Thread.sleep

[DariusX]

How about adding an implementation of Sleeper, based on ScheduledExecutorService that one could use if ThreadWaitSleeper is blocking too many threads?

[ashamukov]

Hi Darius! Could you please provide an example of detailed usage scenario?
Now, I'm concerned about one thing: public API of RetryOperations is synchronous, because it returns values directly from methods.
Even if retryCallback will be executed without blocking the executing thread while backoff, caller thread (that calls execute(...)) should still blocks until whole operation completes of fails.
Consequently, to really improve whole process we should introduce asynchronous execute api first.
What do you think?

[DariusX]

The use-case would be very similar to using the default sleeper. Only the implementation would not block thread.
I was looking into another retry library, that takes an asynchronous approach. It's described here: https://www.nurkiewicz.com/2013/07/asynchronous-retry-pattern.html
So, I was wondering if that would be a possible enhancement to Spring Retry.

As you point out, since the public API is synchronous, this may be asking the impossible. The client code that calls the API would also need to be aware of the asynchronous approach, and will need to work with CompletableFuture, etc

[dsyer]

I had wondered some time ago whether RetryTemplate could be adapted to behave differently for a RetryCallback that returns an async type (like a Future for instance). I don't see any reason in principle why not, but the implementation is likely to get a little hairy. If anyone is interested in prototyping something that is where I would start.

[ashamukov]

I was looking into another retry library, that takes an asynchronous approach. It's described here: https://www.nurkiewicz.com/2013/07/asynchronous-retry-pattern.html

@DariusX , thanks for the link, haven't seen it before

[DariusX]

I had a thought on whether the public API is currently synchronous: to the extent that the public API is declarative (i.e. incorporated via annotations), it isn't really specifically synchronous. Not sure where to go with that thought, but I'm putting it out there in case is sparks an idea.

[dsyer]

There is a public API in RetryTemplate too, and RetryCallback, neither of which is explicitly synchronous, since they are parameterized with the return type, and that can be an async type at runtime. What is missing there is special case code inside RetryTemplate to deal with those return types.

BackoffPolicy is more likely to cause an issue. But I'm not sure it would be a complete blocker.

[dsyer]

Here's a prototype that works for Future and CompletableFuture: https://github.com/dsyer/spring-retry/tree/future. Similar features for e.g. reactor types (Mono in particular) would be trivial. To make it work on master we would probably need some wrappers for Executor and other thread pool abstractions (like in Spring Security) so that the RetrySynchronizationManager can still work.

Streaming types would be more of a challenge - you have to deal with the issue of whether to resume at the point where the exception occurred, or replay the whole result. Probably the latter is the best default.

[ashamukov]

Dave, thank for prototype, this reusing of existing api looks very elegant for me.
Now, I'm thinking about, how to avoid Thread.sleep'ing in backoffs.
The only way I see now is to give to template an instrument for "scheduling after delay", like this:
ashamukov@f041565#diff-c660155233131833aeaf6d5df2ca9b2bR151
Or, do you have another idea how to free the executing thread while backoff?

[dsyer]

Yes, that bit will be hard. I’m not sure I want add new methods to existing interfaces yet, either. Also note that Spring has some abstractions already for scheduling and triggers.

Another missing piece is intercepting callbacks from the user registered for the retry result (eg a CompletableFuture) and ensuring that they are applied to the eventually successful return value.

[ashamukov]

It looks like I've solved a half of the problems, hope to come up with an extended prototype in a few weeks.

Meanwhile, about support of java.util.concurrent.Future: because it has no "thenApply"-like notation, exception handling and retry scheduling for actual job can not be performed by worker right after previous attempt fails, i.e. invocation of "resultFuture.get()" is required to start even the first retry of actual job. That mixes sync and async approaches in a not very beautiful way, so I propose to not specially support java.util.concurrent.Future at all. (can show the details in my working copy)

[ashamukov]

Currently have pretty small free time, so, decided at least to commit my old draft to discuss the approach in general. Here is the PR: #176

[dsyer]

I get what you mean about Future and mixing async with non-async, but on the other hand, that's what a Future is, so if the user decided to use it, then that's what she wants. I expect, as a user, that I would need to call .get() to access the result, so I would be fine with that triggering a retry, as necessary.

I'll have a quick look at your backoff implementation. Sounds interesting.