Draft
fix: add shell-level timeout to perf stat capability probes in metadata collection#717
Conversation
Co-authored-by: romirdes <86635949+romirdes@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix indefinite hanging during metadata collection on large VMs
fix: add shell-level timeout to perf stat capability probes in metadata collection
Aug 31, 2026
…arge virtualized instances perf list --json can also stall indefinitely on some hypervisors (e.g. m6i.16xlarge), just like perf stat. The stderr.txt from the failing test showed PerfSpect was stuck at "collecting metadata", and perf list is the remaining unguarded perf command in that stage. Wrap both scriptPerfSupportedEvents and scriptPerfAllSupportedEvents with timeout 30 to bound them consistently with the perf stat probes.
Metrics collection could stall indefinitely during the "collecting metadata" phase. Nothing in the chain bounded it: RunScripts launches the controller script with no timeout, and the controller's `wait` blocks forever on a script that never exits. A single wedged probe therefore hung the whole run. Wrapping the inner command in `timeout` does not fix this. Each script runs under setsid and may fork; `timeout` signals only its direct child, so a wedged grandchild -- e.g. a perf stuck on a PMU access, as seen on some virtualized instance types -- survives and the controller keeps waiting on it. Add a Timeout field to ScriptDefinition and have the controller start a per-script watchdog that signals the script's entire process group (SIGTERM, then SIGKILL after a grace period), which reaps wedged descendants. Apply a 60s budget to the metadata probes, which are all short by construction. Scripts with no timeout keep running unbounded, so indefinite-duration collection is unchanged. Also: - Report each script's exit code and elapsed time, plus a stderr tail on failure, so a hanging probe is identifiable from the log instead of presenting as a silent stall. Timeouts are logged even when continuing on script error, where the controller still exits 0. - Reduce kill_script's post-SIGTERM grace from 60s to 5s. Cleanup is serial and perfspect's signal handler only allows ~20s for the whole controller to exit, so the old budget let one hung script drag shutdown far past that deadline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A probe that hangs during metadata collection produced no information at all: the controller waited on it forever, and RunScripts only reads the controller's output once it exits, so nothing was ever reported. The run appeared to stall in "collecting metadata" until ssh gave up 300 seconds later (ServerAliveInterval 30 x ServerAliveCountMax 10) and returned exit code 255 with empty output, which was then reported without the stderr that would have explained it. Signalling harder does not fix this. A process in uninterruptible sleep (state D) -- the expected shape of a wedged PMU access on a virtualized instance -- does not act on any signal, SIGKILL included, until the kernel call it is blocked in returns. So the controller must stop waiting rather than try harder to kill: - Abandon a script that survives SIGKILL: the watchdog leaves a marker, wait_for_script stops waiting on it, and the run completes with that script reported as ABANDONED. Other scripts' results are still returned. - Capture why it could not be stopped: per-process state and wchan for the whole process group, plus the kernel stack of any process in state D, which names the call it is stuck in. Readable because metadata scripts run elevated. - Announce each script as it starts, so a controller that dies before producing results still identifies which scripts were in flight. - Remove the blocking wait from kill_script for the same reason; it would stall shutdown past the ~20s the perfspect signal handler allows. On the Go side: - Bound the controller run when every script is bounded, deriving the deadline from the script budgets (sequential budgets add, concurrent overlap). This guarantees we regain control even if the target wedges or the connection stops delivering. A single unbounded script, such as indefinite-duration collection, leaves the controller unbounded as before. - Include stderr in the returned error. An ssh transport failure and a script failure both surface as a non-zero exit code, and the distinction is not recoverable from the code alone. - Log the controller's own reporting before deciding whether its exit code is fatal, so a diagnosis survives the failure paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
On large virtualized x86 instances (e.g.
m6i.16xlarge), the hypervisor stallsperf_event_openindefinitely rather than returning an error. Since metadata probe scripts run with no timeout (timeout := 0in the controller), PerfSpect hangs forever at "collecting metadata" and never reaches "collecting metrics". Smaller VMs (e.g.c6i.2xl) fail fast with a non-zero exit → expected error; bare-metal works normally.Changes
cmd/metrics/metadata.go: Prefix all 9perf statcapability probeScriptTemplatevalues withtimeout 30:Affected probes:
scriptPerfStatInstructions,scriptPerfStatRefCycles,scriptPerfStatPEBS,scriptPerfStatOCR,scriptPerfStatTMA,scriptPerfStatAMDUncoreProbe,scriptPerfStatFixedInstr,scriptPerfStatFixedCycles,scriptPerfStatFixedRefCycles.Each probe normally completes in ~1 s (it runs
sleep 1as the workload). All probes run concurrently, so the 30 s budget is ~30× the normal wall-clock cost and poses no risk of false failures on loaded instances. On a hung instance,timeoutexits with code 124 (non-zero) → probe treated as unsupported → fast-fail with a clear error instead of an indefinite hang.