lymerin commented on code in PR #7777:
URL: https://github.com/apache/shenyu/pull/7777#discussion_r4303357854
##########
shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/FixedRetryStrategy.java:
##########
@@ -56,15 +61,22 @@ public FixedRetryStrategy(final AbstractHttpClientPlugin<R>
httpClientPlugin) {
* @return Response Mono object after retry processing
*/
public Mono<R> execute(final Mono<R> response, final ServerWebExchange
exchange, final Duration duration, final int retryTimes) {
- Retry retrySpec = initFixedBackoff(exchange, retryTimes);
- Duration totalTimeout = RetryTimeoutUtils.totalTimeout(duration,
retryTimes, Duration.ofSeconds(2));
- return response.retryWhen(retrySpec)
+ Mono<R> retriedResponse;
+ if
(RetryEnum.FAILOVER.getName().equals(exchange.getAttribute(Constants.RETRY_STRATEGY)))
{
+ retriedResponse = new
DefaultRetryStrategy<>(httpClientPlugin).executeFailover(response, exchange,
duration, retryTimes, BACKOFF,
Review Comment:
Thanks for the detailed review!
You're right, I overlooked the single-upstream case. I initially approached
this with RPC-style failover in mind, focusing on switching to another upstream
after a failure, but missed the case where no alternative is available.
I agree that option 2 is more appropriate here, as it preserves the existing
fixed retry behavior without sacrificing failover when another upstream is
available.
I'll address this tomorrow and add regression tests for the single-upstream
case and `retryTimes = 0`.
Thanks again for catching this!
--
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]
