Pull requests / #3590
#3590 [Engine] stabilise the serve test job: lifeline identity by start time, sampler that survives a bad source
open · @justindressler · 0 コメント · GitHub で見る
Server & APIAMD / HIPNVIDIA / CUDAWindowsLinux
本文
## Problem and result The **Minimal serving runtime** job fails intermittently on changes that do not touch `serve/` at all, which makes every PR's check look broken for reasons that have nothing to do with the PR. Two different failures were observed on this account's fork, on the same 929-test run: 1. `test_responses.NoLeakedSampler.test_server_close_ends_the_sampler` — `AssertionError: 1 != 2`, twice in a row on one branch and green on the next run of the same commit. 2. `test_lifeline.Lifeline.test_the_portable_watcher_ends_the_child_when_the_server_is_killed` — `AssertionError: False is not true : the child outlived a killed server (watcher)`, on `main` with no local changes, and on `v0.2.2`'s own run. **Result: both are fixed at the cause, not by loosening the assertion.** The lifeline one was a real bug, and it reproduced here 6 times out of 8. `serve/lifeline.py` built the child's identity with `ps -o lstart=,stat=` and kept the whole line, so the token changed whenever the process changed state — measured on one live child: `... R` once, `... S` eleven times inside 150 ms. The portable watcher compares its own reading against the token taken at start, concluded "that is not my child any more", and exited **without ending it**. Identity is now the start time alone: field 22 of `/proc/<pid>/stat` on Linux (the same rule `strata_control/procs.py::start_token` already used), and `ps -o lstart=` elsewhere; a zombie has no identity, so an already-dead child is skipped rather than signalled. After the change the same test passed 8 out of 8. The sampler one turned out **not** to be a race in the test. The count-based assertion was ambiguous (`1 != 2`), so it was rewritten to compare thread identities and say which half failed — and the next CI run on that rewritten test reported `0 != 1 : the server did not start its hardware sampler`. The thread was being started and killed immediately: `Telemetry._loop` had no exception handling, so the first failing `sample()` on a machine whose GPU/psutil source is not readable ended the sampler for good. `GET /metrics` then silently stops updating. `_loop` now catches a failing sample, prints one line, and keeps sampling once a second (`"a monitor must outlive its source"`), and the test asserts on thread identity so it can only fail for the real reason. ## Related discussion - #3407, *[Engine] serve/lifeline.py: the portable watcher does not end the child when the child's ps state changes (test_lifeline fails 5/5 on Linux)* — the same root cause, reported independently a day earlier with the same `ps -o lstart=,stat=` diagnosis. That report proposes dropping the state column from the `ps` output; this PR takes the equivalent idea further and removes the `ps` call: on Linux the identity is field 22 of `/proc/<pid>/stat`, which is what `strata_control/procs.py::start_token` already uses, so no subprocess and no locale-dependent date parsing. The sampler half has no existing thread. - Scope agreement is not needed for either half: both are focused fixes to an existing test job, which CONTRIBUTING.md allows straight to a PR. ## Approach Three small changes, no behaviour change beyond the watcher and the sampler doing what they were meant to do: - `serve/lifeline.py`: `_start_token()` and the inline `start_of()` inside `_WATCHER` use the process start time as identity instead of a `ps` line that includes the volatile state column. Linux reads `/proc/<pid>/stat` directly (no `subprocess` call, which also removes a `ps` dependency from the watcher on that path); other POSIX uses `ps -o lstart=` only. Zombies return no identity. - `serve/test_responses.py`: `NoLeakedSampler` uses a set of `threading.enumerate()` idents instead of a count, and asserts on the difference, with a message that names which half failed. - `serve/telemetry.py`: `Telemetry._loop` survives a failing `sample()` instead of dying on the first one, and complains once rather than once a second. Both fixes are additive to the existing behaviour: the `pdeathsig` and Windows job-object paths are untouched, and `release()`/`end_children()` are unchanged. ## Validation Revision: `main` at `011c9e60` (0.2.2). Environment: WSL2 on x86-64, Python 3.12, no GPU (none needed). **Passed:** - `python -m unittest serve.test_lifeline.Lifeline.test_the_portable_watcher_ends_the_child_when_the_server_is_killed` — **8/8 runs green** after the change; the same command was **6 failures out of 8 before it**, so the repro and the fix are both measured, not argued. - `python -m unittest serve.test_lifeline` — 6 tests, OK. - `python -m unittest serve.test_responses.NoLeakedSampler` — **6/6 runs green** (and green again after the telemetry change). - A sampler whose `sample()` always raises: thread still alive after 2.4 s, 3 sample attempts, one complaint line, and it still stops cleanly. - `python -m unittest discover -s serve -p 'test_*.py'` (the job's own command) — **929 tests, OK (skipped=14) in 307 s, 0 failures** — the first fully green run of this suite in this campaign; the same command failed one or both of the tests above on every previous attempt.. **Failed:** - None observed after the change. **Not run:** - Windows (job-object path) and macOS (`ps -o lstart=` fallback): no hardware here. The Linux path is exercised on every run of these tests; the other two are reasoned about, not measured. - CUDA, HIP and SYCL builds: these files are Python process supervision and a test, nothing the engine compiles. ## Limits - Identity on the non-Linux POSIX path is `ps -o lstart=`, which is second-granular. A PID recycled within the same second could alias — the same limitation this code had before, and the Linux path (where the watcher runs in practice) is exact. - The watcher still gives the child SIGTERM, waits up to 10 s, then SIGKILLs; a child that ignores SIGTERM is unchanged in behaviour.
関連リンク
インストール・モデル・リリースへのサイト内リンク。