Repository navigation
Show CPU usage against the current allocation - #34
Merged
Merged
Conversation
2 tasks done
feinburger244
requested changes
Oct 14, 2026
2 tasks done
feinburger244
added a commit
that referenced
this pull request
Oct 30, 2026
Quota commands get the job cgroup or the login search list from one call, and cgroupAncestors reads /proc/self/cgroup itself, so the walk can no longer receive an unvalidated path. After the #34 review removed cgroupAncestors' duplicate ".." check, the walk stopped only if callers validated first. For a path above the root, such as "/../x", filepath.Dir reaches "/" without ever equaling the root, and the loop appends until the process runs out of memory. Ruled out: - Restoring the parent == current guard. It can't trigger once the walk validates its own input. - Having findSlurmJobCgroup call cgroupAncestors. Login scope selection also needs the ancestor list, so callers would read /proc/self/cgroup twice. #34 and #35 call currentCgroupScope from selectCPUContext and selectMemoryContext. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
feinburger244
force-pushed
the
refactor/shared-cgroup-locator
branch
from
October 30, 2026 17:35
db15fe2 to
38ab020
Compare
quota cpu reports CPUs in use against the Slurm job's or login session's allocation, e.g. "Using 1.9 of 3 CPUs". It replaces host-wide per-core percentages, which count every job on the node. Usage is the cgroup's cpu.stat usage_usec sampled over two seconds. A job's allocation is its effective cpuset, capped by a finite cpu.max. SLURM_CPUS_ON_NODE and scontrol are not consulted. This relies on ConstrainCores=yes (infra-config #5956); jobs started before that keep their old cpusets, which may cover the whole node. A login allocation is the tightest finite cpu.max up to user-<uid>.slice. On equal caps the broader cgroup wins, so usage covers all work sharing the cap. Throttling is read from the cumulative cpu.stat counters, so earlier throttling still shows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
feinburger244
force-pushed
the
fix/12-cgroup-cpu-usage
branch
from
October 30, 2026 18:44
b8ac7b8 to
5ebb81a
Compare
feinburger244
changed the base branch from
refactor/shared-cgroup-locator
to
main
October 30, 2026 18:47
Contributor
Contributor
There was a problem hiding this comment.
I ran this commit on
- both login nodes (idle, and with two busy loops under the 1-CPU cap)
- in a 2-CPU
srunjob with three busy loops - from an
sallocshell that stays on the login node.
All reported the right allocation.
Three small things before I approve:
- Use
go mod tidyto drop four indirect dependencies that only gopsutil'scpupackage needed:lufia/plan9stats,shoenig/go-m1cpu,tklauser/go-sysconfandtklauser/numcpus. - The description still says this targets
refactor/shared-cgroup-locatorand points tob8ac7b8, which is gone after the rebase. Could you refresh it? - The README paragraph below.
Please push these as --fixup=5ebb81a commits.
Remove the CPU-only indirect modules left behind after quota cpu stopped using gopsutil/cpu. Refresh the user-facing README and trim comments requested during review; quota behavior is unchanged. The task/cgroup and ConstrainCores prerequisites remain recorded in the target commit.
feinburger244
approved these changes
Nov 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Resolves #12.
quota cpureports usage against the current allocation, such asUsing 1.9 of 3 CPUs, instead of host-wide per-core percentages. It samples for two seconds and reports cumulative and in-sample throttling for finite caps.Login sessions select the tightest finite cap up to their user-slice boundary. Host Slurm jobs use node-local, job-wide usage and the smaller of their effective cpuset and finite CPU cap. Scope comes from the shared cgroup locator, independently of Slurm environment variables.
Targets
mainfollowing the merge of #37; #35 remains based on this branch. Fixup3c0ce59removes unused indirect dependencies, the outdated operator-facing README paragraph, and redundant comments. It does not change CPU reporting behavior. Thetask/cgroupandConstrainCores=yesprerequisite is recorded in5ebb81a.Testing
On
3c0ce59, Go 1.24.4 / macOS arm64: uncached full and race tests,go vet, native and Linux amd64 builds, formatting, diff checks, andgo mod tidy -diffpassed.The review of
5ebb81areports correct allocations on both login nodes, in a 2-CPUsrunjob, and from ansallocshell remaining on a login node. This fixup has not been deployed; multi-node and container behavior remain unverified.Separate limitation found during self-review: an isolated fixture with a nested, delegated
user-9999.sliceselects its 4-CPU cap instead of the enclosing user's 1-CPU cap. This comes from the existing shared locator and is unchanged by this fixup; live reproduction and a separate correction remain pending.Checklist