Skip to content

Add MVC retry backoff support#4225

Open
goutamadwant wants to merge 1 commit into
spring-cloud:mainfrom
goutamadwant:GH-4141
Open

Add MVC retry backoff support#4225
goutamadwant wants to merge 1 commit into
spring-cloud:mainfrom
goutamadwant:GH-4141

Conversation

@goutamadwant

Copy link
Copy Markdown

Summary

  • Add backoff support to MVC RetryConfig with firstBackoff, maxBackoff, factor, and basedOnPreviousValue
  • Apply configured backoff in both MVC retry implementations: GatewayRetryFilterFunctions and FrameworkRetryFilterFunctions
  • Document YAML and Java DSL usage and add regression coverage for both retry paths plus nested config binding

Fixes #4141

Testing

  • ./mvnw -pl spring-cloud-gateway-server-webmvc -Dtest=RetryFilterFunctionBackoffTests,RetryFilterFunctionSelectionLogicTests,RetryFilterFunctionNoSpringRetrySelectionTests test
  • ./mvnw -pl spring-cloud-gateway-server-webmvc -DskipTests verify

Fixes spring-cloudgh-4141

Signed-off-by: Goutam Adwant <workwithgoutam@gmail.com>
@ryanjbaxter

Copy link
Copy Markdown
Contributor

@spencergibb I wonder if we should just move to using Framework retry. However just glancing at the code it looks like it might involve breaking changes due to some interfaces we implement.

@spencergibb

Copy link
Copy Markdown
Member

I think spring-retry is around until the next major

* `backoff`: The configured exponential backoff for the retries.
Retries are performed after a backoff interval of `firstBackoff * (factor ^ n)`, where `n` is the iteration.
If `maxBackoff` is configured, the maximum backoff applied is limited to `maxBackoff`.
If `basedOnPreviousValue` is true, the backoff is calculated by using `prevBackoff * factor`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

basedOnPreviousValue does not appear to be used in the MVC implementation...I am not sure it makes sense fir MVC

}

public void validate() {
Objects.requireNonNull(this.firstBackoff, "firstBackoff must be present");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It probably makes sense to do some additional validation here

Objects.requireNonNull(this.firstBackoff, "firstBackoff must be present");
    Assert.isTrue(this.firstBackoff.toMillis() >= 1, "firstBackoff must be >= 1ms");
    Assert.isTrue(this.factor > 1, "factor must be greater than 1");
    if (this.maxBackoff != null) {
        Assert.isTrue(this.maxBackoff.compareTo(this.firstBackoff) > 0,
                "maxBackoff must be greater than firstBackoff");
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add backoff support to Retry filter in Spring Cloud Gateway MVC (server-mvc)

4 participants