[konlue]

Fixes #8158.

Problem

On HTTP failover the initial target's leastActive reservation is held until the whole logical request finishes, while the failover target's reservation is released as soon as its response headers arrive. While the failover target's body is still streaming, leastActive therefore sees the failed node as busy and the serving node as idle — the opposite of the actual workload.

Change

• Add an attempt-ownership object on the exchange (UpstreamAttemptOwner). It names the upstream that currently owns the request's reservation and releases only that upstream, using identity CAS so an immediate resend cannot release a newer target.
• DividePlugin installs the owner for leastActive / P2C after the reservation is taken and releases the current owner when the response body completes, errors or is cancelled. P2C keeps using responseTrigger (decrement + response-time accounting); leastActive decrements.
• DefaultRetryStrategy binds releases to each attempt:
  • The initial attempt releases itself on error/cancel, including during the configured backoff.
  • A resend releases the previous failed attempt, transfers ownership to the selected retry target, and releases the retry target only on its own error/cancel; a successful retry stays owned until the response body finishes.
  • Synchronous failures while constructing/starting a retry release that selected target.
  • Callers without a Divide owner keep the previous retry accounting behavior, so non-Divide usage does not leak.
• The former debug artifacts and temporary error strings were removed; the original CANNOT_FIND_HEALTHY_UPSTREAM_URL_AFTER_FAILOVER diagnostic is restored.

Tests

• Retry tests cover A-fail/B-fail/C-success with per-attempt baselines and final ownership, backoff release before the resend, immediate and synchronous retry failures, ownerless fallback, and the existing P2C/leastActive success/cancel cases.
• Divide tests keep the reservation/complete/cancel coverage; the owner is installed only when an attempt is actually reserved.
• Red/green: the new tests fail on the previous logic (A held through backoff, B released at headers, owner not transferred) and pass after the change.

Verification

shenyu-plugin-httpclient: 128/128
shenyu-plugin-divide: 155/155
shenyu-plugin-base: 334/334
shenyu-common: 626 (4 skipped)
loadbalancer: 83

Checkstyle clean on the affected modules. Only the failover/accounting path changed; ordinary forwarding without retry is untouched.

[Aias00]

Reviewed initial-attempt release, delayed failover, attempt ownership transfer, successful response-body lifetime, cancellation and synchronous request-construction failures. No actionable finding. Focused HTTP retry and Divide reactor tests: 59 tests passed.

Checkstyle and a separate Apache RAT check passed locally. This review covers source and the stated targeted verification; full Docker integration/E2E suites were not run locally.

[konlue]

@Aias00 I split the #8156 fix out of this PR: it now lives in the standalone PR #8253, so this branch again contains only the #8158 fix (one commit, fe58aa2). Nothing in the #8158 change itself moved; the branch was rebased to drop the second commit, hence the force push (which is also why the earlier approval shows as stale). Both modules pass clean again (httpclient 169, divide 158, checkstyle clean). Could you take a fresh look when convenient?

[Aias00]

Stream Agent analysis

FINAL review of apache/shenyu PR #8251 (head fe58aa2): fix(divide) bind failover inflight accounting to each attempt's response lifetime. All 14 partition reviews passed with no blocking findings; global complete_diff true; CI passed with 64 checks.

Evidence

• Attached cross-file review: all_parts_passed=true, complete_diff=true, global_complete_diff=true
• CI: check_count=64, complete=true, passed=true
• 14 child partition reviews (part-0..part-13) all verdict PASS with empty blocking_findings

Recommended next steps

• Confirm non-Divide plugins (if any) that use DefaultRetryStrategy still correctly account inflight via the new ownership model
• Optionally extend test coverage with a chaos/fault-injection integration test for failover inflight leaks

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: a58e9162-df7b-420c-9c4d-3168bbc0c57a · Commit: fe58aa28455c6f280b3c3fd87f6b35a22b80f285

Review verdict: PASS

FINAL scope: assessed cross-file correctness from all child summaries. No child returned NEEDS_CHANGES or INCONCLUSIVE. all_parts_passed=true, complete_diff=true, global_complete_diff=true, CI passed=true. The supplied diff segments cover new UpstreamAttemptOwner.java, import additions, DefaultRetryStrategy failover release logic, DividePlugin ownership tracking, and corresponding tests. Reviews indicate internally consistent accounting: reservations released on attempt terminal events, P2C/LEAST_ACTIVE increments balanced by owner release. No anchored HIGH/CRITICAL defects. Limitation: no local tests executed; reliance on CI and partition reviews. Verdict PASS is justified under the given rules.

Publication: APPROVE (EXPLICIT_PASS)

Review details

Synthesis of partition reviews shows the new UpstreamAttemptOwner class and modifications in DefaultRetryStrategy and DividePlugin consistently bind inflight reservation to attempt response lifetime, releasing on terminal events. All supplied diff segments (complete) were reviewed and no HIGH/CRITICAL defects were found. CI is green. Recommendation: maintainers may proceed with merge after their own checklist; this review does not execute tests or approve.

[Aias00]

Stream Agent analysis

FINAL review of apache/shenyu PR #8251 (head fe58aa2): fix(divide) bind failover inflight accounting to each attempt's response lifetime. All 14 partition reviews passed with no blocking findings; global complete_diff true; CI passed with 64 checks.

Evidence

• Attached cross-file review: all_parts_passed=true, complete_diff=true, global_complete_diff=true
• CI: check_count=64, complete=true, passed=true
• 14 child partition reviews (part-0..part-13) all verdict PASS with empty blocking_findings

Recommended next steps

• Confirm non-Divide plugins (if any) that use DefaultRetryStrategy still correctly account inflight via the new ownership model
• Optionally extend test coverage with a chaos/fault-injection integration test for failover inflight leaks

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: a58e9162-df7b-420c-9c4d-3168bbc0c57a · Commit: fe58aa28455c6f280b3c3fd87f6b35a22b80f285

Review verdict: PASS

FINAL scope: assessed cross-file correctness from all child summaries. No child returned NEEDS_CHANGES or INCONCLUSIVE. all_parts_passed=true, complete_diff=true, global_complete_diff=true, CI passed=true. The supplied diff segments cover new UpstreamAttemptOwner.java, import additions, DefaultRetryStrategy failover release logic, DividePlugin ownership tracking, and corresponding tests. Reviews indicate internally consistent accounting: reservations released on attempt terminal events, P2C/LEAST_ACTIVE increments balanced by owner release. No anchored HIGH/CRITICAL defects. Limitation: no local tests executed; reliance on CI and partition reviews. Verdict PASS is justified under the given rules.

Publication: COMMENT (MERGEABILITY_NOT_CONFIRMED)

Review details

Synthesis of partition reviews shows the new UpstreamAttemptOwner class and modifications in DefaultRetryStrategy and DividePlugin consistently bind inflight reservation to attempt response lifetime, releasing on terminal events. All supplied diff segments (complete) were reviewed and no HIGH/CRITICAL defects were found. CI is green. Recommendation: maintainers may proceed with merge after their own checklist; this review does not execute tests or approve.