Repository navigation
Stop the wire meters a launcher starts once its runs end - #87
Conversation
…ab#82) Each launcher records the meters it starts and, through a detached waiter, stops them once its trainings (ServeLearnBench: the tests that follow) have ended; the harness2 restart loop is stopped with its meter as one process group. A meter found already running is left to the launch that started it.
…econd look A launch started right after another could reuse that launch's meter while its waiter was about to stop it. ServeLearnBench's waiter also waits for the training itself.
|
Thanks for the fix. A review of 6de34f0 (read through, plus small shell/Python probes; not run with real trainings) found one issue worth fixing before merge and a few small ones. Worth fixingA restarted meter overwrites the previous ledger. Before this PR the meter stayed up, so a later launch on the same port reused it and kept adding to the same ledger. Now the meter stops when its run ends, and the next launch starts a new one. The new meter starts empty and writes to the same
Reproduced: a ledger holding Possible fixes: give each meter start its own ledger file (as Small
Noted, no change suggested
Checked and fine: |
# Conflicts: # examples/officeqa/harness2_stream/launch.sh
…arted one keeps its ledger Review fixes for Infini-AI-Lab#82: - A meter started on a ledger an earlier one left carries its totals on (LEDGER_RESUME=1 in hermit/runtime/wire_meter.py, set by the eight launchers whose ledger outlives a meter). Once the launchers stop their meters, the next meter on the port (`run && run eval`, a ServeLearnBench test run again, a CL-Bench tag refused because it exists) wrote an empty ledger over the earlier bill. Off by default: lean_swarm's rerun of an attempt starts empty as before. - Every launch on a port lists its processes in /tmp/hermit-wire-meter-<uid>-<port>.users, and the launch that started the meter stops it only once none of them is alive, so a launch that reuses a running meter no longer loses it when the starter's run ends. - OfficeQA ReHarness prints the meter's pid, not the watcher's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
haizhongzheng
left a comment
There was a problem hiding this comment.
Thanks for fixing #82. We reviewed the PR in five rounds, with two independent reviews per round. Each finding was checked against the code, and most were reproduced with the real wire_meter. We then pushed one commit and a merge of main to this branch.
The stop itself works as described. On Linux, a meter stops about a second after its run ends. The group kill takes the harness2 restart loop with it. The meter writes its ledger on SIGTERM. ServeLearnBench's handoff from training to tests keeps the host meter up.
66fd8a6: review fixes
-
Ledger overwritten on restart. A meter starts with an empty ledger and replaces the file when it flushes. Before this PR the meter never stopped, so this never came up. Once meters stop, the next meter on the same ledger wrote an empty ledger over the earlier bill. Three cases hit it:
- OfficeQA's documented
launch.sh run && launch.sh run evallost the training's embedding bill; - running a ServeLearnBench
testagain lost the earlier host bill; - a CL-Bench tag refused because it already exists wiped that tag's ledgers, because its meters start before the check.
With
LEDGER_RESUME=1, the meter now loads the file's totals at start, after its bind. The eight launchers whose ledger outlives a meter set it. harness2 writes a new ledger per start, so it does not need it. Resuming is off by default, becauselean_swarmreruns a crashed attempt in the same directory and expects an empty ledger. Test:test_a_meter_carries_on_the_ledger_an_earlier_one_leftcovers both settings and fails without the fix. - OfficeQA's documented
-
Shared meter stopped under a running launch. This was the limit stated in the PR. OfficeQA ReHarness is built around one meter per port, with one bucket per config, so two configs running at once share it. Every launch on a port now lists its processes in
/tmp/hermit-wire-meter-<uid>-<port>.users:- its own pid when it checks or starts a meter, and the processes it leaves running when it exits;
- the launch that started the meter stops it once none of the listed processes is alive, and empties the file;
- the file lives in
/tmprather than$TMPDIR, because clusters give each job its own$TMPDIR.
Checked with the real meter: launch A starts the meter and launch B reuses it. The meter outlives A, stops within two seconds of B's end, and a later launch starts a fresh meter.
-
Wrong pid in the log. OfficeQA ReHarness logged the watcher's pid as the meter's; it now logs the meter's.
We also merged main (#88) into the branch; the only conflict was the two CHANGELOG entries. The PR description now has an "After review" section, and the old limit is replaced.
Tests: tests/core/test_backends.py and tests/methods/reharness/test_reharness.py pass (31 tests), and all nine launchers pass bash -n.
Noted, no change
These are rare cases:
- Zombie process. A watched process that exits but is never reaped still answers
kill -0, so its meter stays up. - Last request's usage. A SIGTERM that lands in the milliseconds between a response's last byte and the usage record loses that request's usage.
- Three launches at once. If three launches hit one port within the same three seconds (one meter stopping, one launch listing itself, another restarting the meter), a launch can end up on the new meter without being listed.
- Another user's meter. When a different Unix user reuses a meter, it lists itself in its own users file, so the owner's watcher ignores it. Reusing another user's meter already sends your records into their ledger.
- Private
/tmp. A per-job private/tmp, such as a container or Slurmjob_container/tmpfs, does not share the users file. - PID reuse. A watched pid that the system reuses within the one-second poll keeps the watcher waiting.
setsidunder job control. In a shell with job control on,setsidforks, so$!is not the meter's pid.
🤖 Generated with Claude Code
Change
Fixes #82. Each launcher records the meters it starts and stops them once its runs end (
stop_meters_after): CL-Bench after its trainings, ServeLearnBench after training and tests, OfficeQA ReHarness and harness2 after thehermitcommand theyexec(harness2's restart loop included).A launch reuses a running meter only if it still answers three seconds later, so
run && run evalnever takes a meter being stopped. A meter another launch reuses stops only once that launch's runs have ended too: each launch on a port lists its processes in/tmp/hermit-wire-meter-<uid>-<port>.users. A meter restarted on the same ledger carries the earlier totals on (LEDGER_RESUME=1, set by the launchers).Validation
bash -non the nine launchers.setsid(macOS) and the realwire_meter.py: meters stop after their runs;run && evalstarts a fresh meter; a concurrent launch reuses a live one.Review
8489508: a meter could be reused while being stopped, and ServeLearnBench did not wait for the training; fixed in6de34f0, which a second review found clean.After review
Maintainer review fixes (
66fd8a6, then main merged in):hermit/runtime/wire_meter.py:LEDGER_RESUME=1makes a meter started on an existing ledger keep its totals. Without it, stopping meters made the next meter on the port (run && run eval, a ServeLearnBenchtestrerun, a CL-Bench tag refused because it exists) write an empty ledger over the earlier bill. Off by default, solean_swarmis unchanged. Test:test_a_meter_carries_on_the_ledger_an_earlier_one_left.