Skip to content

fix(divide,httpclient): attribute each HTTP attempt's outcome to its own circuit breaker (#8156) - #8253

Open
konlue wants to merge 1 commit into
apache:masterfrom
konlue:fix/8156-breaker-attribution
Open

konlue wants to merge 1 commit into
apache:masterfrom
konlue:fix/8156-breaker-attribution

Conversation

@konlue

@konlue konlue commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #8156.

Problem

With retryStrategy = failover the circuit breaker recorded the outcome of the whole logical request against the initial upstream's key: DividePlugin.forwardWithCircuitBreaker wrapped the entire downstream chain (failover retries and body write-back included) with recordOutcome(response, initialKey). A successful failover therefore credited the failed upstream with a success, clearing its consecutive failures — the breaker never opened and the upstream kept receiving initial traffic. The failover target recorded its own outcome (DefaultRetryStrategy.resend), but the initial attempt had no immediate failure record.

Change

  • The divide plugin publishes the initial attempt's breaker key onto the exchange (UpstreamCircuitBreaker.INITIAL_BREAKER_KEY) when retries are possible.
  • AbstractHttpClientPlugin.execute wraps the initial attempt with recordOutcome(response, initialKey): a headers-stage failure (5xx, connect errors, timeouts) is recorded immediately, even when a failover follows; a success is credited only when no failover happens. CURRENT-policy retries record per attempt as well.
  • The divide-level whole-chain recording is removed when retries are possible; with zero retries the terminal recording stays, so the no-retry behavior is unchanged.
  • Half-open probes are unchanged and mutually exclusive: probe requests forbid retries and are recorded exclusively by the probe path.
  • Compatibility: an attempt's outcome is judged at the response-header stage — where the issue's reproduction path (503, connect errors) fails. Body-stage errors are no longer attributed to the initial upstream when retries are possible.

Tests

  • InitialAttemptBreakerAttributionTest drives the real execute() with a per-host stubbed doRequest and the real breaker singleton.
  • Repeated initial failures with no failover target open the initial upstream's breaker.
  • A fails / B succeeds on every request: after the threshold, A's breaker is open even though every logical request succeeded, and B's breaker stays closed.
  • A succeeded initial attempt clears its own consecutive failures.

Verification

shenyu-plugin-httpclient: 166/166
shenyu-plugin-divide: 158/158
shenyu-plugin-base: 357/357

Checkstyle clean on the three touched modules.

This branch has not been deployed

No deployments
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] Successful HTTP failover hides repeated failures of the initial upstream from its circuit breaker

1 participant