ExponentialBackOffContext not considering the interval set by application.properties

[charles-effting]

Hello there,
I'm reporting a bug that I found in the class ExponentialBackOffPolicy when trying to configure it with the application.properties.

Basically the interval set by the delayExpression in the annotation @Retryable is not used when calling the method getNextInterval, but rather the one set by delay. Consequentially that leads to wrong exponential retries. We can see in the screenshot below that the class field this.interval is being used instead of the getInterval method which evaluates the delayExpression.

To reproduce the issue:

• Configure the application.properties:

# Distribute the recovery attempts over a period of 15 minutes
recovery.back-off.max-attempts=5
# 1 minute
recovery.back-off.delay=60000
# 15 minutes
recovery.back-off.max-delay=900000
recovery.back-off.multiplier=2

• Then configure a bean with @Retryable:

@Retryable(maxAttemptsExpression = "${recovery.back-off.max-attempts}",
                backoff = @Backoff(delayExpression = "${recovery.back-off.delay}",
                        maxDelayExpression = "${recovery.back-off.max-delay}",
                        multiplierExpression = "${recovery.back-off.multiplier}"))
        void reconnect(SomethingToRetry retry) {
            retry.tryToConnect();
        }

When running the code the retry is attempted every 1 minute what should not be the case.

An initial fix would be to call the getInterval, however as I don't have a complete overview of the library, it might be needed further changes.

Thank you and if you have more questions, just let me know.

Cheers!

[artembilan]

The logic there in the AnnotationAwareRetryOperationsInterceptor is like this:

		if (minExp != null) {
			builder.delaySupplier(() -> evaluate(minExp, Long.class, stateless));
		}

So, that delayExpression is evaluated only once when this.initialIntervalSupplier.get() is called in the ExponentialBackOffPolicy.
The getInterval() is called from the getSleepAndIncrement():

long sleep = getInterval();

So, we call supplier only once and set its value into the this.interval property.
The getSleepAndIncrement() is called from the backOff().

So, everything around delayExpression looks legit.

I would be happy to see some unit test to prove opposite, but before that let's see you really use the latest Spring Retry 2.0.5!

[charles-effting]

Thanks for the quick reply. So I noticed this issue around 2 weeks ago and tried the latest version 2.0.4 then, but no luck. That was in my backlog and today I took the time to report it without noticing that there is a fresh 2.0.5. That's good timing 😄

I prepared the unit tests anyway just to be sure that I didn't miss something. Up to 2.0.4 I could reproduce the problem. Project files: retry-test-case.zip

Here the output of the test shouldDistributeAttemptsOver15Minutes using delay works as expected:

And shouldDistributeAttemptsOver15MinutesWithExpression is triggered every minute instead.

Nonetheless the problem is gone in 2.0.5 and this issue is not relevant anymore. Many thanks!

[artembilan]

Yep! See release notes: https://github.com/spring-projects/spring-retry/releases/tag/v2.0.5.

The issue which has been fixed on the matter is this: #397.

So, closing this as Duplicate.