fix: bound restart attempts across all strategies (#318) - #320
Open
FedericoPonzi wants to merge 1 commit into
Open
fix: bound restart attempts across all strategies (#318)#320FedericoPonzi wants to merge 1 commit into
FedericoPonzi wants to merge 1 commit into
Conversation
FedericoPonzi
force-pushed
the
fponzi/318
branch
2 times, most recently
from
July 24, 2026 19:34
22fb11c to
86bd3ff
Compare
kemingy
reviewed
Jul 29, 2026
FedericoPonzi
added a commit
that referenced
this pull request
Aug 3, 2026
Address review feedback on #320. A service that fails to spawn never produces a ServiceExited event, which is the only place restart_attempts was incremented. Spawn failures were therefore restarted forever without consuming the attempts budget, and since the retry delay is backoff * attempts_made, the delay stayed at zero and the service was respawned on every supervisor iteration. This is not limited to fork() failing: exec_args/find_program run in the parent before forking, so a command that isn't found on PATH takes the same path. Increment restart_attempts when handling SpawnFailed. A process that never came to exist is by definition an early failure, so it counts as a rapid failure like any other. Also clarify in the docs what counts as failing "too quickly", pointing at healthiness.healthy-after, and note that spawn failures count too.
restart.attempts previously failed to bound repeated post-start crashes: the on-failure and always strategies ignored the budget and restarted forever, and the attempts counter was reset on the Started transition, so a service that briefly reached Started before crashing never exhausted its budget. Make all three restart strategies honor the attempts budget: - attempts = 0 keeps the existing unbounded behavior (default), so on-failure/always still restart forever unless a budget is set, while never still stops immediately on failure. - attempts > 0 bounds every strategy: once the budget is exhausted the service becomes FinishedFailed instead of restarting. Reset the attempts counter when the service reaches Running (genuinely stable/green) rather than on Started, so crash loops that momentarily reach Started still exhaust their budget. Keep never distinct from on-failure by requiring an actual rapid failure to have been counted before it restarts: failures after a service becomes stable never consume the budget, so restarting them would loop forever. Count spawn failures against the budget too. A command that cannot be executed at all never produces a process exit, so it previously bypassed the only site that incremented the counter and was retried forever. Cap the backoff multiplier at the configured budget, or at 10 when it is unbounded. Since the counter now survives until a service is stable, the respawn delay (backoff * attempts_made) would otherwise grow without bound and push restarts infinitely far apart. Add healthiness.healthy-after (duration, default 0s): for services with no explicit health check, delay the Healthy signal until the process has survived this long, keeping a crash-prone service in the not-yet-green state so its budget deterministically exhausts. Stale health-check results are cleared on each (re)start. Update docs and add unit and integration tests.
FedericoPonzi
force-pushed
the
fponzi/318
branch
from
August 21, 2026 19:41
b1bfd18 to
4062244
Compare
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
2 participants
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.
Bound repeated post-start crashes so restart.attempts is honored by all restart strategies, reset the budget only once a service is genuinely stable, and add a configurable healthiness.healthy-after stable interval.
Motivation and Context
Fixes #318.
restart.attemptswas documented as a bound on rapid restart loops, but the implementation diverged from that intent in three ways:Description
handle_restart_strategynow applies the budget check to all three strategies, so an exhausted service goes toFinishedFailedinstead of restarting forever;attempts = 0~ still means unbounded, so the default behavior is unchanged. Therestart_attemptsreset moves from theStartedto theRunningtransition, so only a service that actually becomes stable clears its budget. A new healthiness.healthy-after duration (default 0s ) makes "stable" configurable for services without an explicit healthcheck by delayingHealthyuntil that much uptime has elapsed, with stale health cleared on relaunch. Docs are corrected accordingly, and unit and integration tests cover the budget across strategies, the reset semantics,healthy-after` parsing, and crash loops terminating after exhausting their attempts.How Has This Been Tested?
Unit and integ tests.
Types of changes
Checklist: