Aias00 commented on code in PR #7266:
URL: https://github.com/apache/shenyu/pull/7266#discussion_r4110185299
##########
shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/DefaultRetryStrategy.java:
##########
@@ -75,7 +75,9 @@ public Mono<R> execute(final Mono<R> clientResponse, final
ServerWebExchange exc
.onRetryExhaustedThrow((retryBackoffSpecErr, retrySignal)
-> {
throw new ShenyuTimeoutException("Request timeout, the
maximum number of retry times has been exceeded");
});
+ Duration totalTimeout = RetryTimeoutUtils.totalTimeout(duration,
retryTimes, Duration.ofSeconds(20));
return clientResponse.retryWhen(retryBackoffSpec)
+ .timeout(totalTimeout, Mono.error(() -> new
TimeoutException("Retry sequence took longer than timeout: " + totalTimeout)))
Review Comment:
Non-blocking, please write into the docs/release note: this is the first
aggregate cap the default strategy has ever had (before this change it only had
the per-attempt timeout applied in AbstractHttpClientPlugin.java:91), and it
noticeably widens the worst-case latency.
With the defaults - `HTTP_TIME_OUT` = 3000 ms
(AbstractHttpClientPlugin.java:80) and three retries - the budget here is `4 x
3s + 3 x 20s = 72s`. Previously a hanging upstream stayed open until the client
gave up; now it terminates after 72s. Bounded is better, I just want the number
to be findable.
Also note the two failure modes map to different statuses below: blowing
this budget becomes `504 GATEWAY_TIMEOUT`, exhausting retries becomes
`ShenyuTimeoutException -> 408 REQUEST_TIMEOUT`. The message text `Retry
sequence took longer than timeout: ...` is what separates them in the logs -
worth quoting in the docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
