Repository navigation
fix(circuit_breaker): start a fresh probe success streak on each probe election - #8646
Conversation
…e election _LOCAL_SUCCESSES is module-level and was only reset when a probe failed in the same environment or when this environment closed the circuit. A streak left from an earlier recovery window (this environment probed, lost its lease, and another environment reopened the circuit) counted towards the next window, so the circuit could close after fewer than success_threshold probes. Reset the streak right after this worker wins try_acquire_half_open, on both the OPEN path and the expired-lease takeover path. Add regression tests for both paths. Closes aws-powertools#8645
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
|
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks @unknownhackerworld, this is great work! I ran it through the same multi-environment setup as yesterday, including 4 processes against a real DynamoDB table, and the circuit never closed early anymore, while develop still did. The two tests cover exactly the two election paths. Approving now, this will wait here a bit until the release, no need to rebase or touch anything, and you'll get the credit in the release notes. Thank you!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8646 +/- ##
========================================
Coverage 96.93% 96.93%
========================================
Files 318 318
Lines 16031 16036 +5
Branches 1423 1423
========================================
+ Hits 15540 15545 +5
Misses 349 349
Partials 142 142 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |



Issue number: closes #8645
Summary
Changes
The half-open probe success streak lives in the module-level
_LOCAL_SUCCESSESdict inaws_lambda_powertools/utilities/circuit_breaker/base.py. Before this change it was reset in only two places:_call_probe, when a probe fails in this environment_call_probe, when this environment reachessuccess_thresholdand closes the circuitNothing reset it when a worker won a new probe election in
handle(). So a streak from an earlier recovery window carried into the next one. Withsuccess_threshold=3:That breaks the documented rule ("If
success_thresholdconsecutive probes succeed, the circuit closes") and sends full traffic back to a downstream that just failed a probe.The fix adds a small
_reset_probe_successes()helper, which sets_LOCAL_SUCCESSES[name] = 0under_COUNTERS_LOCK.handle()calls it right aftertry_acquire_half_opensucceeds, on both paths:recovery_timeoutelapsed, fresh election)The thread that already owns the lease skips the election (
record.half_open_owner == _probe_owner_id()), so a streak within one window is unchanged. Only a newly won election starts from zero. This matches how_LAST_OBSERVED_STATEalready throws away a stale failure streak after an external close (test_stale_local_failures_reset_on_external_close).The lock is held only for the dict write, never across persistence I/O or callbacks, as the existing comment on
_COUNTERS_LOCKrequires.Tradeoffs
success_thresholdsuccesses and no longer inherits the old owner's partial streak. That means a few more probes before closing. It follows the "correctness over recovery latency" choice from fix(circuit_breaker): make probe election per-thread and synchronize local counters #8323, which noted the inherited streak as pre-existing and left it unchanged.Alternatives considered
handle(), like_LAST_OBSERVED_STATEdoes for failures. This misses the lease takeover path, where the record goes from HALF_OPEN (old owner) to HALF_OPEN (new owner) without passing through OPEN or CLOSED. Tying the reset to winning the election covers both paths in one place.User experience
Before (
success_threshold=3, after the scenario above):After:
Single-environment functions that never lose their lease behave the same as before.
Tests
Two new regression tests in
tests/functional/circuit_breaker/test_circuit_breaker.py, next to the existing bug regressions:test_new_probe_election_does_not_reuse_successes_from_an_earlier_recovery_window: 2 successes, then an external reopen, then a new election. The new window needs its own 3 successes.test_probe_lease_takeover_starts_a_fresh_success_streak: a takeover of an expired lease in an environment that has a leftover streak of 2. The first success counts as 1 and the circuit stays HALF_OPEN.Both fail on
develop(the circuit closes after one probe) and pass with this change. Locally:pytest tests/functional/circuit_breaker tests/unit/circuit_breaker: 85 passedruff checkandruff format --checkon the changed files: cleanmypy aws_lambda_powertools/utilities/circuit_breaker/base.py: no issuesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.