Skip to content

Coordinated backoff: should the open wait grow under a shared Storage, and how? #196

Description

@bagowix

wait_duration_backoff_multiplier lengthens the open wait after each consecutive failed probe round. It works on a local breaker and does nothing on a coordinated one, so #195 refuses the combination rather than accepting it and ignoring it. This issue is the other half: whether coordinated backoff is worth building, and if so, how.

Why it does not work today

Reopening a coordinated breaker is the backend's decision, not the caller's:

# _coordination.py:322 and :530
view = self._storage.begin_half_open_if_elapsed(
    name=self._name,
    wait_duration=self._config.wait_duration_in_open,   # base wait, always
    permitted=self._config.permitted_calls_in_half_open,
    ttl=self._ttl,
)

The server compares its own clock against opened_at, because instance clocks are not comparable. Nothing tells it how many rounds have already failed, and SharedState has nowhere to put that: state, opened_at, version, probes_permitted, probes_remaining, probes_completed, probe_failures, probe_slows.

The local machine keeps counting _failed_rounds and computing a grown wait, but while the shared view is authoritative (_engine.py:545) that number is never consulted.

Is it even needed?

Worth settling first, because the answer might be no.

Coordination already bounds probe pressure: permitted_calls_in_half_open caps probes per round, lease_probe makes the budget global rather than per-instance, and the poll interval bounds how often anyone asks. A fleet of fifty instances does not probe fifty times harder than one.

What that does not bound is the rate over time against a dependency that stays down. A constant wait means the same probing forever, and the failing round costs the dependency a real call each time. The argument for backoff is the same one that applies locally, and a fleet makes it larger, not smaller. But it deserves a number rather than an assertion — if the coordinated probe rate is already low enough to be harmless, this issue can close.

What five independent designs agreed on

Every approach explored, including the ones that scored worst, landed on the same mechanism: a failed-round counter belongs in SharedState. The docstring already draws the line — "Threshold policy stays in the core state machine (Python). This type carries only what crosses the wire: mechanism, not policy." A count of failed rounds is mechanism, in the same sense version and probes_completed are.

The transition that increments it is already observable server-side: a failed round calls trip_open from HALF_OPEN (_coordination.py:294), a passing one calls close (:298). So trip_open increments when the current state is HALF_OPEN, leaves it alone when tripping from CLOSED, and close resets it. No new call, no new round trip.

Where they disagreed: who does the arithmetic

A. The server computes the effective wait. begin_half_open_if_elapsed gains backoff_multiplier and max_wait_duration; the Lua script raises the multiplier to the stored count and compares atomically against its own clock. Symmetric with wait_duration and permitted, which are already policy passed per call. Scored highest.

Its fatal flaw, found in review: Lua has no equivalent of the MAX_BACKOFF_ROUNDS clamp that _state_machine.py uses. With max_wait_duration=None the exponent runs away and the effective wait becomes effectively infinite, wedging the breaker open — the precise bug #194 fixed on the Python side, reintroduced in the backend. Any server-side arithmetic has to carry that clamp with it.

B. The client computes it from what it read. The poll already holds a view; it reads failed_rounds, applies the policy in Python, and passes the resulting wait_duration. No protocol signature changes at all — one new field, and policy stays where the docstring says it belongs.

The open question is whether this races. The argument that it does not: while the shared state is OPEN, nothing mutates failed_rounds — the only exit from OPEN is begin_half_open_if_elapsed itself, so every instance reads the same value and computes the same wait. One reviewer disagreed and claimed a window between read() and the call. This needs settling by reading the Lua scripts, not by argument — it decides between A and B.

C. Store a deadline instead of a duration. trip_open writes reopen_at in backend time; the server only compares. Removes arithmetic from the backend entirely, but needs the count anyway to know what the next deadline should be, and two instances racing on trip_open have to agree — expected_version fencing probably covers it.

Open questions

  1. Does B actually race? If not, it wins: one additive field, no signature change, policy stays in Python.
  2. Silent degradation. A Storage that predates the field returns failed_rounds=0 forever, so backoff quietly does nothing — the same trap fix: refuse a backoff the coordinated lane cannot honour #195 just closed, moved one layer down. Detect it? Version the protocol? Accept it and document it? One reviewer suggested a protocol_version in SharedState so implementations can declare what they support; that may be worth having regardless of this feature.
  3. Mixed fleet. During a rolling deploy, half the instances know about backoff and half do not. The behaviour has to be predictable throughout, not just before and after.
  4. Minor or major? A new SharedState field with a default keeps third-party implementations constructing; a changed method signature does not. That difference decides the release.

Not urgent

The refusal in #195 makes the current behaviour honest, so nothing is broken while this is discussed. Input welcome, particularly from anyone running a coordinated breaker against a dependency that stays down long enough for the constant wait to matter — that is the case this would serve, and I would rather hear about it than guess.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions