Repository navigation
[Eval][Benchmark] e2e matrix: find a server's SHM files when its namespace carries a launch token - #772
Conversation
0d76df8 to
f51bca0
Compare
Automated review🟡 No findings on Not applicable: there is no earlier review summary. Description
Questions
What the review checked
Advisory, not a merge gate. Rules: |
|
@NSagan271 rebased onto main. launch_token took MSTAR_DEPLOYMENT_TOKEN from the lowest pid, usually the API server. /proc there is the env from exec, so it can still be the previous token or empty, and the workers' token was ignored. it now reads the tokens on the processes the server spawned, and falls back to the server's own only when nothing was spawned. left the arena exclusion. mstar_arena* stays out of the leak count, including after a worker dies. same reason mstar_arena_* is left out, and #552 reclaims a dead deployment by the namespace lock. those names have no pid. #552 (fda3dd7) isn't on main. the hash is unchanged, and this doesn't call its helpers. the test that calls _deployment_namespace clears the env token, so it still matches. no GPU stress run. could you take a look when you get a chance? |
…unch token The /dev/shm health check and the stress leak count name a server's tensor files by the hash of its conductor endpoint alone. Once the namespace mixes in a per-launch token, as mstar-project#552 does, they match none of the server's files and every leak check passes vacuously. Read the token from the environment of a process in the server's group and match both namespaces, and leave segments named under the namespace out of the leak count, as pid-named arena segments already are. A build without the token is matched as before.
…oc file The server mints MSTAR_DEPLOYMENT_TOKEN after exec, and /proc keeps the environment from exec. The lowest pid is usually the server, so an old or empty token hid the one the workers were spawned with.
…ed files and an unreadable environ - After a server process exits, the health check and the stress log name the mstar_<ns>_arena_* segments no live process of the server maps. Arena segments still stay out of the leak count: a live arena keeps the segments it grew into until shutdown. - A stress run against a fake server that names its files under a launch token its spawned worker carries and grows arena segments: no leak when it gives its tensor files back, a leak when it does not. - An unreadable /proc/<pid>/environ falls back to the server's own token. - The namespace the harness rebuilds matches tensors._deployment_namespace with a launch token as well as without.
f51bca0 to
07e99fc
Compare
A cold start spawns Torch Inductor's compile workers (one per core) in the server's process group, and they exit once compiles go quiet. The stress run counted all 16 of them as server processes that exited and marked the run a crash; the health check could do the same. Both now leave those helpers out of the server's processes.
merceod
left a comment
There was a problem hiding this comment.
Approve on ca58194. Your numbers reproduce (43 passed on the head, the same 9 fail with main's harness) and a trial merge onto main 5c2871c gives 43 passed too. The rebuilt namespace matches tensors._deployment_namespace with no token, an empty token and a token, 15 of 15, and it always contains the formula #862's test locks in. With a real SHM manager the head's harness counts 5 of 5 held files where main's counts 0. The arena exclusion does what you say, removing it fails the tokened no-leak case and the health test. With the real arena the dead-worker report shows 0 segments while the worker lives and both after it is killed. The compile workers are recognised by the compile_worker path in their cmdline, which no M* worker can match, and all 5 pool processes are caught under torch's default subprocess start.
Notes, fold into one follow-up.
- With TORCHINDUCTOR_WORKER_START set to fork or spawn the compile workers are not recognised (0 of 4 and 0 of 5), so the false crash would come back. M* never sets it.
- unmapped_arena_segments does not re-check that a name still exists after reading the maps, so a process that exits cleanly in between gets its just-unlinked segments reported, an informational line next to the exited finding. One os.path.exists filter fixes it.
- run_session uses cfg shm_dir as the arena root (stress.py lines 508 and 535) while segments are always under /dev/shm (arena.py line 480), which matters only when a job overrides shm_dir.
Merging on this approval, eval tooling, after #862.
What does this PR do?
The e2e matrix's
/dev/shmhealth check (#653) and the--stressleak count (#655) find a server's tensor files by the namespace in their names, which they computed as the hash of the conductor endpoint. #552 (now on main) mixes a per-launch token into that namespace, so on main today both checks match none of the server's files and pass whatever is left behind. This reads the token from the processes the server spawned (/proc/<pid>/environ; the API server mints it after exec, so its own/procentry can still show a stale one) and matches both the token namespace and the endpoint-only one.Arena segments are left out of the leak count on purpose: #552 names them
mstar_<ns>_arena_*, and a live arena keeps every segment it grew into until shutdown, so their count rises under load without any leak. What is reported instead: once a server process has exited, the health check and the stress log name themstar_<ns>_arena_*segments no live process of that server maps (a crashed or SIGKILLed worker's arena). A segment the dead worker grew that a live peer still maps isn't counted, so this can miss one but never flags a live arena.The GPU run below also turned up a false crash: a cold start spawns Torch Inductor's compile workers (one per core) in the server's process group, and when they go idle and exit, the stress run and the health check called it a crash. Those helpers are now left out of the server's processes.
How was it tested?
test/modular/test_eval_compare_transport.pyandtest/modular/test_eval_stress.py: 43 passed on ca58194 (head, on main 82375ad); 9 fail with main's harness (details below).ruff check .clean.ci.ymlruns it, on 2e7c33c1b (this PR before its last commit, on main 294cbab): 6,334 passed, 1,138 skipped, 6 failed, the same 6 as main here (ftfyandprometheus_clientmissing from this venv, 2 signal-timing tests that pass alone).compare --stress --models qwen3_5_2b(arena on) head 0 of 20 failed runs, base 4 of 20 (a false crash this PR now fixes, below). The harness matched the live server'smstar_<ns>_*entries through the worker's token; main's harness matches none.compare --models qwen3tts_1p7b_split_shm,qwen3tts_1p7b_split_shm_arena: no health findings on either arm, WER 2.90% and UTMOS 4.30 on both, latency and throughput within 2%.GPU runs in detail
python -m benchmark.eval compare --base 294cbab66 --head HEAD --stress --models qwen3_5_2b, withMSTAR_SHM_ARENA=1set in the model'scases.yamlenv (harness at 406cc2233). Base's 4 failed runs are one server session: right after the cold start all 16 Inductor compile workers (one per core on this VM) went idle and exited, the stress check counted them as server processes that exited, and the harness stopped the server; its log shows no failure before that. The last commit leaves those helpers out of the server's processes.MSTAR_DEPLOYMENT_TOKENfrom the spawned worker, built the token namespace and the endpoint one, and matched the server'smstar_<ns>.lockand its twomstar_<ns>_arena_*segments (all left out of the count, as they should be). Main's harness builds only the endpoint namespace and matches none of them.python -m benchmark.eval compare --base 82375ad11 --head HEAD --models qwen3tts_1p7b_split_shm,qwen3tts_1p7b_split_shm_arena(the arena entry is the SHM split config withMSTAR_SHM_ARENA=1; harness at this head). Thetimeoutcase is "incomplete" on both models because base did not time the long request out (status 200), as on main. Run with the harness before the last commit, both arms' TTS case reported "16 server process(es) exited", the same compile-worker exit.Tests that fail with main's harness
The token from the spawned workers (stale and empty parent token); the namespace matching
tensors._deployment_namespacewith a token; a stress run against a fake server that names its files under a launch token and growsmstar_<ns>_arena_*segments (a leak when it keeps its tensor files; with the arena exclusion removed, the no-leak case fails too); an unreadable/proc/<pid>/environfalling back to the server's own token; the dead-worker arena report and the health test that now expects it; compile workers left out of the server's processes.Not tested
The stress run with the arena off (file transport between the API server and the worker), and a rerun of the stress session with the compile-worker fix (the fix only changes which processes count, and the SHM run above shows it on both arms).
Checklist
ruff check .passesAI assistance: written with AI coding agent