Skip to content

Introduce minimal retry functionality as a core framework feature #34716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Introduce minimal retry functionality as a core framework feature #34716

wants to merge 1 commit into from
+1,136 −0

Conversation

fmbenhassine
Copy link
Contributor

@fmbenhassine fmbenhassine commented Apr 4, 2025 •
edited
Loading

This PR introduces a minimal core retry feature in Spring Framework. It is inspired by Spring Retry, but redesigned and trimmed to the bare minimum to cover most cases.

The main entry point is the RetryTemplate class, which is designed in a similar fashion to other templates provided by Spring. The package is minimal on purpose to keep its usage as simple as possible (see examples in RetryTemplateTests). It focuses only on stateless retries for now, but can be extended with other retry operations as well.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 4, 2025
@sbrannen sbrannen changed the title Introduce a minimal retry functionality as a core framework feature Introduce minimal retry functionality as a core framework feature Apr 4, 2025
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Apr 4, 2025
sbrannen
sbrannen requested changes Apr 4, 2025
View reviewed changes
Copy link
Member

@sbrannen sbrannen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fmbenhassine,

First and foremost, thanks for putting this PR together! 👍

I took a quick look, added a few comments, and asked a few questions (as food for thought).

And we will also discuss the proposal within the team.

Cheers,

Sam

@fmbenhassine
Copy link
Contributor Author

Thank you for the review, @sbrannen !

I added a couple of inline comments and I will update the PR accordingly.

@fmbenhassine
Copy link
Contributor Author

I updated the PR with the following changes:

  • Add/Use a specific RetryException in the API instead of the generic Exception
  • Add RetryCallback interface with a logical name for diagnostics purposes
  • Improve RetryPolicy with the ability to specify which exceptions to retry based on a Predicate<Exception>

I find this new version to be more coherent and consistent with other templates in terms of structure and API. Looking forward to the team's thoughts on this!

@fmbenhassine fmbenhassine force-pushed the spring-retry branch 2 times, most recently from 5facde5 to b7a7ca6 Compare May 27, 2025 13:22
@fmbenhassine fmbenhassine requested a review from sbrannen May 27, 2025 13:25
artembilan
artembilan suggested changes May 27, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great even if I have left so many questions.
Thanks

@fmbenhassine
Copy link
Contributor Author

Thank you for the review, @artembilan and @sbrannen !

I updated the PR accordingly and added a few comments. Please let me know if we need other updates.

The build of the PR is failing for tests that are not related to this change set.

@spencergibb
Copy link
Member

👋🏻 Joining the conversation here as Spring Cloud is an advanced user of Spring Retry, so we would be interested in how the functionality compares to the Spring Retry project.

@spencergibb

This comment was marked as outdated.

Sign in to view
@sbrannen

This comment was marked as outdated.

Sign in to view
sbrannen
sbrannen requested changes Jun 4, 2025
View reviewed changes
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 4, 2025
@jhoeller jhoeller added this to the 7.0.0-M6 milestone Jun 4, 2025
@fmbenhassine
Introduce a minimal retry functionality as a core framework feature
3b68551 
This commit introduces a minimal core retry feature. It is inspired
by Spring Retry, but redesigned and trimmed to the bare minimum to
cover most cases.
@sbrannen sbrannen closed this in 3fb4a75 Jun 5, 2025
sbrannen added a commit that referenced this pull request Jun 5, 2025
@sbrannen
Revise core retry support
02af9e5 
This commit constitutes a first pass over the new core retry support.

- Fix code and Javadoc formatting

- Polish/fix Javadoc

- Fix default FixedBackOff configuration in RetryTemplate

- Consistent logging in RetryTemplate

- Fix listener handling in CompositeRetryListener, allowing addListener()
  to work

- Polish tests

- Ensure RetryTemplateTests do not take over 30 seconds to execute

See gh-34716
@sbrannen
Copy link
Member

sbrannen commented Jun 5, 2025

This has been merged into main in 3fb4a75 and slightly revised in 02af9e5. 🚀

Thanks everybody for the collaborative effort to get this into 7.0!

@jhoeller jhoeller mentioned this pull request Jun 5, 2025
Closed
@fmbenhassine fmbenhassine deleted the spring-retry branch June 9, 2025 07:06
@fmbenhassine fmbenhassine mentioned this pull request Jun 9, 2025
Open
sbrannen added a commit that referenced this pull request Jun 10, 2025
@sbrannen
Rename RetryCallback to Retryable
51b6e8c 
See gh-34716
sbrannen added a commit that referenced this pull request Jun 10, 2025
@sbrannen
Fix wording in Javadoc for RetryTemplate
d42d3f1 
See gh-34716
sbrannen added a commit that referenced this pull request Jun 11, 2025
@sbrannen
Polish Javadoc for core retry functionality
d74b863 
See gh-34716
sbrannen added a commit that referenced this pull request Jun 11, 2025
@sbrannen
Rename Retryable.run() to Retryable.execute()
8f3ca49 
See gh-34716
sbrannen added a commit that referenced this pull request Jun 11, 2025
@sbrannen
Change signature of RetryOperations.execute() regarding nullability
b668042 
Due to lacking support in NullAway for the current arrangement, we are
(perhaps temporarily) changing the signature of the execute() method in
RetryOperations (and thus also in RetryTemplate)...

from: <R extends @nullable Object> R execute(Retryable<R> retryable);

to:   <R> @nullable R execute(Retryable<? extends @nullable R> retryable);

Once uber/NullAway#1075 has been resolved, we
will consider switching back to the original signature.

See gh-34716
@sbrannen sbrannen mentioned this pull request Jun 26, 2025
Closed
4 tasks
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Jun 27, 2025
@sbrannen
Redesign RetryPolicy to directly incorporate BackOff
9a78a8e 
After experimenting with our newly introduced core retry support
(RetryPolicy, RetryTemplate, etc.) and @⁠Retryable support, it
became apparent that there are overlapping concerns between the current
RetryPolicy and BackOff contracts.

- RetryPolicy and BackOff both have stateful executions: RetryExecution
  and BackOffExecution. However, only one stateful execution is
  necessary.

- FixedBackOff and ExponentialBackOff already incorporate "retry" logic
  in terms of max attempts, max elapsed time, etc. Thus, there is no
  need to duplicate such behavior in a RetryPolicy and its
  RetryExecution.

- RetryTemplate currently accepts both a RetryPolicy and a BackOff in
  order to instrument the retry algorithm. However, users would
  probably rather focus on configuring all "retry" logic via a single
  mechanism.

In light of the above, this commit directly incorporates BackOff
in RetryPolicy as follows.

- Remove the RetryExecution interface and move its shouldRetry() method
  to RetryPolicy, replacing the current RetryExecution start() method.

- Introduce a default getBackOff() method in the RetryPolicy interface.

- Introduce RetryPolicy.withDefaults() factory method.

- Completely overhaul the RetryPolicy.Builder to provide support for
  configuring a BackOff strategy.

- Remove BackOff configuration from RetryTemplate.

- Revise the method signatures of callbacks in RetryListener.

The collective result of these changes can be witnessed in the
reworked implementation of AbstractRetryInterceptor.

RetryPolicy retryPolicy = RetryPolicy.builder()
		.includes(spec.includes())
		.excludes(spec.excludes())
		.predicate(spec.predicate().forMethod(method))
		.maxAttempts(spec.maxAttempts())
		.delay(Duration.ofMillis(spec.delay()))
		.maxDelay(Duration.ofMillis(spec.maxDelay()))
		.jitter(Duration.ofMillis(spec.jitter()))
		.multiplier(spec.multiplier())
		.build();

RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);

See spring-projectsgh-34716
See spring-projectsgh-34529
See spring-projectsgh-35058
Closes spring-projectsgh-35110
sbrannen added a commit that referenced this pull request Jun 27, 2025
@sbrannen
Redesign RetryPolicy to directly incorporate BackOff
bcdf26d 
After experimenting with our newly introduced core retry support
(RetryPolicy, RetryTemplate, etc.) and @⁠Retryable support, it
became apparent that there are overlapping concerns between the current
RetryPolicy and BackOff contracts.

- RetryPolicy and BackOff both have stateful executions: RetryExecution
  and BackOffExecution. However, only one stateful execution is
  necessary.

- FixedBackOff and ExponentialBackOff already incorporate "retry" logic
  in terms of max attempts, max elapsed time, etc. Thus, there is no
  need to duplicate such behavior in a RetryPolicy and its
  RetryExecution.

- RetryTemplate currently accepts both a RetryPolicy and a BackOff in
  order to instrument the retry algorithm. However, users would
  probably rather focus on configuring all "retry" logic via a single
  mechanism.

In light of the above, this commit directly incorporates BackOff
in RetryPolicy as follows.

- Remove the RetryExecution interface and move its shouldRetry() method
  to RetryPolicy, replacing the current RetryExecution start() method.

- Introduce a default getBackOff() method in the RetryPolicy interface.

- Introduce RetryPolicy.withDefaults() factory method.

- Completely overhaul the RetryPolicy.Builder to provide support for
  configuring a BackOff strategy.

- Remove BackOff configuration from RetryTemplate.

- Revise the method signatures of callbacks in RetryListener.

The collective result of these changes can be witnessed in the
reworked implementation of AbstractRetryInterceptor.

RetryPolicy retryPolicy = RetryPolicy.builder()
		.includes(spec.includes())
		.excludes(spec.excludes())
		.predicate(spec.predicate().forMethod(method))
		.maxAttempts(spec.maxAttempts())
		.delay(Duration.ofMillis(spec.delay()))
		.maxDelay(Duration.ofMillis(spec.maxDelay()))
		.jitter(Duration.ofMillis(spec.jitter()))
		.multiplier(spec.multiplier())
		.build();

RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);

See gh-34716
See gh-34529
See gh-35058
Closes gh-35110
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbrannen sbrannen sbrannen requested changes

@artembilan artembilan artembilan requested changes

Assignees

@sbrannen sbrannen

@jhoeller jhoeller

Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Milestone
7.0.0-M6
Development

Successfully merging this pull request may close these issues.
6 participants
@fmbenhassine @spencergibb @sbrannen @artembilan @jhoeller @spring-projects-issues