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
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