bench: a failed or interrupted leg can leave stale or late artifacts for the report #426

Closed

opened 2026-09-25 09:39:22 +09:00 by natsukium · 0 comments

natsukium commented

2026-09-25 09:39:22 +09:00

Owner

Split from #408. While fixing #408 (the macOS warm startup daemon, and refusing legs the display cannot present), review found faults in how tools/bench cleans up after a failed, interrupted or refused leg. They predate #408 and are not needed for its two points, so they are tracked here.

#408's fix keeps a refused leg out of the report and forces its retake from the leg's env record: field.sweep_refused moves a refused leg's artifacts to refused/ before a suite starts and before the report loads a directory. That covers late writes for refused legs. The faults below remain for legs that were not refused.

1. A stale row survives a retry that raised

Review finding (sol):

A failed startup retry can publish a stale warm result. tools/bench/suites.py:563–595. Both felis rows are replaced only after prime() and the warm measurements return. If an interrupted attempt left felis-warm.startup.json without a .done marker, then a later priming launch raises (for example, Popen fails), run_suite catches the exception and continues to reporting. The loader can chart the old warm samples as this run's result.
Correction: Remove or set aside both existing felis startup rows before retrying the entry, and add a test for an exception during priming with a stale row present.

The same applies to any leg and suite: artifacts from an attempt interrupted after writing its result but before its .done marker stay in the suite directory. If the retry fails before rewriting them, the report charts them. A likely fix is to clear every leg's result stems (startup: felis-cold/felis-warm/<term>) before the retry.

2. A stopped launch can outlive its stop and write afterwards

Review findings (sol):

On an after-sample refusal, set_aside runs before the terminal is stopped. A wrapper or child still finishing a workload can subsequently write a result or .done file into the suite directory.

If the launch does not exit within five seconds of SIGTERM, TimeoutExpired is suppressed and its artifacts are moved while it can still write.

reap snapshots the process tree once, then moves artifacts without confirming that every writer has exited. A workload can spawn a child after the snapshot, or a SIGKILLed process can remain alive briefly.

For #408 these were resolved by the env-record sweep, which does not rely on the writer being dead. A launch that ignores SIGTERM can still outlive open_leg for a leg that was not refused, and felis's wrapper shell runs under the daemon rather than the launch. It can then touch its result or .done while the next leg runs. Decide whether open_leg should wait for, and escalate against, the leg's whole process set.

3. Escalating to SIGKILL needs process identity, not a pid

Review finding (sol), against a reap helper that was dropped from #408:

reap waits and reaps the launch, then sends SIGKILL to every captured PID, including ones that exited during the wait. If one of those PIDs has been reused, the signal targets an unrelated process.
Correction: Track process identity, not just PID, through cleanup; signal only processes confirmed to be the captured ones.

Any fix for (2) that escalates must satisfy this: pidfd on Linux, or a start-time check on macOS.

Split from #408. While fixing #408 (the macOS warm startup daemon, and refusing legs the display cannot present), review found faults in how `tools/bench` cleans up after a failed, interrupted or refused leg. They predate #408 and are not needed for its two points, so they are tracked here. #408's fix keeps a refused leg out of the report and forces its retake from the leg's env record: `field.sweep_refused` moves a refused leg's artifacts to `refused/` before a suite starts and before the report loads a directory. That covers late writes for refused legs. The faults below remain for legs that were not refused. ## 1. A stale row survives a retry that raised Review finding (sol): > A failed startup retry can publish a stale warm result. `tools/bench/suites.py:563–595`. Both felis rows are replaced only after `prime()` and the warm measurements return. If an interrupted attempt left `felis-warm.startup.json` without a `.done` marker, then a later priming launch raises (for example, `Popen` fails), `run_suite` catches the exception and continues to reporting. The loader can chart the old warm samples as this run's result. > Correction: Remove or set aside both existing felis startup rows before retrying the entry, and add a test for an exception during priming with a stale row present. The same applies to any leg and suite: artifacts from an attempt interrupted after writing its result but before its `.done` marker stay in the suite directory. If the retry fails before rewriting them, the report charts them. A likely fix is to clear every leg's result stems (startup: `felis-cold`/`felis-warm`/`<term>`) before the retry. ## 2. A stopped launch can outlive its stop and write afterwards Review findings (sol): > On an after-sample refusal, `set_aside` runs **before** the terminal is stopped. A wrapper or child still finishing a workload can subsequently write a result or `.done` file into the suite directory. > If the launch does not exit within five seconds of SIGTERM, `TimeoutExpired` is suppressed and its artifacts are moved while it can still write. > `reap` snapshots the process tree once, then moves artifacts without confirming that every writer has exited. A workload can spawn a child after the snapshot, or a SIGKILLed process can remain alive briefly. For #408 these were resolved by the env-record sweep, which does not rely on the writer being dead. A launch that ignores SIGTERM can still outlive `open_leg` for a leg that was not refused, and felis's wrapper shell runs under the daemon rather than the launch. It can then touch its result or `.done` while the next leg runs. Decide whether `open_leg` should wait for, and escalate against, the leg's whole process set. ## 3. Escalating to SIGKILL needs process identity, not a pid Review finding (sol), against a `reap` helper that was dropped from #408: > `reap` waits and reaps the launch, then sends SIGKILL to **every** captured PID, including ones that exited during the wait. If one of those PIDs has been reused, the signal targets an unrelated process. > Correction: Track process identity, not just PID, through cleanup; signal only processes confirmed to be the captured ones. Any fix for (2) that escalates must satisfy this: pidfd on Linux, or a start-time check on macOS.

natsukium referenced this issue

2026-09-25 09:41:53 +09:00

bench: keep the warm startup daemon alive and refuse legs the display cannot present #427

natsukium referenced this issue from a pull request that will close it,

2026-09-25 11:16:05 +09:00

bench: promote a leg's artifacts only from a completed attempt #433

natsukium closed this issue

2026-09-25 12:29:38 +09:00

natsukium referenced this issue from a commit

2026-09-25 12:29:39 +09:00

bench: promote a leg's artifacts only from a completed attempt (#433)

natsukium referenced this issue