-
Notifications
You must be signed in to change notification settings - Fork 4
feat: let a breaker recover when its probes never reach the dependency #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a746799
226facb
f077b7e
28839c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,75 @@ stateDiagram-v2 | |
| thresholds as `CLOSED`: rates below the thresholds close it, at or above | ||
| re-open it. Calls beyond the probe caps are rejected while the round runs. | ||
|
|
||
| ## When a probe cannot reach the dependency | ||
|
|
||
| A probe asks one question: has the dependency recovered? Some failures cannot | ||
| answer it, because the call never left the process — no free connection in the | ||
| local pool, no bulkhead permit. Counting one as a probe failure re-opens the | ||
| breaker on evidence it does not have, and if the local cause outlives the outage | ||
| that opened the breaker, every round fails the same way and the breaker never | ||
| closes again. | ||
|
|
||
| Integrations mark those failures for you. The httpx and httpx2 transports treat | ||
| `PoolTimeout` this way: in `HALF_OPEN` the probe hands its slot back without a | ||
| verdict, and the round continues. `CLOSED` is untouched — there an exhausted | ||
| pool usually *is* the dependency holding connections open, and shedding load is | ||
| exactly what should happen. | ||
|
|
||
| A round still has to end. Once as many probes have come back inconclusive as the | ||
| round permits, the breaker re-opens: nothing was learned, so waiting is the only | ||
| honest move left. | ||
|
|
||
| Other guards pass their own set: | ||
|
|
||
| ```python | ||
| from interlock import CircuitBreaker, Registry | ||
|
|
||
|
|
||
| class NoLocalSlot(Exception): | ||
| """Raised by the guard's own pool when it has no permit to give out.""" | ||
|
|
||
|
|
||
| breaker = CircuitBreaker(name='payments', unreachable_exceptions=(NoLocalSlot,)) | ||
| registry = Registry(unreachable_exceptions=(NoLocalSlot,)) | ||
| ``` | ||
|
|
||
| ## Backing off between probe rounds | ||
|
|
||
| `wait_duration_in_open` is constant by default: a breaker that cannot recover | ||
| retries at exactly the same rate forever, hammering a dependency that is already | ||
| in trouble. Set `wait_duration_backoff_multiplier` above `1.0` to lengthen the | ||
| wait after each consecutive failed round, and `wait_duration_in_open_max` to cap | ||
| it. A round that passes resets both. | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| ```python | ||
| from interlock import CircuitBreaker, Config | ||
|
|
||
| breaker = CircuitBreaker( | ||
| name='payments', | ||
| config=Config( | ||
| wait_duration_in_open=5.0, | ||
| wait_duration_backoff_multiplier=2.0, | ||
| wait_duration_in_open_max=120.0, | ||
| ), | ||
| ) | ||
| # Failed rounds wait 5s, then 10s, 20s, 40s… up to 120s. | ||
| ``` | ||
|
|
||
| The growing interval is also a signal in its own right: a breaker waiting out a | ||
| blip looks nothing like one that has failed ten rounds in a row. | ||
|
|
||
| The backoff is local. Under a shared [storage](../integrations/redis.md) the | ||
| coordinated lane reopens on `wait_duration_in_open` and keeps no failed-round | ||
| count, so a coordinated breaker retries on the base wait no matter how many | ||
| rounds have failed. | ||
|
|
||
| Growth stops after 64 consecutive failed rounds. Any sane multiplier has long | ||
| since passed `wait_duration_in_open_max` by then, and an unbounded exponent | ||
| would eventually overflow to an infinite wait that never elapses — leaving the | ||
|
Comment on lines
+92
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Correct the cap explanation.
As per path instructions, user-facing documentation must match the current public API and behavior. 🤖 Prompt for AI AgentsSource: Path instructions |
||
| breaker open for good, which is precisely the failure this release set out to | ||
| remove. | ||
|
|
||
| ## Proactive transition (`auto_transition`) | ||
|
|
||
| By default the `OPEN → HALF_OPEN` move is **lazy**: it happens on the first call | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,36 @@ | |
| _MANUAL_STATES = frozenset({State.FORCED_OPEN, State.DISABLED, State.METRICS_ONLY}) | ||
|
|
||
|
|
||
| def validate_unreachable_exceptions( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Make the shared validator private. Rename As per coding guidelines, “Expose the public API through the package 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| types: tuple[type[Exception], ...], | ||
| ) -> tuple[type[Exception], ...]: | ||
| """Reject entries that could never be classified, before any call runs. | ||
|
|
||
| The annotation is a promise, not a guarantee: an untyped caller can still | ||
| pass a string. Left to ``_settle``, the ``isinstance`` would raise on the | ||
| first real failure — masking the protected exception and stranding the probe | ||
| slot it should have returned. | ||
|
|
||
| The container is checked too: ``isinstance`` rejects a list of types with a | ||
| ``TypeError`` of its own, so a caller who passes one would hit it on the | ||
| first real failure rather than here. | ||
|
|
||
| Raises: | ||
| TypeError: If the container is not a tuple, or an entry is not an | ||
| ``Exception`` subclass. | ||
| """ | ||
| if not isinstance(cast('object', types), tuple): | ||
| raise TypeError(f'unreachable_exceptions must be a tuple, got: {type(types).__name__}') | ||
|
|
||
| for entry in cast('tuple[object, ...]', types): | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| if not (isinstance(entry, type) and issubclass(entry, Exception)): | ||
| raise TypeError( | ||
| f'unreachable_exceptions must hold Exception subclasses, got: {entry!r}' | ||
| ) | ||
|
|
||
| return types | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class Admission: | ||
| """What ``_admit`` granted: the era it happened in, and probe provenance.""" | ||
|
|
@@ -91,11 +121,13 @@ def __init__( | |
| classifier: FailureClassifier | None = None, | ||
| listener: CoreEventListener | StorageEventListener | None = None, | ||
| storage: Storage | AsyncStorage | None = None, | ||
| unreachable_exceptions: tuple[type[Exception], ...] = (), | ||
| ) -> None: | ||
| self._name = name | ||
| self._config = config | ||
| self._clock = clock | ||
| self._classifier = classifier if classifier is not None else DefaultFailureClassifier() | ||
| self._unreachable_exceptions = validate_unreachable_exceptions(unreachable_exceptions) | ||
| self._listener = listener | ||
| self._machine = StateMachine( | ||
| config=config, | ||
|
|
@@ -464,11 +496,27 @@ def _settle( | |
| failure = self._classifier.is_failure(result=result, exception=exception) | ||
| slow = duration >= self._config.slow_call_duration_threshold | ||
| outcome = _OUTCOME_BY_FLAGS[failure, slow] | ||
| # Only HALF_OPEN cares; the machine owns that check because only it knows | ||
| # its own state. Outside a probe the same exception stays an ordinary | ||
| # failure — shedding load is the point. The ``is not None`` guard keeps | ||
| # the common path — a call that returned — off ``isinstance``. | ||
| # | ||
| # A *leased* probe is the exception: it holds a remote grant that only an | ||
| # outcome returns, and ``Storage`` has no way to hand one back unspent, so | ||
| # dropping the verdict would strand the shared budget until its TTL. | ||
| # ``admission.probe`` marks exactly those, which also keeps the local | ||
| # fallback covered — when storage degrades the admission comes from the | ||
| # local machine, with no lease to strand. | ||
| unreachable = ( | ||
| not admission.probe | ||
| and exception is not None | ||
| and isinstance(exception, self._unreachable_exceptions) | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| with self._lock: | ||
| effective_before = self._effective_state_locked() | ||
| before = self._machine.state | ||
| self._machine.record(outcome, generation=admission.generation) | ||
| self._machine.record(outcome, generation=admission.generation, unreachable=unreachable) | ||
| after = self._machine.state | ||
| effective_after = self._effective_state_locked() | ||
| if failure and exception is not None: | ||
|
|
@@ -590,11 +638,21 @@ def _schedule_auto_transition(self) -> None: | |
| pending one never blocks interpreter shutdown, and it is cancelled the | ||
| moment the breaker leaves ``OPEN`` by any path (a real probe, ``reset``, | ||
| ``force_open`` or the timer itself). | ||
|
|
||
| The delay comes from ``retry_after()`` rather than the configured wait: | ||
| with a backoff in play they differ, and a timer armed for the base wait | ||
| would fire early, be refused, and leave nothing scheduled for the rest of | ||
| the interval. | ||
| """ | ||
| if not self._config.auto_transition: | ||
| return | ||
|
|
||
| timer = threading.Timer(self._config.wait_duration_in_open, self._fire_auto_transition) | ||
| with self._lock: | ||
| delay = self._machine.retry_after() | ||
| if delay is None: | ||
| return # left OPEN before the timer could be armed | ||
|
|
||
| timer = threading.Timer(delay, self._fire_auto_transition) | ||
| timer.daemon = True | ||
| with self._timer_lock: | ||
| if self._closed: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.