Skip to content

fix(kanban): stall-veto CPU probe measures CPU burned NOW, not Linux lifetime pcpu (t_46a2f8a1) - #1038

Closed
Kyzcreig wants to merge 1 commit into
mainfrom
fix/kanban-cpu-probe-delta-t_46a2f8a1
Closed

Kyzcreig wants to merge 1 commit into
mainfrom
fix/kanban-cpu-probe-delta-t_46a2f8a1

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Card t_46a2f8a1. Fixes the _wait_idle: pid never read idle to the real probe flake in tests/hermes_cli/test_kanban_progress_stall.py (PR #999 CI slice 1/16), and the production bug under it.

Root cause. _worker_cpu_active read ps -o pcpu. On Linux (procps), pcpu is lifetime CPU time divided by lifetime elapsed time, not a current rate. Measured on ACE-AI: a child that burned 2 s of CPU and then blocked read pcpu 66.4 / 26.4 / 16.5 / 12.0 / 9.4 at 3 / 7.5 / 12 / 16.5 / 21 s of pure idleness, while its CPU-time delta over each 0.5 s window was 0.000 s. On macOS the same child reads 0.0 by 7.5 s, because there pcpu is a decaying average. That is why the test passes 10/10 on the Mac Studio and flakes on loaded Linux runners, where a slow-starting child cannot fall below 0.1% within 30 s.

Production impact. This veto is the only process-level input to detect_progress_stalls. On any Linux dispatcher host, a stalled worker that had ever done real work read nonzero pcpu for hours and vetoed its own reclaim. So the stall detector could not reclaim on Linux.

Fix. Two psutil.Process(pid).cpu_times() samples _CPU_SAMPLE_SECONDS (0.5 s) apart. It vetoes only when user+system time goes up in that window. psutil is already a core dependency. An exited or zombie process reads idle. psutil missing, AccessDenied, or any other probe error returns True, so the probe still never authorizes a kill. The 0.5 s sample only runs for runs already past stall_seconds.

Tests.

  • New test_past_cpu_burn_does_not_veto_a_worker_idle_now. A real child burns about 1 s of CPU, signals, then blocks, and the probe must read idle. This is the class regression test.
  • New test_exited_process_reads_idle.
  • The probe-failure matrix now patches psutil instead of subprocess.run. It covers AccessDenied, OSError, RuntimeError and psutil missing.
  • _wait_idle now needs 3 consecutive delta samples instead of 6 pcpu samples.
  • An autouse fixture shortens the real sample window to 100 ms. The file runs in 27 s instead of 53 s on the Mac Studio.

Linux verification on ACE-AI (load about 45-65 from its CI runners) will be posted as a comment.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…lifetime pcpu (t_46a2f8a1)

_worker_cpu_active read `ps -o pcpu`. On Linux (procps) that is lifetime CPU
time / lifetime elapsed, so any worker that ever did real work read nonzero
for hours after stalling and vetoed its own reclaim: detect_progress_stalls
could not reclaim a stalled worker on a Linux host. Measured on ACE-AI: a
child that burned 2s then blocked read 66.4 -> 9.4 pcpu over 3-21s of pure
idleness while its CPU-time delta was 0.000s. The same bias is the CI flake:
a slow-starting test child could not decay below 0.1% within _wait_idle's 30s
on a loaded runner.

The probe now takes two psutil cpu_times() samples _CPU_SAMPLE_SECONDS apart
and vetoes only on a positive user+system delta. Exited/zombie -> idle;
psutil missing / AccessDenied / any probe error -> True (never authorizes a
kill), as before.

Tests: regression test for past-burn-then-idle; exited-process reads idle;
probe-failure matrix moved from subprocess.run to psutil (AccessDenied,
OSError, RuntimeError, psutil missing); _wait_idle streak 6->3 over delta
samples; autouse fixture shortens the real sample window to 100ms.
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Linux verification on ACE-AI at head 08ecc92. The host was already loaded by its CI runners (load1 about 45-72); this test generated no extra load. Interpreter: py3.11.15, psutil 7.2.2.

  • A. New probe + new tests: 13 passed in 30.8 s, 13 passed in 18.7 s, 13 passed in 18.1 s.
  • B. Old probe + old tests (baseline): 2 failed (pid N never read idle to the real probe, the reported flake) in 143 s, then 10 passed in 99 s, then 10 passed in 132 s.
  • C. Mutation (old pcpu probe body + new tests): test_past_cpu_burn_does_not_veto_a_worker_idle_now fails. All 4 samples read [True, True, True, True] for a child that is blocked on stdin. The new test gates the property. test_exited_process_reads_idle passes on both old and new, as expected.

Raw bias measurement (a child burns 2 s, then blocks):

  • Linux: pcpu 66.4, 26.4, 16.5, 12.0, 9.4 at 3 / 7.5 / 12 / 16.5 / 21 s. The CPU delta was 0.000 s in every window.
  • macOS: pcpu 18.7 at 3 s, then 0.0 from 7.5 s on.

@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: kanban-merge-pass · gate: BYPASS: FleetReview paused by Ace 2026-09-22 (state/fleetreview-pause marker present) · why: t_46a2f8a1: Flake: test_kanban_progress_stall "_wait_idle: pid never read idle to the real p; Argus off card review (Ace 13:08), CI green

@Kyzcreig
Kyzcreig enabled auto-merge September 24, 2026 23:07
@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 24, 2026
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: kanban-merge-pass · gate: BYPASS: FleetReview paused by Ace 2026-09-22 (state/fleetreview-pause marker present) · why: t_46a2f8a1: Flake: test_kanban_progress_stall "_wait_idle: pid never read idle to the real p; Argus off card review (Ace 13:08), CI green

@Kyzcreig
Kyzcreig removed this pull request from the merge queue due to a manual request Sep 25, 2026
@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 25, 2026
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Apollo 04:25 PT: superseded — duplicate of #1028 and both conflict with #1040 (merged). Re-port tracked on t_46a2f8a1 (Daedalus); this PR will be closed when that lands.

@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Superseded by #1063 (card t_46a2f8a1). This PR conflicts with main after #1040 merged. #1063 re-ports the same psutil cpu_times() delta onto a sampler seam (_process_cpu_seconds) and carries this PR's tests.

@Kyzcreig Kyzcreig closed this Sep 25, 2026
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Superseded by #1063 (card t_46a2f8a1): same psutil cpu_times() delta, re-ported onto a _process_cpu_seconds sampler seam on top of #1040, with this PR's tests carried over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant