fix(master): keep the failure count across cooldown windows so they double - #269
Merged
beinan merged 2 commits intoSep 26, 2026
Merged
Conversation
…ouble The cooldown record was leased for the window's own duration, so when the window lapsed the count went with it and the next failure started again from the threshold. On staging every master's sweep re-probed the broken store the instant its 10-minute window closed -- a burst of one task per master, three failures, and a fresh 10-minute window, forever. Lease the record for the maximum window instead. The count now survives the window closing, so the first failure after it is N+1 and the next window doubles as the policy intends. is_cooling_down and list_cooldowns check until_ms against now rather than treating the record's presence as the window. A success still clears the record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Sep 26, 2026
…270) ## Problem `record_failure` (#267) bumped the per-target failure count with a plain get → +1 → put. Several masters finish a failing task for the same target within seconds of one another (they all swept it at the same instant), so two could read the same count and both write `count+1` — a lost update. Staging showed three masters failing the same store at 10:34:49/:51/:52 with the record under-counting; a broken store then takes an extra window or two to reach the threshold. ## Fix `get_cooldown_versioned` returns the key's `mod_revision`; `record_failure` writes the new record in a `Txn` conditioned on `Compare::mod_revision == observed`, and on conflict revokes its lease, backs off 5–25 ms with jitter, and retries (up to 64 attempts — contention is bounded by the master count, a handful). The success path is unchanged. ## Verification New etcd-backed test `concurrent_failure_records_are_not_lost`: 12 concurrent `record_failure` calls on one target, asserts `failures == 12`. Under the pre-fix get/put it lost updates; with CAS but a tight 8-retry loop it exhausted retries; with jittered backoff all 12 land. Master suite 28/28 etcd-backed + 42 default, clippy clean. Refs #266, #267, #269. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Beinan Wang <> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
#267 leased the cooldown record for the window's own duration. When the window lapsed, the failure count went with it, so the next failure started again from the threshold and the window never grew. On staging with 6 masters: the instant a broken store's 10-minute window closed, every master's sweep re-probed it (a burst of one task per master), three failures later it was back in a fresh 10-minute window — forever, never doubling.
Observed: failures at 09:19:33/37/38 → window to 09:29 → failures at 09:29:40/41/42 → window to 09:39.
failuresstayed at 3.Fix
Lease the record for
TASK_COOLDOWN_MAX_SECSregardless of the window, so the count survives the window closing. The first failure after a window is N+1 and gets a doubled window, as the policy intends.is_cooling_down/list_cooldownsnow checkuntil_ms > nowrather than treating the record's presence as the window. A success still clears the record.Verification
Extended
repeated_failures_cool_the_target_down_and_sweeps_skip_it: after two failures (threshold 2, base 1 h) the manual enqueue fails as failure 3 and the record's window is >1.5 h, i.e. doubled — not a fresh 1 h. Master suite 27/27 etcd-backed + 42 default, clippy clean.Refs #266, #267, #268.
🤖 Generated with Claude Code