fix: keep the health deadline armed when the configured cadence is absurd - #342
Merged
Conversation
Brings health.Lease, which the next commit adopts.
…surd The freshness deadline was three scan intervals, multiplied inline. That product can overflow int64 nanoseconds, and WithMaxAge cannot tell a wrapped negative duration from the non-positive value that deliberately disables the deadline -- so a large enough SCAN_INTERVAL silently disarmed the wedge check instead of widening it. health.Lease does the same arithmetic with saturation, so an absurd cadence now yields the largest representable deadline rather than none, and the field names carry what the multiplier meant.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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.
What
Bumps
github.com/cplieger/healthto v1.8.0 and derives this app's probe freshness deadline from the newhealth.Leaseinstead of multiplying its interval inline.Why
WithMaxAgetakes a bare duration, so each app computed one by hand. Across the eight apps that arm a deadline, one rule was spelled six different ways, and what each multiplier meant lived only in a comment beside it.The correctness half is overflow. A deadline derived from an operator-supplied cadence can overflow int64 nanoseconds, and
WithMaxAgecannot tell a wrapped negative duration from the non-positive value that deliberately disables the deadline — so a large enough configured interval silently disarmed the wedge check the deadline exists to arm, instead of widening it.Lease.Duration()saturates at the largest representable duration instead.The smallest disarming input is around 97 years, so this is a class fix rather than a live defect. It is worth making anyway because the failure is silent and inverted: the guard reports healthy forever rather than failing loudly.
Shape
Two commits: the dependency bump, then the adoption. The bump alone compiles and passes —
Leaseis purely additive to the library — so neither commit breaks a bisect.The per-app specifics (which config value feeds the interval, what the multipliers were, and any app policy deliberately left outside the library) are in the adoption commit's own message.
Verification
gofmt,go build,go vetand the full test suite, all run withGOWORK=offso the app resolves the publishedhealthv1.8.0 rather than a local checkout.