fix(health-check): cap the pre-healthy backoff at 2s - #125
Merged
Conversation
The pre-healthy backoff doubled from 50ms up to --health-check-interval. With a 20s interval the probe schedule reaches 12.75s then 25.55s, so a Rails app ready at 13s was not noticed until 25.55s — every deploy on cosmos took ~26s regardless of how fast the app booted. The steady-state interval says how often to re-check a target in service. Before the first success nothing is routed there and a probe is cheap, so clamp the pre-healthy delay to min(delay*2, 2s, interval). After the first success the interval governs exactly as before; sub-2s intervals still win. ## Test Coverage - TestHealthCheck_PreHealthyBackoffIsCappedBelowTheInterval: 20s interval, ready at 3.5s, healthy by 5.15s (uncapped: 6.35s); no extra probes after ## Verification - [x] gofmt -l internal/ cmd/ clean - [x] make test passes, go test -race on health check tests - [x] go vet ./... clean Closes #124 Claude-Session: https://claude.ai/code/session_01QkuqoL4xoqxxWkzo7zJgpt
A normal deploy disposes a target that misses its deploy timeout, but `deploy --force` skips that wait and installs the target unhealthy. With the 2s ceiling alone such a target would be probed at boot cadence for as long as it never answered. Past a 60s window the backoff resumes doubling toward the configured interval, as it did before the cap. The ceiling and window are fields with the constants as defaults so the test can shrink them; NewHealthCheck's signature is unchanged. ## Test Coverage - TestHealthCheck_FastWindowExpiresForATargetThatNeverBecomesHealthy: cap 100ms / window 300ms, 503 forever — 7 probes in 1.5s, 16 without the window ## Verification - [x] gofmt -l internal/ cmd/ clean, go vet ./... clean - [x] go test -race ./internal/server/ passes Claude-Session: https://claude.ai/code/session_01QkuqoL4xoqxxWkzo7zJgpt
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.
Summary
internal/server/health_check.go: addmaxPreHealthyDelay = 2s; the not-yet-healthy branch is nowmin(delay*2, maxPreHealthyDelay, hc.interval). Steady state (delay = hc.intervalafter first success) is untouched;nextDelaystill clamps tointervalso sub-2s intervals keep their faster cadence.preHealthyFastWindow = 60s: the 2s ceiling applies only while a target is younger than that; afterwards the backoff resumes doubling towardinterval. This closes the one unbounded path —deploy --forceskipsWaitUntilHealthy, so a target that never comes up would otherwise be probed every 2s indefinitely. Normal deploys dispose such a target atDeployTimeout(30s default) and never reach the window's end.HealthCheck, defaulted from the constants via anewHealthCheckconstructor so tests can shrink them;NewHealthCheck's signature is unchanged.internal/server/health_check_backoff_test.go:TestHealthCheck_PreHealthyBackoffIsCappedBelowTheInterval— 20s interval, backend flips to 200 at 3.5s, asserts first success < 6s (capped schedule lands at 5.15s; uncapped 6.35s) and no further probes in the 500ms after success.TestHealthCheck_FastWindowExpiresForATargetThatNeverBecomesHealthy— cap 100ms / window 300ms / 503 forever: 7 probes in 1.5s; 16 with the window disabled (verified RED).With a 20s interval the old schedule was 0.05, 0.15, 0.35, 0.75, 1.55, 3.15, 6.35, 12.75, 25.55s; now it is 0.05, 0.15, 0.35, 0.75, 1.55, 2, 2, 2… for 60s, then 4, 8, 16, 20, 20… so a booting target ready at T is seen by T+2s.
Closes #124
Test plan
make testgreen,go test -race ./internal/server/greengofmt -l internal/ cmd/empty,go vet ./...clean,make lint0 issuesscript/release-dash v1.1.0.1, verify both arches viadocker buildx imagetools inspect, then bump the gem'sMINIMUM_VERSION(Faster rolling deploys without losing the gates: boot.canary, per-phase timings, unbuffered CI output, proxy v1.1.0.1 dash#146)Deviations & judgment calls
deploy --force+ never-healthy target probing at 2s forever. Thedashgem never passes--force, so this only affects manual operator runs, but a 60s window is cheap and restores the old doubling behaviour after it. 60s is 2× the default deploy timeout so a normal deploy never observes the window's end.make lint: clean with golangci-lint 2.13.2 (built with go1.27). Theci.yml-pinned v2.11.3 cannot read go1.27 export data locally; CI still runs it on its own toolchain and passed.v1.1.0.1) and gem PR intentionally not done here — release is user-supervised per repo rules.https://claude.ai/code/session_01QkuqoL4xoqxxWkzo7zJgpt