fix(visor): bound service-health probes so one dead endpoint can't stall visor state - #4241
Merged
Merged
Conversation
…all `visor state` ServiceHealth fans every /health probe out in parallel and wg.Wait()s on them. A single unresponsive endpoint — in practice a dmsg server whose /health does not answer over the discovery-routed path — blocked the whole call for the dmsg-HTTP client's full ~10s timeout. ServiceHealth is folded into the curated `visor state` snapshot, so that snapshot took ~10s and, under any additional load, tipped over callers' RPC timeouts and came back empty — which then reads downstream as "zero routes / no legs". Bound each probe with a per-request context timeout (4s). A dead endpoint now costs at most that, not the client timeout; healthy services (~300ms) are unaffected. Adds tests pinning both the bound and the healthy path.
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.
ServiceHealth fans every
/healthprobe out in parallel andwg.Wait()s on them. A single unresponsive endpoint — in practice a dmsg server whose/healthdoes not answer over the discovery-routed path — blocked the whole call for the dmsg-HTTP client’s full ~10s timeout.ServiceHealthis folded into the curatedvisor statesnapshot, so the snapshot itself took ~10s. Under any additional load that tipped over callers’ RPC deadlines and the snapshot came back empty, which reads downstream as "zero routes / no legs" — a misleading artifact seen while debugging live route groups.Bound each probe with a per-request context timeout (4s). A dead endpoint now costs at most that, not the underlying client timeout; healthy services (~300ms) are unaffected. Tests pin both the bound and the healthy path.
Evidence: on a live visor
visor statewas a consistent 10.6–10.8s (all individual health entries reported <600ms, yet the call hung — a goroutine dump showedServiceHealthblocked inwg.Wait()on a dmsg-server version probe stuck inclient.Get). With healthy probes at ~300ms, bounding that single hang returns the snapshot to sub-second; unit tests confirm the bound (a dead endpoint returns in ~4s, not the full client timeout). Live re-measurement to follow once the fleet updates.