context.isExhaustedOnly() returns false after the task is completed(It should be true, whether it succeeds or fails)

[yahyaoo]

Before explaining the problem, I am very sorry that my English level may not be very good. If you have any questions or I do not clearly express, please remember to ask or send me an email [email protected]. Thank you.

Brief description

If a RetryCallback executed completes (whether many failures can retries until exhausted or successful completion of the task), RetryContext.IsExhaustedOnly()The method should always return true.

The user can in the framework of the listener, and other places in the direct call RetryContext.isExhaustedOnly() to judge whether a task has already completed,but not need to use RetryContext.getRetryCount()>=MAX_ATTEMPT to judge whether a task has already completed.

logically, after a retryable callback task completes, it should not be able to retry again, even if it is a stateful or restored retry,namely RetryContext.isExhaustedOnly() is true forever.

Problem of repetition

Let me elaborate on some problems I've encountered in my company projects using simple code.

Here is my test case code:

    @Test
    public void exhaustedTest() {
        RetryTemplate retryTemplate = RetryTemplate.builder()
                .maxAttempts(3)
                .fixedBackoff(500)
                .withListener(new RetryListenerSupport() {
                    @Override
                    public <T, E extends Throwable> void onError(RetryContext context, RetryCallback<T, E> callback, Throwable throwable) {

                        if (log.isDebugEnabled() && !context.isExhaustedOnly()) {
                            log.debug(MessageFormat.format("An exception has occurred. Will try again. Now retry times:", context.getRetryCount()), throwable);
                        }

                        if (context.isExhaustedOnly()) {
                            //It never be executed
                            log.info("onError,context.isExhaustedOnly(),The maximum number of retries has been reached, and an exception will be thrown.");
                        }

                        if (context.getRetryCount() >= 3) {
                            //The last retry will be called
                            log.info("onError,context.getRetryCount() >= 3,The maximum number of retries has been reached, and an exception will be thrown.");
                        }
                    }
                })
                .build();

        Object test_exception = retryTemplate.execute(

                (RetryCallback<Object, RuntimeException>) context -> {
                    throw new RuntimeException("test exception");
                },

                context -> {

                    if (context.isExhaustedOnly()) {
                        //It never be executed
                        log.info("recover,context.isExhaustedOnly(),The maximum number of retries has been reached, and an exception will be thrown.");
                    }

                    if (context.getRetryCount() >= 3) {
                        //It always be executed
                        log.info("recover,context.getRetryCount() >= 3,The maximum number of retries has been reached, and an exception will be thrown.");
                    }

                    return "complete";
                });

        log.info(test_exception.toString());
    }

You can see from the above test case.Even if the maximum number of retries is 3,context.isExhaustedOnly() method still returns false,
indicating that the framework thinks the retryable callback task is still incomplete,but context.getRetryCount() shows that the maximum number of retries has been three.

I think, retryable callback task (regardless of success or a failure), after the completion of should be context.isExhaustedOnly() set to true.

How to improve

I think, this is part of the logic in the RetryTemplate.Here I point out my improvement methods.

	protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback,
			RecoveryCallback<T> recoveryCallback, RetryState state) throws E, ExhaustedRetryException {
        
		...//Omit the less important parts of the problem

		//boolean exhausted = false; //We don't need this variable here
		try {

			...//Omit the less important parts of the problem

			while (canRetry(retryPolicy, context) && !context.isExhaustedOnly()) {
				try {
					if (this.logger.isDebugEnabled()) {
						this.logger.debug("Retry: count=" + context.getRetryCount());
					}
					// Reset the last exception, so if we are successful
					// the close interceptors will not think we failed...
					lastException = null;
					T result = retryCallback.doWithRetry(context);
					context.setExhaustedOnly(); //here
					return result;
				}
				catch (Throwable e) {

					lastException = e;

					try {
						registerThrowable(retryPolicy, state, context, e);
					}
					catch (Exception ex) {
						throw new TerminatedRetryException("Could not register throwable", ex);
					}
					finally {
						/*
						* We must decide whether to need to retry again before call interceptor,
						* otherwise the context in the interceptor,
						* IsExhaustedOnly() will not be able to obtain the correct results.
						* And we must call it after registering the exception with the registerThrowable() method.
						* Because the registration exception method contains the logic to calculate whether you need to retry the next time.
						* We must call setExhaustedOnly() depending on the situation after it determines whether it needs to retry the next time.
						* Of course, the task of calling context.setExhaustedOnly() could also be put in the charge of registerThrowable()
						* if registerThrowable() decided that there was no need to try again.
						* I'm just giving you an example of when you need to decide if you can retry.
						* */
						if (!canRetry(retryPolicy, context)){
							context.setExhaustedOnly();
						}
						doOnErrorInterceptors(retryCallback, context, e);
					}

					...//Omit the less important parts of the problem

				}
			}

			if (state == null && this.logger.isDebugEnabled()) {
				this.logger.debug("Retry failed last attempt: count=" + context.getRetryCount());
			}

			//exhausted = true;// don't need

			return handleRetryExhausted(recoveryCallback, context, state);

		}
		catch (Throwable e) {
			throw RetryTemplate.<E>wrapIfNecessary(e);
		}
		finally {
			close(retryPolicy, context, state, lastException == null || context.isExhaustedOnly());//use isExhaustedOnly()
			doCloseInterceptors(retryCallback, context, lastException);
			RetrySynchronizationManager.clear();
		}

	}

In particular, if the callback call is successful,and there’s no need to try again.
We need to manually set the context.setExhaustedOnly(); in the return value of method.

If the current callback fails,we can call context.setExhaustedOnly(); The task is give to registerThrowable(retryPolicy, state, context, e);.

registerThrowable(retryPolicy, state, context, e);The function's original mission is to determine whether the next retry can be made.
We can do this by checking context.setExhaustedOnly(); to assign.

See the code for more details.

End

So that's my general logic.If there is something the author or other developer does not understand.Everyone is welcome to write to me or comment in the issue.Due to the time difference, I may not be able to reply in time, but if I see it, I will definitely answer the question.
Or if you don't understand my suggestion, please point it out.thank you.