test(worker-threads): trace worker.js phases for the intermittent stall - #389
Open
szegedi wants to merge 1 commit into
Open
test(worker-threads): trace worker.js phases for the intermittent stall#389szegedi wants to merge 1 commit into
szegedi wants to merge 1 commit into
Conversation
szegedi
requested review from
IlyasShabi,
nsavoire and
r1viollet
as code owners
August 10, 2026 12:18
Overall package sizeSelf size: 2.49 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.3.1 | 504.33 kB | 504.33 kB | | source-map | 0.8.0 | 185.66 kB | 185.66 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
This comment has been minimized.
This comment has been minimized.
szegedi
marked this pull request as draft
August 10, 2026 13:18
szegedi
force-pushed
the
szegedi/worker-test-stall-diagnostics
branch
from
August 10, 2026 13:22
3386f62 to
5bf8784
Compare
szegedi
marked this pull request as ready for review
August 10, 2026 14:37
'should work' times out at 20s on CI while the workload is almost entirely wall-clock deadline work: it finishes in ~4-5s regardless of machine speed, and with the deadline cut to 1ms the whole thing runs in ~60ms, so only that ~60ms scales with the runner. A run reaching 20s is therefore wedged, not slow, and raising the timeout would hide it. Record the phases — per worker chain: spawned, sent a profile, exited — so whoever picks this up can see where it died instead of reconstructing it from a bare "Timeout of 20000ms exceeded". Collecting a mark costs a string and an array push. Set DD_WORKER_TRACE_FILE to also append each mark to a file as it happens. A file rather than stdout because execFile buffers a child's output and only delivers it once the child closes, so a wedged child yields nothing; and appending synchronously as we go means the trail survives the child being killed, which is what the reap in afterEach does to it. Verified: SIGTERM mid-run still leaves the phases up to that point on disk. Nothing here runs on a schedule. An earlier version of this change armed a watchdog timer that produced a diagnostic report and called process.exit(); it wedged CI jobs for 40+ minutes. Bisecting over three runs pinned it to arming the timer specifically — the test-side change alone was green 4/4 on Linux Node 22, the full change wedged 2/4, and the same instrumentation with the timer removed wedged none. The mechanism was not identified: process.exit() with a live busy worker and process.report.getReport() against an unresponsive worker both complete in ~550ms locally, so neither explains it. Hence no timer, no report call and no exit here. Also worth recording from that bisect: the stall reproduces on Linux Node 22 at roughly one job in four, not only on darwin-arm64 and win32 as the earlier 20-run history suggested. Linux is a much easier target for whoever debugs it. No behaviour change unless DD_WORKER_TRACE_FILE is set.
szegedi
force-pushed
the
szegedi/worker-test-stall-diagnostics
branch
from
August 10, 2026 14:50
60c7a07 to
95df11e
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
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.
Worker Threads should workflakes on CI with nothing but:This records which phase the child was in, so whoever picks the stall up can see where it died instead of reconstructing it. It does not fix the stall, and there is no behaviour change unless
DD_WORKER_TRACE_FILEis set.Why it is a stall, not a slow runner
Measured on darwin-arm64 / Node 22:
worker.jsis almost entirely wall-clock deadline work, so only ~60ms actually scales with machine speed. On the same CI runner where a passing job ranshould workin 5105ms, the failing job exceeded 20s while its CPU-bound sibling test took a comparable 13846ms vs 14138ms — so the runner was not slower. Reaching 20s is four to five times beyond anything slowness explains, and raising the timeout would hide it rather than fix it.What this adds
Phase marks — per worker chain: spawned, sent a profile, exited — costing a string and an array push.
DD_WORKER_TRACE_FILEadditionally appends each mark to a file as it happens.A file rather than stdout, deliberately:
execFilebuffers a child's output and only delivers it once the child closes, so a wedged child yields nothing. Appending synchronously as we go also means the trail survives the child being killed, which is exactly what the reap inafterEachdoes to it. Verified — SIGTERM mid-run still leaves the phases on disk:What was removed, and why
An earlier version of this PR armed a watchdog timer that produced a diagnostic report and called
process.exit(). It wedged CI jobs for 40+ minutes each. Bisected over three runs:envoption + async rewrite)So arming the timer is the trigger. The mechanism was not identified —
process.exit()with a live busy worker andprocess.report.getReport()against an unresponsive worker both complete in ~550ms locally, so neither explains it. Hence no timer, no report call and noprocess.exit()here. Nothing in this PR runs on a schedule.Also worth recording
The bisect showed the stall reproduces on Linux Node 22, at roughly one job in four. The earlier 20-run history — in which only
win32-test-22(12×) anddarwin-arm64-test-22(5×) ever failed — was undersampling, not a platform boundary. Linux is a much easier target for whoever debugs this.It is also not our profiler configuration: on win32,
DD_WALL_USE_SIGPROFandDD_WALL_USE_CPEDare both false for every Node version (they sit inside#ifndef _WIN32) andworker.jssetswithContexts = darwin || linux, so all five win32 versions run identical configuration and identical compiled paths — yet only Node 22 fails.Verification
Full matrix green: 30/30 test jobs, 100–172s each, across alpine / centos / linux-x64 / linux-arm64 / darwin-arm64 / win32 on Node 18, 20, 22, 24, 26. Locally:
gts check0 errors,should work3847ms with tracing off, 22 trace lines with it on.