Skip to content

ci(python): contain and sample the experimental legs so a lost runner becomes a warning (#1740) - #1749

Merged
JarryShaw merged 1 commit into
mainfrom
ci/1740-graalpy-resource-monitor
Oct 15, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
ci/1740-graalpy-resource-monitor

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Refs #1740. The owner's rule is that a non-blocking leg may fail only as a warning, and its check must stay green. GraalPy's leg keeps losing its runner 18–30 minutes into its tests, and a lost runner fails the check no matter what continue-on-error says. This keeps the runner alive, so a resource breach fails the step instead, and that is already a warning.

  • Monitor: a detached sampler runs outside the cap. Every 30 seconds it logs memory, disk, load, the top processes by RSS, and the OOM-kill count, as [monitor] lines in the tests' log, uploaded if: always().
  • Containment:
    • GraalPy gets --vm.Xmx2g and -n 3; 3.14t keeps auto.
    • pytest runs under timeout --signal=INT --kill-after=3m 110m and prlimit --fsize=2 GiB, inside a 12 GB, swapless, CPUWeight=20 systemd-run --scope.
    • The probe uses the same flags. If it fails, the tests run uncapped on the same argv, and the log says so.
  • Containment summary, a new step, warns separately on no cap, an OOM kill, a timeout (exit 124, or 137 after 110 minutes) and a MemoryError, each with the last sample. Experimental summary is byte-identical to main's.
  • Timing: at -n 3, GraalPy likely needs about 117–151 minutes (an inference), so it may end on the 110-minute timeout as a warning. That is expected and non-blocking. Caps, check names and jobs are unchanged.
  • Tests: tests/project/test_resource_monitor_1740.py, with sudo always stubbed. 1672's list of steps that carry LEG gains Containment summary.
  • Real-runner only: passwordless sudo plus systemd-run, the cgroup path, and whether the sizes keep the runner alive. This PR's own Experimental Tests run is the first evidence.

… becomes a warning (#1740)

GraalPy's leg loses its runner 18-30 minutes into its tests, which fails
the job, and so its check, past any `continue-on-error`. experimental.yml:

- `Start the resource monitor`: a detached sampler, outside the tests'
  scope, writes `free -m`, `df -h`, /proc/loadavg, the OOM-kill count, the
  scope's memory and the top five RSS processes every 30 s, each line led
  by `[monitor]`; `Run unit tests` streams it with `tail -F`; the stop
  step and the `resource-monitor-<version>` upload are `if: always()`.
- `Run unit tests`: GraalPy alone gets GRAAL_PYTHON_VM_ARGS=--vm.Xmx2g and
  `-n 3`; pytest runs under `timeout --signal=INT --kill-after=3m 110m`
  and `prlimit --fsize=2147483648`, in a 12G, swapless, CPUWeight=20
  systemd-run scope, probed with the same flags under a throwaway unit,
  else uncapped on the same argv. It writes the exit status, elapsed
  seconds, OOM kills and MemoryErrors to `tests-status`.
- `Containment summary`, a new step before `Experimental summary`, warns
  of no scope, an OOM kill, a timeout (exit 124, or 137 with no OOM kill
  after 110 min) and a MemoryError, each with the last sample.
  `Experimental summary` is unchanged, the KTLO summary's script.
- No cap, check name or job changes; nothing gates on it. Tests in
  tests/project/ pin and run each part; workflows.rst describes them.

Refs #1740
@JarryShaw JarryShaw added the ci Pull requests that change CI or workflow configuration (ci: subject prefix) label Oct 15, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 15, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at ddbd9a3b0. A Sonnet cross-review checked it over three rounds (author: Opus). Round 2 broke 1672, and round 3 fixed it.

  • Full project tier: all 48 tests/project/*.py files, plus tests/test_tier_guard.py, pass one at a time. A decoy sudo was never reached, and no tail was left behind.
  • Mutants: round 2's surviving one, a fallback running bare pytest -q, is now caught by the argv pin. Across the earlier rounds 16 of 18 were killed, including GraalPy settings leaking to 3.14t, 110m changed to 120m, a dropped prlimit, tee instead of PIPESTATUS, and dropped uid, --preserve-env, PATH or LD_LIBRARY_PATH.
  • Sources: checked for sudo dropping LD_* (env.c:133-141), systemd scope semantics (run.c), and GraalPy's GRAAL_PYTHON_VM_ARGS (GraalPythonMain.java:232).
  • Unchanged: Experimental summary is byte-identical to main's, and nothing gating changed.
  • My own re-run in the PR's tree: 1740 (26 passed), 1672 (19) and 1738 (13).
  • Real-runner only: sudo systemd-run, the cgroup path, and whether the sizes hold. This PR's own Experimental Tests run is the first evidence.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.57% (unit tier, Python 3.15, ddbd9a3b0, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18860 954 2368 868 91.06%
pcapkit/corekit 2457 69 780 35 96.54%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2906 164 1016 34 93.57%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17174 225 4588 190 98.07%
pcapkit/toolkit 864 182 298 9 76.33%
pcapkit/utilities 512 2 158 4 99.10%
pcapkit/vendor 4417 2342 1010 157 43.38%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit dd846e3 into main Oct 15, 2026
56 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 15, 2026
@JarryShaw
JarryShaw deleted the ci/1740-graalpy-resource-monitor branch October 15, 2026 01:34
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 15, 2026
JarryShaw added a commit that referenced this pull request Oct 15, 2026
) (#1755)

#1749's 2 GiB heap holds, but a GraalPy worker's live heap grows with the
tests it runs. In job 116172356218 the three workers reached 2.2-2.6 GB RSS
in 16 minutes; four tests raised MemoryError, then gw1 raised one in pytest's
own failure report, and the session ended in INTERNALERROR, exit 3.

- Every GraalPy process gets --vm.XX:+ExitOnOutOfMemoryError with its heap:
  a spent worker exits, xdist fails the test it ran as crashed and starts a
  fresh worker. pytest gets --max-worker-restart=30, 30 restarts in all,
  where xdist's default is 4 a worker (12 at -n 3).
- tests-status counts xdist's "node down" lines, and Containment summary
  warns "tests lost an xdist worker". The MemoryError count is word-bounded,
  so OutOfMemoryError and the flag itself are not counted.
- The comments and workflows.rst give the measurements and their sources.

test_graalpy_worker_heap_1753.py pins it: 7 of its 8 tests fail on main's
experimental.yml, the eighth reads the docs. The 1740 and 1732 tests follow
the new argv. tests/project: 921 passed, 1 skipped.

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

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant