-
Notifications
You must be signed in to change notification settings - Fork 3.4k
RetryGatewayFilterFactory RetryConfig support Jitter & Timeout #3713
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.
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
Conversation
04b0156 to
605e476
Compare
|
PTAL @spencergibb |
|
Can you add documentation and tests? |
|
Also it would be good to have his PR made against the 4.1.x branch |
96eac94 to
442325f
Compare
.../springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java
Show resolved
Hide resolved
Signed-off-by: jiangyuan <joe469391363@gmail.com>
Signed-off-by: jiangyuan <joe469391363@gmail.com>
Signed-off-by: jiangyuan <joe469391363@gmail.com>
| public void retryWithBackoffTimeout() { | ||
| // backoff > timeout | ||
| testClient.get() | ||
| .uri("/retry?key=retry-with-backoff-timeout&count=3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain how hitting /retry tests timeouts?
It looks like all this endpoint does is immediately return a 500
https://github.com/spring-cloud/spring-cloud-gateway/pull/3713/files#diff-a2252bbc8510b2b95c8edffb04415464d9b1c5840d8069753726236cf771d71dR346-R360
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
route:retry_with_backoff_timeout_test config:
retries:3
firstbackoff: 100ms factor:2
timeout 100ms
The final calculation result will result in backoff>timeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But /httpbin/retry will never take 100ms it just immediately returns a 500 based on count
https://github.com/spring-cloud/spring-cloud-gateway/pull/3713/files#diff-a2252bbc8510b2b95c8edffb04415464d9b1c5840d8069753726236cf771d71dL322-L342
It seems like you would want to use /httpbin/sleep to test timeout functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes /httpbin/retry will return 500 immediately.
but i set backoff config. firstbackoff will delay 100ms . and backoff will increase exponential(factor 2).
i update the unit test to verfiy TestConfig.map key retry-with-backoff-timeout value is 2 .
this proves that after the first retry, the backoff will be greater than timeout, retry will terminate and then return the current response.
@Test
public void retryWithBackoffTimeout() {
// backoff > timeout
testClient.get()
.uri("/retry?key=retry-with-backoff-timeout&count=3")
.header(HttpHeaders.HOST, "www.retrywithbackofftimeout.org")
.exchange()
.expectStatus()
.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
assertThat(TestConfig.map.get("retry-with-backoff-timeout")).isNotNull().hasValue(2);
}
route config:
.route("retry_with_backoff_timeout_test", r -> r.host("**.retrywithbackofftimeout.org")
.filters(f -> f.prefixPath("/httpbin").retry(config -> {
config.setRetries(3)
.setBackoff(Duration.ofMillis(100), null, 2, true)
.setTimeout(Duration.ofMillis(200));
}))
.uri(uri))
...rc/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java
Show resolved
Hide resolved
Signed-off-by: joecqupt <joe469391363@gmail.com>
Signed-off-by: jiangyuan <joe469391363@gmail.com>
|
Sorry this was my fault. I did not realize this timeout was for the backoff itself. I appreciate your patience with my questioning. |
#3710