Skip to content

Commit 57aef79

Browse files
fix(circuit_breaker): run a single probe per process when coroutines call the circuit concurrently
The probe lease is per thread, so concurrent coroutines in one event loop all passed the owner check and probed together. Track in-flight probes per circuit name and give the other callers the open response, matching the single-prober guarantee threads already had.
1 parent dd58df6 commit 57aef79

4 files changed

Lines changed: 105 additions & 19 deletions

File tree

‎aws_lambda_powertools/utilities/circuit_breaker/base.py‎

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from aws_lambda_powertools.utilities.circuit_breaker.persistence.base import (
2828
CircuitBreakerPersistenceLayer,
2929
)
30+
from aws_lambda_powertools.utilities.circuit_breaker.persistence.record import CircuitStateRecord
3031
from aws_lambda_powertools.utilities.circuit_breaker.states import CircuitInfo
3132

3233
logger = logging.getLogger(__name__)
@@ -47,6 +48,11 @@
4748
# callbacks.
4849
_COUNTERS_LOCK = threading.Lock()
4950

51+
# Circuit names with a probe currently running in this process. The probe lease is per
52+
# thread, so concurrent coroutines in one event loop would all pass the owner check and
53+
# probe together; this keeps the single-prober guarantee within a process too.
54+
_PROBES_IN_FLIGHT: set[str] = set()
55+
5056
# Identifier used to claim the half-open probe lock, unique per thread so the store's
5157
# conditional election picks a single prober across threads as well as processes.
5258
_PROBE_OWNER = threading.local()
@@ -175,35 +181,59 @@ def _route(self) -> tuple[_Route, CircuitInfo]:
175181
if prev is not None and prev != CircuitState.CLOSED:
176182
_LOCAL_FAILURES[self.name] = 0
177183
_LAST_OBSERVED_STATE[self.name] = CircuitState.CLOSED
178-
return _Route.CLOSED, record.to_circuit_info()
184+
route = _Route.CLOSED
185+
elif record.state == CircuitState.OPEN:
186+
route = self._route_open(record)
187+
else:
188+
route = self._route_half_open(record)
179189

180-
if record.state == CircuitState.OPEN:
181-
with _COUNTERS_LOCK:
182-
_LAST_OBSERVED_STATE[self.name] = CircuitState.OPEN
183-
# ``opened_at`` may legitimately be 0 (epoch); treat only None as missing.
184-
opened_at = record.opened_at if record.opened_at is not None else self._now()
185-
if self._now() >= opened_at + self.config.recovery_timeout:
186-
# Recovery window elapsed: try to become the single prober.
187-
if self.persistence_store.try_acquire_half_open(self.name, _probe_owner_id(), opened_at):
188-
self._reset_probe_successes()
189-
self._notify(CircuitState.OPEN, CircuitState.HALF_OPEN, opened_at=opened_at)
190-
return _Route.PROBE, record.to_circuit_info()
191-
return _Route.OPEN, record.to_circuit_info()
192-
193-
# HALF_OPEN: only the thread that owns the probe lock runs.
190+
return route, record.to_circuit_info()
191+
192+
def _route_open(self, record: CircuitStateRecord) -> _Route:
193+
"""OPEN: reject, or elect this worker as the single prober once the recovery window elapsed."""
194+
with _COUNTERS_LOCK:
195+
_LAST_OBSERVED_STATE[self.name] = CircuitState.OPEN
196+
# ``opened_at`` may legitimately be 0 (epoch); treat only None as missing.
197+
opened_at = record.opened_at if record.opened_at is not None else self._now()
198+
if self._now() < opened_at + self.config.recovery_timeout or not self._claim_probe():
199+
return _Route.OPEN
200+
# Recovery window elapsed: try to become the single prober.
201+
if self.persistence_store.try_acquire_half_open(self.name, _probe_owner_id(), opened_at):
202+
self._reset_probe_successes()
203+
self._notify(CircuitState.OPEN, CircuitState.HALF_OPEN, opened_at=opened_at)
204+
return _Route.PROBE
205+
self._release_probe()
206+
return _Route.OPEN
207+
208+
def _route_half_open(self, record: CircuitStateRecord) -> _Route:
209+
"""HALF_OPEN: only the probe owner runs, one probe at a time; expired leases can be taken over."""
194210
with _COUNTERS_LOCK:
195211
_LAST_OBSERVED_STATE[self.name] = CircuitState.HALF_OPEN
196212
if record.half_open_owner == _probe_owner_id():
197-
return _Route.PROBE, record.to_circuit_info()
213+
# Owner thread: one probe at a time, even when coroutines run concurrently.
214+
return _Route.PROBE if self._claim_probe() else _Route.OPEN
198215

199216
# If the probe lease has expired (owner recycled mid-probe), take over.
200-
if record.probe_lease_expiry is not None and self._now() >= record.probe_lease_expiry:
217+
lease_expired = record.probe_lease_expiry is not None and self._now() >= record.probe_lease_expiry
218+
if lease_expired and self._claim_probe():
201219
logger.debug("Circuit '%s' probe lease expired; attempting takeover.", self.name)
202220
if self.persistence_store.try_acquire_half_open(self.name, _probe_owner_id(), record.opened_at or 0):
203221
self._reset_probe_successes()
204-
return _Route.PROBE, record.to_circuit_info()
222+
return _Route.PROBE
223+
self._release_probe()
224+
return _Route.OPEN
205225

206-
return _Route.OPEN, record.to_circuit_info()
226+
def _claim_probe(self) -> bool:
227+
"""Mark a probe as in flight for this circuit; False when one is already running in this process."""
228+
with _COUNTERS_LOCK:
229+
if self.name in _PROBES_IN_FLIGHT:
230+
return False
231+
_PROBES_IN_FLIGHT.add(self.name)
232+
return True
233+
234+
def _release_probe(self) -> None:
235+
with _COUNTERS_LOCK:
236+
_PROBES_IN_FLIGHT.discard(self.name)
207237

208238
def _reset_probe_successes(self) -> None:
209239
"""
@@ -270,6 +300,8 @@ def _call_probe(self) -> Any:
270300
except Exception as exc:
271301
self._on_probe_failure(exc)
272302
raise
303+
finally:
304+
self._release_probe()
273305
self._on_probe_success()
274306
return result
275307

@@ -280,6 +312,8 @@ async def _call_probe_async(self) -> Any:
280312
except Exception as exc:
281313
self._on_probe_failure(exc)
282314
raise
315+
finally:
316+
self._release_probe()
283317
self._on_probe_success()
284318
return result
285319

‎docs/utilities/circuit_breaker.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ After `recovery_timeout` seconds, the circuit moves to `HALF_OPEN` and elects a
191191
thread, so the single-prober guarantee spans threads as well as environments, and the in-memory failure counter is
192192
synchronized. Single-threaded functions (the normal Lambda model) are unaffected.
193193

194+
The same holds for coroutines: concurrent `await`s of the same circuit in one event loop run a single probe at a
195+
time, and the others get the open response.
196+
194197
Probe ownership belongs to the thread that won the election. If that thread never runs the circuit again (for
195198
example, a thread-per-request worker pool), recovery waits for the probe lease to expire before another thread or
196199
environment takes over.

‎tests/functional/circuit_breaker/conftest.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ def reset_local_counters():
102102
base_module._LOCAL_FAILURES.clear()
103103
base_module._LOCAL_SUCCESSES.clear()
104104
base_module._LAST_OBSERVED_STATE.clear()
105+
base_module._PROBES_IN_FLIGHT.clear()
105106
yield
106107
base_module._LOCAL_FAILURES.clear()
107108
base_module._LOCAL_SUCCESSES.clear()

‎tests/functional/circuit_breaker/test_circuit_breaker.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import asyncio
34
import threading
45
import time
56
import warnings
@@ -1024,3 +1025,50 @@ async def call():
10241025

10251026
assert result == "bypassed"
10261027
assert any("Disabling the circuit breaker is intended for development" in str(w.message) for w in recorded)
1028+
1029+
1030+
@pytest.mark.asyncio
1031+
async def test_async_concurrent_calls_run_a_single_probe(store, now):
1032+
"""Coroutines share the thread's probe owner id, so without a guard they would all probe at once."""
1033+
store.db["c"] = CircuitStateRecord(name="c", state=CircuitState.OPEN, failure_count=5, opened_at=now - 60)
1034+
config = CircuitBreakerConfig(recovery_timeout=30, success_threshold=2, local_cache_max_age=0)
1035+
probes = []
1036+
1037+
@circuit_breaker(name="c", persistence_store=store, config=config)
1038+
async def call(i):
1039+
probes.append(i)
1040+
await asyncio.sleep(0.01)
1041+
return "ok"
1042+
1043+
results = await asyncio.gather(*(call(i) for i in range(8)), return_exceptions=True)
1044+
1045+
assert len(probes) == 1, "exactly one coroutine must probe per recovery window"
1046+
assert sum(isinstance(r, CircuitBreakerOpenError) for r in results) == 7
1047+
assert store.db["c"].state == CircuitState.HALF_OPEN
1048+
1049+
1050+
@pytest.mark.asyncio
1051+
async def test_async_probe_slot_is_released_after_success_and_failure(store, now):
1052+
store.db["c"] = CircuitStateRecord(name="c", state=CircuitState.OPEN, failure_count=5, opened_at=now - 60)
1053+
config = CircuitBreakerConfig(recovery_timeout=30, success_threshold=3, local_cache_max_age=0)
1054+
calls = []
1055+
1056+
@circuit_breaker(name="c", persistence_store=store, config=config)
1057+
async def call(fail=False):
1058+
calls.append(fail)
1059+
if fail:
1060+
raise ValueError("probe failed")
1061+
return "ok"
1062+
1063+
# a successful probe keeps the circuit half-open and must leave the slot free for the next probe
1064+
assert await call() == "ok"
1065+
assert await call() == "ok"
1066+
assert store.db["c"].state == CircuitState.HALF_OPEN
1067+
# a failing probe reopens; once the recovery window elapses the next call must be able to probe again
1068+
with pytest.raises(ValueError):
1069+
await call(fail=True)
1070+
assert store.db["c"].state == CircuitState.OPEN
1071+
store.db["c"].opened_at = now - 60
1072+
assert await call() == "ok"
1073+
assert calls == [False, False, True, False]
1074+
assert "c" not in base_module._PROBES_IN_FLIGHT

0 commit comments

Comments
 (0)