Reap orphaned R workers on RSPH job cancellation

[smjenness]

Adrien, some context since this comes out of nowhere.

RSPH runs ProctrackType = proctrack/linuxproc with no cgroups. linuxproc finds a job's processes by walking parent-PID chains, but R's PSOCK workers reparent to init at launch (parallelly uses system(wait = FALSE)), so SLURM never has a chain to walk. scancel kills the step and the workers keep running at full CPU, invisible to SLURM, which then schedules new work onto cores they still hold.

I cancelled one swfcalib array yesterday and left 163 orphaned R processes on 15 nodes for about 2.5 hours, roughly 400 core-hours. They slowed my own India COVID job and two other users' jobs on the same nodes.

scancel sends SIGTERM before SIGKILL (KillWait = 30s), so swf_configs_rsph() now appends a trap to r_loader that reaps R processes carrying this job's SLURM_JOB_ID. A/B tested on RSPH: cancelling a 4-worker job left 4 orphans without the trap and 0 with it. Gated on reap_orphans = TRUE, and swf_configs_hyak() is untouched.

Why here rather than in slurmworkflow: this is an RSPH misconfiguration workaround, not general behavior. On a properly configured cgroup cluster the trap would convert a graceful SIGTERM (where R can flush and save) into an immediate kill -9, which is strictly worse for no benefit. swf_configs_rsph() exists to hold RSPH-specific bash, so it seemed like the right layer.

Three details in the trap are load-bearing, and each one was wrong in my first draft, so they are worth a look:

• grep -qxz, not -qz. Unanchored, SLURM_JOB_ID=4135 matches 41352064. /proc/<pid>/environ is permission-gated across users but not across your own concurrent jobs, so nothing else prevents that.
• The SLURM_JOB_ID guard. An empty variable degrades the pattern to SLURM_JOB_ID=, which matches every R process you own.
• The trailing :. The generated job.sh runs set -e, and the loop otherwise returns the last grep's status. On a normal exit this job's workers are already gone, so that grep fails against some other job's R process and the step exits non-zero despite having succeeded.

SLURM_JOB_ID is unique per array task (checked: tasks 42/43/44 carried 41352064/65/66 while sharing SLURM_ARRAY_JOB_ID), so a cancelled task never touches its siblings.

Also bumps the r_version default from 4.2.1 to 4.5.1. As far as I can tell only PrEP-DistCEA calls this function today and everything else hand-rolls its own hpc_node_setup, which I suspect is partly because the default went stale.

Unrelated, but spotted while in there: swf_configs_hyak()'s own defaults (hpc = "klone", partition = "csde") error out, since klone wants "compute" or "ckpt". Left alone here.

The real fix is RSPH running proctrack/cgroup, which I have emailed them about. This is mitigation until then.

[AdrienLeGuillou]

The code makes sense and I think it's indeed the right spot to put it.
I haven't used swf_config_rsph in a while and relied on bespoke instructions directly on the "R/hpc_config.R" file on the projects. But for this I think it makes sense to start using it again.

I think we can remove the git section as well. It's not necessary on RSPH
For reference, the Template uses these setup lines:

  hpc_node_setup <- c(
  ". /projects/epimodel/spack/share/spack/setup-env.sh",
  "spack unload -a",
  "spack load r@4.5.1"
)

Just one thing, have you checked the case where only a single array job is canceled? I think the current code is supposed to kill the children only of this array_job, but is it actually the case?

[AdrienLeGuillou]

I suggest removing this as well as it's not necessary on RSPH

[AdrienLeGuillou]

I suggest removing this as well as it's not necessary on RSPH (complement to the argument removal)

[smjenness]

Merged main and acted on both comments.

Most of this branch is redundant now. #52 shipped the same trap as swf_cleanup_r_workers(), wired into swf_configs_rsph() behind cleanup_workers, and the bash is byte-identical to what was here. I dropped the local copy and the reap_orphans argument in favour of the merged one. What is left is the r_version bump and your review.

git

Removed, both the spack load git@ line and the git_version argument. r_loader is now the three lines from the template you quoted, plus the cleanup trap.

I smoke tested rather than assuming, because step_tmpl_renv_restore() in this package runs git rev-parse and git pull off r_loader as its setup lines. On node1, after the new loader:

git=/usr/bin/git
git --version -> git version 2.43.5
Rscript=.../r-4.5.1-fckpa4zolgxzqnfjj64eqsh3npdghcrb/bin/Rscript
R version 4.5.1 (2025-06-13)

git is the system binary, so spack unload -a never touches it. git rev-parse --abbrev-ref HEAD returns 0 in all nine cluster checkouts. I did not run git pull with 119 tasks live, but git ls-remote --heads origin returned a real ref over SSH with exit 0, so the binary, the network and the credentials all work from a compute node.

Two notes. spack load git@2.35.1 still resolves, so the pin was not broken, just unnecessary. And the login and compute nodes carry different system git (2.39.3 vs 2.43.5), so the version is now whatever the node image ships. Anything needing a specific git has to load it explicitly.

Dropping git_version is breaking for anyone passing it, but it fails with "unused argument" when hpc_configs is built, not later on the node. swf_configs_hyak() still loads git and is untouched.

Single cancelled array task

Verified, including the awkward case the doctor work in #52 turned up: exactly one task per array carries SLURM_JOB_ID equal to the bare ArrayJobId. I built NUL-delimited environ fixtures and ran the real grep against them.

cancelling task42, whose SLURM_JOB_ID == ArrayJobId
  spared otherjob
  MATCH  task42
  spared task43

Siblings carry that same number, but as SLURM_ARRAY_JOB_ID, which is a different record, so grep -qxz cannot match it. Sibling safety is a property of the exact-record match, not of the ids happening not to collide.

The same fixtures show what -x is buying. A short job id without it matches every longer id sharing the prefix, across unrelated arrays:

short id 4135 without -x        short id 4135 with -x
  MATCH  otherjob                 MATCH  otherjob
  MATCH  task42                   spared task42
  MATCH  task43                   spared task43

Also in this merge: r_version moves to 4.5.1 with the vignette example updated to match, and I kept one line from this branch's docs that main did not have, that sweeping orphans up afterwards is a poor substitute for trapping the SIGTERM, since a SIGKILL to a worker mid-write on PanFS can wedge it in D state, which is the condition degen_watch.sh classifies as a filesystem stall.