Repository navigation
fix: circuit breaker ignores straggler outcomes, unwraps budget exhaustion, rate-limits circuit.rejected - #167
Merged
Merged
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Five fixes to
CircuitBreaker,AsyncCircuitBreakerand 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:
success_threshold, so a circuit withsuccess_threshold=3could close after one real probe,Now every state transition advances an epoch.
admit()returns a ticket made of the epoch plus a probe bit, andrecord(ticket, outcome, request)drops any ticket from an earlier epoch. A matching ticket also tellsrecordwhich state applies (CLOSED or the HALF_OPEN probe), so it no longer reads_stateto decide.on_success,on_failureandrelease_probeare gone; both drivers callrecord.Budget exhaustion
With the breaker placed before a retry, a
RetryBudgetExhaustedErrorreached 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 itslast_exception: a counted failure there is a counted failure, a 4xx is a success, and anything else (includingNone) is still foreign. Both drivers share the oneclassify.circuit.rejectedRejections 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 newsuppressedattribute: 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_openandcircuit.closedwere still emitted from inside the state methods, which the sync driver runs under a non-reentrantthreading.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:_RejectionErrorfor a rejection, and a new_HalfOpenedfor 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 checksrecord()'s return value forNone.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.rejectedwas under a second ago.suppressedonly counts rejections from the current outage.circuit.closedgains arejectedattribute, 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.
RetryBudgetExhaustedErrornow counts against the breaker according to itslast_exception. A breaker sitting outside a starved retry can now open where it used to stay closed.circuit.rejectedfires at most once per second per circuit, and always for the first rejection of an outage. It has a newsuppressedattribute. Anyone counting rejections from log lines should sum1 + suppressed, or readrejectedoncircuit.closed, the outage's total.circuit.closedhas a newrejectedattribute.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
CircuitBreakeraround a no-opnextcame 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
mainfor 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 withsuccess_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_underintest_circuit_breaker_props.pystarts 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 onmainin every run tried.5xx and
NetworkErrorbehind an exhausted budget now open the circuit; a 4xx behind an exhausted budget resets the failure streak; a foreignlast_exceptionstill leaves the circuit alone.test_rejected_event_is_emitted_at_most_once_per_second(sync and async) checkssuppressed == [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) andtest_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 overrideHandler.handleso a deadlocked thread cannot hanglogging.shutdownat interpreter exit.test_every_event_is_emitted_outside_the_lockfailed 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(norejectedattribute), andtest_suppressed_never_counts_a_previous_outage(was[0, 2], expected[0, 0]).test_events_of_one_call_arrive_in_transition_orderguards ordering.just lint,just lint-ciandjust testpass: 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), thecircuit.rejectedsentence indocs/observability.md, and the "Counted failure" entry inCONTEXT.md.