fix: fall back to OPEN when half-open probe is stuck and update retry timestamp after rollback - #3643
Conversation
… timestamp after rollback
oss-sentinel-ai
left a comment
There was a problem hiding this comment.
Summary
This PR fixes two stuck-state scenarios in the HALF_OPEN circuit breaker (fixes #1638): (1) the probe-block-rollback loop where nextRetryTimestamp was never updated on HALF_OPEN→OPEN rollback, causing immediate re-probing during outages; (2) a hung probe that never completes and leaves the breaker stuck in HALF_OPEN forever.
The fix looks correct: CAS guards prevent double-fallback races, the new volatile probeStartTimestamp field is safely published (the probeStartTimestamp > 0 sentinel handles the brief visibility gap after the CAS), and reusing recoveryTimeoutMs as the probe deadline avoids a new config parameter while staying semantically appropriate. Both new tests are deterministic (mocked time, no real sleeps) and directly exercise the described scenarios. Hot-path overhead is negligible — the stuck-probe check only runs while HALF_OPEN.
LGTM, approving.
Automated review by github-manager-bot
Fixes #1638
Background
Issue #1638 reports that the circuit breaker can get stuck in HALF_OPEN. #1645 was merged as a temporary workaround: when the probing request is blocked by upcoming rules, a terminate handler rolls the state back from HALF_OPEN to OPEN. However, the workaround left two gaps, which this PR closes.
Problem 1: rollback does not update the retry timestamp
In
AbstractCircuitBreaker#fromOpenToHalfOpen, the terminate handler rolls back HALF_OPEN to OPEN when the probe is blocked, butnextRetryTimestampis not updated. SinceretryTimeoutArrived()is already true at that point, the very next request immediately triggers a new probe. The probe gets blocked again, rolls back again, and the cycle repeats — during the whole outage window every request probes the downstream, effectively disabling the circuit breaker.Fix: update
nextRetryTimestampwhen the handler rolls the state back, so probing waits for the next recovery window.Problem 2: a stuck probe request keeps the breaker in HALF_OPEN forever
A probe that passes (not blocked) but never completes — e.g. a slow/hung request — never triggers the terminate handler, so the breaker stays in HALF_OPEN and rejects all subsequent requests forever. This matches the report in #1638 (comment) (2024-12, still reproducible).
Fix: record the probe start time when transitioning OPEN → HALF_OPEN; in
tryPass, if the probe has been stuck longer thanrecoveryTimeoutMs, fall back to OPEN and update the retry timestamp so probing resumes in the next recovery window.Changes
AbstractCircuitBreaker: addprobeStartTimestamp, addprobeTimedOut(), update the rollback path in the terminate handler, and handle the stuck-probe case intryPass.ExceptionCircuitBreakerTest: addtestHalfOpenRollbackUpdatesRetryTimestampandtestHalfOpenProbeTimeoutFallback.Verification
All 8 test classes under the
degradepackage pass (CircuitBreakingIntegrationTest, DefaultCircuitBreakerRuleManagerTest, DefaultCircuitBreakerSlotTest, DegradePartialIntegrationTest, DegradeRuleManagerTest, DegradeRuleTest, ExceptionCircuitBreakerTest, ResponseTimeCircuitBreakerTest).