Skip to content

[ISSUE #8230] Preserve failover with exponential retry backoff. - #8231

Merged
Aias00 merged 1 commit into
apache:masterfrom
qiaolezi001:fix/exponential-http-failover
Oct 30, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
qiaolezi001:fix/exponential-http-failover

Conversation

@qiaolezi001

Copy link
Copy Markdown
Contributor

Fixes #8230.

With retryStrategy=failover and retryBackOffSpec=exponential, a GET connection failure retries the original URI and an upstream 503 receives no retry. A healthy standby is never contacted. Both Netty and WebClient reproduce this with ordinary Divide rules.

Changes

  • Apply exponential backoff to the existing failover reselection path. Retain Reactor's 500 ms initial delay, 5 s cap, 50% jitter, retry count and aggregate timeout budget.
  • Accept the clients' explicit 5xx failover signal only for GET failover. Keep CURRENT behavior and the restrictions on unsafe methods and permanent errors.
  • Add a package-private retry-policy overload with per-subscription exclusion state. Reuse upstream exclusion, topology refresh, circuit-breaker and load-balancer accounting, per-attempt timeouts and terminal error mapping. The default/fixed retry loop retains its existing behavior.
  • Add real HTTP regressions for both clients and virtual-time checks for schedule/jitter bounds, retry exhaustion, topology exhaustion, cancellation, timeout cleanup and resubscription.

No new configuration or dependencies are required.

Prior work

This completes the exponential routing correction proposed by @SouthwestAsiaFloat in closed, unmerged #8088 and highlighted in @Aias00's review. The merged #8076 did not include that part. #7777 fixed the corresponding fixed-backoff case. Canary validation is outside this change's scope.

Validation

  • Before the production fix: the same 58-case regression suite reported 10 failures and 4 errors, no skips. All four real-client errors were exponential failover cases; fixed-backoff controls passed.
  • After the fix: 58 tests, 0 failures, 0 errors, 0 skips.
  • The scoped clean install below passed across 17 reactor modules: 1,429 tests, 0 failures, 0 errors, 5 existing skips in shenyu-common. HTTP client: 153 tests, no skips; Divide: 155 tests, no skips; HTTP starter: 21 tests, no skips.
  • Checkstyle and Apache RAT passed in all 17 modules; git diff --check passed. Tests and checks were not disabled.
./mvnw -B -ntp \
  -pl shenyu-plugin/shenyu-plugin-httpclient,shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide,shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-httpclient \
  -am clean install -Dmaven.javadoc.skip=true

JDK: Temurin 17.0.21+9; base: a87a3613f5ef1694f0624911002a821ab3587a45. Maven used a workspace-local dependency cache and temporary directory.

The loopback tests use real ephemeral servers and consume the full responses. Reactor Netty's implicit connection retry is disabled, so counts measure ShenYu attempts. Both clients contact the failed primary once and the healthy standby once for 503 and disconnect cases. Controls cover CURRENT recovery, zero retries and 4xx forwarding. Full-repository install and Docker E2E were not run.

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

The last item is unchecked because validation used the scoped reactor command above.

Fixes apache#8230. Completes the exponential routing correction discussed in apache#8088 with real-client and virtual-time regression coverage.
@Aias00

Aias00 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

hi, pls add my wechat: aias00

@Aias00 Aias00 left a comment

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.

Reviewed commit 3df2cac. No blocking issues found.

The change preserves the configured failover routing policy while applying exponential backoff, reuses existing upstream reselection, and initializes exclusion state per subscription. CURRENT behavior, GET-only eligibility, backoff/jitter and retry budgets are preserved.

Independent validation:

  • On the unmodified base, the real-HTTP regression suite reproduces all four exponential failover errors across Netty/WebClient and HTTP 503/disconnect; the other ten control cases pass. All 14 cases pass on this head.
  • The scoped 17-module reactor reports 1,429 tests, zero failures/errors and four skips (Amazon Corretto 21; project target Java 17). This includes 153 HTTP-client, 155 Divide and 21 HTTP-starter tests.
  • Checkstyle, the separately executed Apache RAT check and git diff --check pass.

Repository workflows currently report action_required for this first-time contribution. Please authorize the workflows and wait for successful CI before merging.

@Aias00
Aias00 merged commit bbcb341 into apache:master Oct 30, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Exponential HTTP backoff bypasses the configured failover retry strategy

2 participants