Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### Fixed

- **A backoff the coordinated lane cannot honour is now refused instead of
ignored.** Reopening a breaker with a shared `Storage` is the backend's
decision, taken from `wait_duration_in_open` and its own clock; no
failed-round count crosses the wire, because `SharedState` carries mechanism
rather than policy and has no field for one. A
`wait_duration_backoff_multiplier` above `1.0` alongside a storage was
therefore read, validated and then silently dropped — the option looked
enabled while every round waited exactly as long as the last. It now raises
`ValueError` at construction, on `CircuitBreaker`, `Registry` and the
per-breaker `Registry.get(config=...)` override alike.

- **A probe that never reached the dependency no longer decides the round.** A
`HALF_OPEN` probe asks one question — has the dependency recovered? — and
every failure was taken as its answer, including failures that never left the
Expand Down
2 changes: 1 addition & 1 deletion docs/guides/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ config = Config(
| `permitted_calls_in_half_open` | `10` | Probe calls allowed while `HALF_OPEN`. |
| `max_concurrent_probes` | `1` | Cap on **simultaneous** probes in `HALF_OPEN`. Must be in `[1, permitted_calls_in_half_open]`. |
| `wait_duration_in_open` | `60.0` | Seconds to stay `OPEN` before the first probe is allowed. |
| `wait_duration_backoff_multiplier` | `1.0` | Multiplies the open wait after each consecutive failed probe round. `1.0` keeps it constant. Must be `>= 1`. |
| `wait_duration_backoff_multiplier` | `1.0` | Multiplies the open wait after each consecutive failed probe round. `1.0` keeps it constant. Must be `>= 1`, and must stay `1.0` on a breaker with a shared storage. |
| `wait_duration_in_open_max` | `None` | Ceiling for the backed-off wait, in seconds. `None` leaves it uncapped; when set, must be `>= wait_duration_in_open`. |
| `auto_transition` | `False` | When `True`, a timer moves the breaker `OPEN → HALF_OPEN` once the wait elapses, instead of waiting for the next call. See [States](states.md#proactive-transition-auto_transition). |
| `window_type` | `COUNT_BASED` | `COUNT_BASED` or `TIME_BASED`. |
Expand Down
11 changes: 7 additions & 4 deletions docs/guides/states.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,13 @@ breaker = CircuitBreaker(
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.
The backoff is local, and deliberately refuses to pretend otherwise. Reopening
a coordinated breaker is decided by the backend from `wait_duration_in_open` and
its own clock, and no failed-round count crosses the wire, so a multiplier above
`1.0` alongside a shared [storage](../integrations/redis.md) raises `ValueError`
at construction rather than being accepted and ignored. Coordinated backoff is
[under discussion](https://github.com/bagowix/interlock/issues); until then a
coordinated breaker waits a constant interval.

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
Expand Down
13 changes: 8 additions & 5 deletions docs/llms-full.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1516,7 +1516,7 @@ config = Config(
| `permitted_calls_in_half_open` | `10` | Probe calls allowed while `HALF_OPEN`. |
| `max_concurrent_probes` | `1` | Cap on **simultaneous** probes in `HALF_OPEN`. Must be in `[1, permitted_calls_in_half_open]`. |
| `wait_duration_in_open` | `60.0` | Seconds to stay `OPEN` before the first probe is allowed. |
| `wait_duration_backoff_multiplier` | `1.0` | Multiplies the open wait after each consecutive failed probe round. `1.0` keeps it constant. Must be `>= 1`. |
| `wait_duration_backoff_multiplier` | `1.0` | Multiplies the open wait after each consecutive failed probe round. `1.0` keeps it constant. Must be `>= 1`, and must stay `1.0` on a breaker with a shared storage. |
| `wait_duration_in_open_max` | `None` | Ceiling for the backed-off wait, in seconds. `None` leaves it uncapped; when set, must be `>= wait_duration_in_open`. |
| `auto_transition` | `False` | When `True`, a timer moves the breaker `OPEN → HALF_OPEN` once the wait elapses, instead of waiting for the next call. See [States](states.md#proactive-transition-auto_transition). |
| `window_type` | `COUNT_BASED` | `COUNT_BASED` or `TIME_BASED`. |
Expand Down Expand Up @@ -1658,10 +1658,13 @@ breaker = CircuitBreaker(
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.
The backoff is local, and deliberately refuses to pretend otherwise. Reopening
a coordinated breaker is decided by the backend from `wait_duration_in_open` and
its own clock, and no failed-round count crosses the wire, so a multiplier above
`1.0` alongside a shared [storage](../integrations/redis.md) raises `ValueError`
at construction rather than being accepted and ignored. Coordinated backoff is
[under discussion](https://github.com/bagowix/interlock/issues); until then a
coordinated breaker waits a constant interval.

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
Expand Down
27 changes: 27 additions & 0 deletions interlock/_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,32 @@ def validate_unreachable_exceptions(
return types


def validate_backoff_support(*, config: Config, storage: Storage | AsyncStorage | None) -> None:
"""Refuse a backoff the coordinated lane cannot honour.

Reopening a coordinated breaker is the backend's decision, taken from
``wait_duration_in_open`` and its own clock, and no failed-round count
crosses the wire — ``SharedState`` carries mechanism, not policy, and has no
field for one. A multiplier set alongside a storage would therefore be read,
validated, and then quietly ignored: the option would look enabled while
every round waited exactly as long as the last.

Silence is the worst of the three possible answers here, so this is an
error. Coordinated backoff is tracked as a separate change; until then the
honest options are a constant wait or a local breaker.

Raises:
ValueError: If a backoff multiplier is set on a coordinated breaker.
"""
if storage is not None and config.wait_duration_backoff_multiplier != 1.0:
raise ValueError(
'wait_duration_backoff_multiplier has no effect on a breaker with shared '
'storage: reopening is decided by the backend from wait_duration_in_open, '
'and no failed-round count is shared. Leave it at 1.0, or drop the storage '
f'to run this breaker locally. Got {config.wait_duration_backoff_multiplier!r}.'
)


@dataclass(frozen=True, slots=True)
class Admission:
"""What ``_admit`` granted: the era it happened in, and probe provenance."""
Expand Down Expand Up @@ -128,6 +154,7 @@ def __init__(
self._clock = clock
self._classifier = classifier if classifier is not None else DefaultFailureClassifier()
self._unreachable_exceptions = validate_unreachable_exceptions(unreachable_exceptions)
validate_backoff_support(config=config, storage=storage)
self._listener = listener
self._machine = StateMachine(
config=config,
Expand Down
6 changes: 5 additions & 1 deletion interlock/breaker.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ class CircuitBreaker:
Raises:
TypeError: If ``unreachable_exceptions`` is not a tuple of ``Exception``
subclasses.
ValueError: If ``initial_state`` is not a supported stable state.
ValueError: If ``initial_state`` is not a supported stable state, or if
``config`` asks for a backoff (``wait_duration_backoff_multiplier``
above ``1.0``) alongside a ``storage``: reopening is then the
backend's decision and no failed-round count is shared, so the
backoff could not be honoured.
"""

def __init__(
Expand Down
9 changes: 5 additions & 4 deletions interlock/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ class Config:
constant wait then retries forever at full rate, and the growing interval is
itself the signal that the dependency is not merely slow to recover.

The backoff is a local decision: with a shared ``Storage`` the coordinated
lane asks the backend to reopen after ``wait_duration_in_open``, and the
failed-round count lives in no shared state, so a coordinated breaker retries
on the base wait however many rounds have failed.
The backoff is a local decision. Reopening a coordinated breaker is the
backend's, taken from ``wait_duration_in_open`` and its own clock, and no
failed-round count crosses the wire — so a multiplier above ``1.0`` is
**refused** on a breaker with a shared ``Storage`` rather than accepted and
ignored.

``auto_transition`` opts into a timer that proactively moves a breaker from
``OPEN`` to ``HALF_OPEN`` once ``wait_duration_in_open`` elapses, emitting the
Expand Down
14 changes: 12 additions & 2 deletions interlock/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from contextlib import AsyncExitStack, ExitStack

from interlock._clock import SystemClock
from interlock._engine import validate_unreachable_exceptions
from interlock._engine import validate_backoff_support, validate_unreachable_exceptions
from interlock._initial_state import validate_initial_state
from interlock.breaker import CircuitBreaker
from interlock.config import Config
Expand Down Expand Up @@ -53,7 +53,11 @@ class Registry:
Raises:
TypeError: If ``unreachable_exceptions`` is not a tuple of ``Exception``
subclasses.
ValueError: If ``initial_state`` is not a supported stable state.
ValueError: If ``initial_state`` is not a supported stable state, or if
``config`` asks for a backoff (``wait_duration_backoff_multiplier``
above ``1.0``) alongside a ``storage``: reopening is then the
backend's decision and no failed-round count is shared, so the
backoff could not be honoured.
"""

def __init__(
Expand All @@ -75,6 +79,7 @@ def __init__(
self._listener = listener
self._storage = storage
self._unreachable_exceptions = validate_unreachable_exceptions(unreachable_exceptions)
validate_backoff_support(config=self._config, storage=storage)
self._breakers: dict[str, CircuitBreaker] = {}
self._lock = threading.Lock()

Expand All @@ -88,6 +93,11 @@ def get(self, name: str, *, config: Config | None = None) -> CircuitBreaker:

Returns:
The cached or newly created breaker.

Raises:
ValueError: If ``config`` asks for a backoff on a registry that holds a
storage. The override route reaches the same constraint as the
registry's own config.
"""
# A hit reads the dict without the lock: this is the per-request path of
# every transport integration, and a breaker is never replaced or
Expand Down
46 changes: 46 additions & 0 deletions tests/test_coordination.py
Original file line number Diff line number Diff line change
Expand Up @@ -1487,3 +1487,49 @@ def test__shared_view__closed_at_the_same_version__does_not_reset_the_local_mach

# Same version: a duplicate or out-of-order delivery, not a recovery.
assert breaker.state is State.OPEN


def test__breaker__backoff_with_shared_storage__rejected(fake_clock: FakeClock) -> None:
"""The combination has to fail loudly: the coordinated lane cannot honour a backoff.

Reopening is decided by the backend from ``wait_duration_in_open``, and no
failed-round count crosses the wire, so a multiplier set here would be read,
validated and then quietly ignored — the exact trap the option exists to
remove.
"""
with pytest.raises(ValueError, match='wait_duration_backoff_multiplier'):
CircuitBreaker(
name='payments',
config=Config(wait_duration_backoff_multiplier=2.0),
clock=fake_clock,
storage=InMemoryStorage(clock=fake_clock),
)


def test__registry__backoff_with_shared_storage__rejected(fake_clock: FakeClock) -> None:
"""Fail at registry construction, not at the first ``get`` that happens to run."""
with pytest.raises(ValueError, match='wait_duration_backoff_multiplier'):
Registry(
config=Config(wait_duration_backoff_multiplier=2.0),
clock=fake_clock,
storage=InMemoryStorage(clock=fake_clock),
)


def test__registry__backoff_in_a_per_breaker_config__rejected(fake_clock: FakeClock) -> None:
"""``get`` takes a config override, and that route must not slip past the guard."""
registry = Registry(clock=fake_clock, storage=InMemoryStorage(clock=fake_clock))

with pytest.raises(ValueError, match='wait_duration_backoff_multiplier'):
registry.get('payments', config=Config(wait_duration_backoff_multiplier=2.0))


def test__breaker__backoff_without_storage__accepted(fake_clock: FakeClock) -> None:
"""Only the combination is refused; a local breaker keeps its backoff."""
breaker = CircuitBreaker(
name='payments',
config=Config(wait_duration_backoff_multiplier=2.0),
clock=fake_clock,
)

assert breaker.state is State.CLOSED
Loading