Skip to content

fix: circuit breaker ignores straggler outcomes, unwraps budget exhaustion, rate-limits circuit.rejected - #167

Merged
lesnik512 merged 3 commits into
mainfrom
fix/breaker-correctness
Oct 17, 2026
Merged

lesnik512 merged 3 commits into
mainfrom
fix/breaker-correctness

Conversation

@lesnik512

@lesnik512 lesnik512 commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

What changed

Five fixes to CircuitBreaker, AsyncCircuitBreaker and their keyed siblings. All of them live in the shared _CircuitBreakerState, so both worlds and the keyed variants get them together.

Straggler outcomes

The breaker used to apply an outcome to whatever state the circuit was in when the response came back. A request sent while CLOSED that finished after the circuit had moved on could:

  • reopen a HALF_OPEN circuit with a single failure,
  • count toward success_threshold, so a circuit with success_threshold=3 could close after one real probe,
  • and, once it had reopened the circuit, let a second probe in while the first was still running, because the stale probe's completion freed the new probe slot.

Now every state transition advances an epoch. admit() returns a ticket made of the epoch plus a probe bit, and record(ticket, outcome, request) drops any ticket from an earlier epoch. A matching ticket also tells record which state applies (CLOSED or the HALF_OPEN probe), so it no longer reads _state to decide. on_success, on_failure and release_probe are gone; both drivers call record.

Budget exhaustion

With the breaker placed before a retry, a RetryBudgetExhaustedError reached the breaker as a foreign exception and left the circuit alone, even when every attempt had hit a 503. _CircuitBreakerState.classify(exc) now unwraps it and classifies its last_exception: a counted failure there is a counted failure, a 4xx is a success, and anything else (including None) is still foreign. Both drivers share the one classify.

circuit.rejected

Rejections used to log one WARNING per request, from inside the sync breaker's lock. An open circuit under load turned that into a log flood, and each rejecting thread did logging I/O while holding the lock every other request needs. Now admit() decides under the lock whether this rejection gets an event, and the driver emits it after releasing the lock. It is emitted at most once per second per circuit, timed with the breaker's injected clock, and carries a new suppressed attribute: how many rejections were not emitted since the previous one. The event name is unchanged.

Events are emitted outside the lock

circuit.opened, circuit.half_open and circuit.closed were still emitted from inside the state methods, which the sync driver runs under a non-reentrant threading.Lock. The keyed sync breaker shares one lock across all keys. A logging handler that ships records through a client using the same breaker therefore deadlocked: the handler's request waited for the lock its own thread was holding. With the keyed breaker this happened even when the handler called a different host.

Transitions now queue their events on the state object instead of emitting them. record() returns the queued events. The sync driver takes them while it still holds the lock and emits them after releasing it, so it never reads a shared queue unlocked. admit() hands over its events through the exceptions it already raises on the rare paths: _RejectionError for a rejection, and a new _HalfOpened for the OPEN to HALF_OPEN transition. A CLOSED admission therefore costs nothing extra. The async driver does the same, so the two stay at parity. A thread emits its own events in the order its transitions happened. When nothing transitions, the hot path allocates nothing and only checks record()'s return value for None.

Rejections are counted per outage

An outage runs from CLOSED to OPEN until the circuit is CLOSED again; a probe failure that reopens it stays in the same outage. When an outage starts, the rejection throttle and its counters reset, so the first rejection of every outage is emitted even if the previous outage's last circuit.rejected was under a second ago. suppressed only counts rejections from the current outage. circuit.closed gains a rejected attribute, the outage's total rejection count whether emitted or not. That reports the tail that a "count the suppressed ones on the next emission" scheme loses when no further rejection comes along to carry it.

Behaviour changes for the 0.22.0 notes

Not breaking; the public API and event names are unchanged.

  • A request that was in flight when its circuit changed state no longer affects the circuit.
  • A RetryBudgetExhaustedError now counts against the breaker according to its last_exception. A breaker sitting outside a starved retry can now open where it used to stay closed.
  • circuit.rejected fires at most once per second per circuit, and always for the first rejection of an outage. It has a new suppressed attribute. Anyone counting rejections from log lines should sum 1 + suppressed, or read rejected on circuit.closed, the outage's total.
  • circuit.closed has a new rejected attribute.
  • Breaker events are emitted after the sync lock is released. Logging handlers no longer run while the breaker's lock is held, so a handler may send requests through the same breaker.

Performance

The hot path takes no new locks. A paired in-process A/B (old and new module loaded side by side, 40 alternating rounds of 100k calls each) of a CLOSED sync CircuitBreaker around a no-op next came out at 290 ns (old) vs 285 ns (new) per call, a paired median of -7 ns and -3 ns on two runs. That is within noise, and under the ~10 ns budget.

With the deferred events added, the same A/B on a heavily loaded machine (load average 11 to 20) came out at +10.0 ns and +9.9 ns against the previous head of this branch, and +3.2 ns and +4.9 ns against main. The first version checked the ticket's probe bit after every admission and cost about 25 ns more. Moving the half-open hand-off into an exception on the rare path removed that.

Tests

Each fix was test-first; every new test failed on main for the reason it names.

  • test_stale_closed_era_failure_does_not_reopen_half_open_circuit (was OPEN, expected HALF_OPEN), test_stale_closed_era_successes_do_not_close_half_open_circuit (was CLOSED with success_threshold=3), test_stragglers_never_let_a_second_probe_in (2 probes in flight). Keyed sync and async: test_stale_closed_era_failure_does_not_reopen_a_half_open_circuit.

  • test_outcomes_only_move_the_state_they_were_admitted_under in test_circuit_breaker_props.py starts and finishes calls in random order and checks that a call finishing after a state change leaves the state alone, and that probes never overlap. It failed on main in every run tried.

  • 5xx and NetworkError behind an exhausted budget now open the circuit; a 4xx behind an exhausted budget resets the failure streak; a foreign last_exception still leaves the circuit alone.

  • test_rejected_event_is_emitted_at_most_once_per_second (sync and async) checks suppressed == [0, 4] across a clock-driven sequence; test_rejected_event_is_emitted_outside_the_lock (sync, plain and keyed) checks the lock is free when the record is handled.

  • test_event_handler_may_send_through_the_same_breaker (sync) and test_event_handler_may_send_to_another_host_through_the_same_breaker (keyed sync) run the client call on a thread with a 5 s join timeout, so a regression fails instead of hanging the suite. Both failed on the previous head because the thread was still alive. The handlers override Handler.handle so a deadlocked thread cannot hang logging.shutdown at interpreter exit. test_every_event_is_emitted_outside_the_lock failed with [True, False, True, True] for opened, rejected, half_open and closed.

  • Sync and async: test_first_rejection_of_a_new_outage_is_emitted (was [0], expected [0, 0]), test_closed_event_reports_every_rejection_of_the_outage (no rejected attribute), and test_suppressed_never_counts_a_previous_outage (was [0, 2], expected [0, 0]). test_events_of_one_call_arrive_in_transition_order guards ordering.

just lint, just lint-ci and just test pass: 1038 tests, 100% coverage on Python 3.14 and 3.11.

Docs

docs/resilience.md (breaker States, Failure classification and Observability, including what an outage is and that events are emitted outside the lock; Composition untouched), the circuit.rejected sentence in docs/observability.md, and the "Counted failure" entry in CONTEXT.md.

…stion, rate-limits circuit.rejected

Outcomes now carry an epoch ticket from admission, so a request that finishes
after its circuit changed state no longer reopens, closes or frees the probe
slot. A RetryBudgetExhaustedError is classified by its last_exception.
circuit.rejected is emitted outside the sync lock, at most once per second per
circuit, with a suppressed count.
On 3.11, awaiting a task that ended with an exception drops coverage line
events in the awaiting frame until its next suspension. The harness tasks
now swallow the breaker's error themselves.
…s per outage

Breaker transitions queue their events and the drivers emit them after
releasing the sync lock, so a logging handler that sends a request through
the same breaker no longer deadlocks. Rejection accounting restarts with
each outage: the first rejection is always emitted and circuit.closed
reports the outage's total in a new rejected attribute.
@lesnik512
lesnik512 merged commit 80bee92 into main Oct 17, 2026
13 checks passed
@lesnik512
lesnik512 deleted the fix/breaker-correctness branch October 17, 2026 09:12
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.

1 participant