Fix multi-QOS per-user/account GRES usage recharged to wrong QOS on reload#207
Open
Yash0270 wants to merge 7 commits into
Open
Fix multi-QOS per-user/account GRES usage recharged to wrong QOS on reload#207Yash0270 wants to merge 7 commits into
Yash0270 wants to merge 7 commits into
Conversation
Yash0270
force-pushed
the
fix/multiqos-gres-usage-leak
branch
6 times, most recently
from
June 28, 2026 19:34
4c0361b to
abf754b
Compare
Yash0270
marked this pull request as ready for review
June 29, 2026 16:18
Yash0270
marked this pull request as draft
June 30, 2026 17:23
Yash0270
marked this pull request as ready for review
June 30, 2026 17:44
Yash0270
marked this pull request as draft
June 30, 2026 17:51
Yash0270
marked this pull request as ready for review
July 1, 2026 05:03
A job submitted with multiple QOS (--qos=a,b) is dispatched and charged under one qos_list member, recorded in job_ptr->qos_id. On state reload (scontrol reconfigure, slurmctld restart, backup controller takeover) job_mgr_load_job_state() resolves qos_ptr via _get_qos_info(), which returns the highest priority qos_list member (list_peek()) and then overwrites job_ptr->qos_id with it. For a running/suspended job this re-attributes its per-user and per-account QOS TRES usage to the head member when _restore_job_accounting() recharges it, inflating that member's counters and spuriously blocking jobs with QOSMaxGRESPerUser. Keep a running/suspended multi-QOS job on the member it was dispatched under (job_ptr->qos_id), mirroring the pending-vs-running handling in _foreach_cache_update_job(). Pending jobs still resolve to the highest priority member. The qos_ptr restore already present in _adjust_limit_usage() keeps acct_policy_add_job_submit() from leaving the job on the head member before the subsequent JOB_BEGIN. Changelog: slurmctld - Fix multi-qos per-user and per-account QOS TRES usage being recharged against the wrong QOS member on state reload. Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
… test Verify a running --qos=low,high job forced onto the non-highest-priority member stays attributed to it (QOS label and per-user gres/gpu usage) across scontrol reconfigure. Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
The existing test only triggers "scontrol reconfigure", which on current 26.05 already preserves the running job's dispatched-QOS attribution, so it passes with or without the fix and does not actually exercise it. The path the fix targets is job state reload via job_mgr_load_job_state(), hit on a slurmctld restart / backup takeover. Add test_multiqos_gres_usage_survives_restart, which restarts slurmctld and asserts the running --qos=low,high job stays charged to its dispatched member (QOS_LOW) instead of being recharged to the highest member. This case fails on unpatched 26.05 and passes with the fix. Add an autouse fixture to cancel jobs between the two tests so the per-user QOS cap is free for each. Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
ATF's function-scoped autouse fixture (conftest.py function_setup) already cancels each test's jobs on teardown, so the extra cancel_jobs_after_test fixture added alongside the restart test is unnecessary; the two tests are isolated by the framework. Removing it (and its misleading comment). Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
used_gpu_for_qos() matched per-user gres/gpu usage with a greedy "[^}]*gres/gpu=..." pattern. A user's assoc_mgr block holds two gres/gpu entries: the MaxTRESPU used value and the MaxTRESRunMinsPU "unlimited" sentinel (2*UINT32_MAX). The greedy match ran to the last one, so the helper returned 8589934590 instead of the used value (2), and the baseline "assert used_gpu_for_qos(QOS_LOW) == JOB_GPUS" failed before the reload path could be exercised. Anchor the match on the MaxTRESPU field and capture the used value from its parentheses, robust to the limit being N or a number. Verified on 26.05 and master: both tests fail on unpatched builds for the intended reason and pass with the fix. Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
Non-functional: black (pre-commit) wraps two >88-char lines added earlier in this series. Kept separate from the functional change per CONTRIBUTING. Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
used_gpu_for_qos only checked the per-user (MaxTRESPU) block, but the fix also covers per-account attribution. Add used_gpu_for_acct_under_qos() parsing the MaxTRESPA (Account Limits) block and assert account usage stays on the dispatched QOS across reconfigure and restart. Drop the now- redundant helper docstrings to match the surrounding test style. Signed-off-by: Yash Gupta <guptayash0270@gmail.com>
Yash0270
force-pushed
the
fix/multiqos-gres-usage-leak
branch
from
July 2, 2026 06:51
3d40a39 to
44f4329
Compare
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.
Problem
A job can be submitted with more than one QOS (
--qos=a,b). It runs under whichever member the scheduler picked, and its GRES/TRES usage is accounted to that member (saved injob_ptr->qos_id).When the controller recovers job state from
StateSaveLocation(aslurmctldrestart or a backup takeover) it re-reads each running job and resets its QOS to the head of the QOS list instead of the member it actually started under, then recharges its per-user/per-account usage there. The list head is the first--qosmember after sorting by priority; when the listed QOS share the same priority, the head is simply the first one listed.So every running multi-QOS job a user has is re-attributed to that single head QOS, regardless of which member it really ran under. The head QOS's usage counter then grows past what is physically running, and new jobs are blocked with
QOSMaxGRESPerUser. In production the head QOS's per-user counter showed ~976 GPUs in use while the user was actually running only ~300 (the two QOS had equal priority, so the head was just the first one in the job's--qoslist).Where it goes wrong:
job_mgr_load_job_state()resolves the QOS via_get_qos_info(), which returnslist_peek()(the list head) and overwritesjob_ptr->qos_idwith it;_restore_job_accounting()then recharges usage against that member.The in-place
scontrol reconfigurepath is affected in Slurm 25.05 version but it got fixed in the future version, 25.11.Fix
At state recovery, keep a running/suspended multi-QOS job on the member it was dispatched under (
job_ptr->qos_id); only pending jobs fall back to the list head. This mirrors the pending-vs-running handling already in_foreach_cache_update_job(), and makes accounting reflect the QOS the job is physically running under.That is the whole fix: one change in
job_mgr_load_job_state(). Theqos_ptrrestore already present in_adjust_limit_usage()keepsacct_policy_add_job_submit()from leaving the job on the head member before the subsequentJOB_BEGIN, so nothing else is needed.(Note for backports: on 25.05 that restore in
_adjust_limit_usage()runs only forACCT_POLICY_REM_SUBMIT, so theADD_SUBMIThighest-QOS bump is left in place and thisjob_mgr.cchange alone is not sufficient there; a 25.05 backport also needs_adjust_limit_usage()to skip_set_highest_prio_qos_ptr()for running jobs.)Target Release
Targets
26.05.2Slurm release. This is a minor patch to an existing bug.Issue
Reported in SchedMD support ticket: https://support.schedmd.com/show_bug.cgi?id=23033
Testing
Regression testing: added an ATF test (
testsuite/python/tests/test_124_2.py) with two cases:test_multiqos_gres_usage_survives_reconfigureguards the in-place cache-rebuild path.test_multiqos_gres_usage_survives_restartexercises the state-recovery path this fix targets; it fails on unpatched 26.05 and passes with the fix.Functional testing: verified end-to-end on a from-source 26.05 cluster, comparing a plain-
26.05build against the patched build and triggering aslurmctldrestart:low=2 high=2low=2 high=2low=0 high=4(job re-attributed to the head)low=2 high=2(stays onlow)(For reference,
scontrol reconfigurewas also tested and is unaffected on 26.05, before and after the fix.)