feat(cron): back a failing cron off instead of re-firing every tick - #625
Open
yzxcj797 wants to merge 1 commit into
Open
feat(cron): back a failing cron off instead of re-firing every tick#625yzxcj797 wants to merge 1 commit into
yzxcj797 wants to merge 1 commit into
Conversation
A cron whose fire failed (e.g. its model provider is down) was re-tried by the schedule alone: nothing updated lastFiredAt on failure, so the next scheduler sweep (1s cadence) re-derived the same scheduledAt and fired again — every 1-5s through a whole outage, feeding the rate limit or degraded provider that caused the failures (yc-software#602). Each cron now carries a failure backoff: a failed fire increments failedAttempts and sets retryNotBefore = now + 30s * 2^n (capped at 5min); due() holds the cron out until the window passes, and a successful fire clears both fields so a recovered cron returns to its schedule immediately. The tick's per-fire isolation (one failing cron not starving the batch) is unchanged and still covered. The starvation test's expectation is updated with the behavior change: its subject — the succeeding cron firing on schedule beside a failing neighbor — is unchanged; the failing neighbor's sub-30s retry it also pinned was the outage-feeding behavior this removes. Fixes the cron-backoff section of yc-software#602
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
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.
Fixes the cron-backoff section of #602 (the remaining half of section 2).
What this fixes
A cron whose fire failed was retried by the schedule alone: nothing updated
lastFiredAton failure, so the next scheduler sweep (1s cadence) re-derived the samescheduledAtand fired again — your "every 1-5s until it recovers" observation, which during a rate-limit outage feeds the limit causing it. (The tick's per-fire isolation your issue also describes is already in place on main —fireDuecatches per cron, so one failing cron does not abort its batch; that part is verified unchanged.)The change
Each cron now carries a failure backoff:
failedAttemptsand setsretryNotBefore = now + cronFailureBackoffMs(n)— 30s × 2ⁿ, capped at 5 minutes (a recurring cron probes a dead provider at most every few minutes, vs every tick);due()holds a cron out whileretryNotBeforeis in the future, whatever the schedule derives — the retry pacing beats the schedule during an outage;markFired), so a recovered cron returns to its exact schedule immediately — no lingering penalty;Tests
a failing cron backs off exponentially instead of re-firing every tick— first failure holds the cron quiet for 30s across four in-window ticks; the window passing re-attempts; the second failure doubles to 60s. Fails onmain(no backoff exists).a cron's backoff clears after a successful fire— a flaky cron fails once, succeeds after the window, and carries no stale not-before.a failing interval cron does not starve later due crons across ticks) — its subject (the succeeding cron firing on schedule beside a failing neighbor) is unchanged and still asserted; the failing neighbor's sub-30s retry its expectation also pinned was precisely the outage-feeding behavior this PR removes, so the expectation now shows the neighbor held off.Full cron suites: scheduler 53/53, store/queue/scope-shared/guardrails 55 pass + 2 pre-existing skips (0 fail), identical on clean
mainapart from the updated expectation;tsc --noEmitclean.Where this leaves #602
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.