Apply the capped memory overhead to a job that requests exactly the cap - #1007
Open
calvinp0 wants to merge 1 commit into
Open
Apply the capped memory overhead to a job that requests exactly the cap#1007calvinp0 wants to merge 1 commit into
calvinp0 wants to merge 1 commit into
Conversation
set_cpu_and_mem clamps an over-large request to max_mem * job_max_server_node_memory_allocation and then, because that value is exactly the threshold it was just compared against with a strict `>`, falls through to the uncapped branch on every subsequent call. The clamped job is therefore given DEFAULT_JOB_MEMORY_OVERHEAD (1.10) instead of CAPPED_JOB_MEMORY_OVERHEAD (1.05), so the submit script asks the scheduler for 0.95 * 1.10 = 104.5% of the memory the server is configured to have, and the 'max_total_job_memory' keyword is never appended - making that keyword unreachable for any job that arrived at the cap through the troubleshooting loop, which is the only way a job gets there. trsh_ess_job clamps Gaussian memory to exactly this value, so the boundary is hit by ordinary troubleshooting rather than by an unusual input. On the 256 GB server2 fixture the request drops from 273941 MiB (267.5 GiB, more than the node has) to 261489 MiB (255.4 GiB). Only the Orca branch of trsh_ess_job reads 'max_total_job_memory'; the Gaussian branch carries it without acting on it, so newly emitting it changes no troubleshooting decision.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1007 +/- ##
==========================================
- Coverage 64.46% 64.45% -0.01%
==========================================
Files 119 119
Lines 39636 39636
Branches 10276 10276
==========================================
- Hits 25550 25548 -2
- Misses 11102 11106 +4
+ Partials 2984 2982 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
A job requesting exactly the node memory cap was not given the capped-overhead treatment.
The guard read "strictly greater than the cap", so a request landing precisely on the boundary fell through and was submitted without the overhead adjustment. The log wording moves from
exceedstoreachesto match.Provenance
Split out of #960, which is being closed. That PR's other commit reclassified Gaussian
gallocfailures asMemoryOverallocationand halved the memory reservation — superseded bymain's5b5b11b9, which classifies the same failure asGaussianMemoryAllocationand steps the%memfraction down while holding the reservation, and whose message explicitly rebuts halving as "equally wrong, and strictly worse". Itsarc/testing/trsh/gaussian/galloc.outfixture is already byte-identical onmain.This commit is independent of that: it touches
arc/job/adapter.pyandarc/job/adapter_test.py, the superseded one touchedarc/job/trsh.py. It cherry-picks ontomaincleanly.Verification
16 passedinarc/job/adapter_test.py, including the boundary case added here. One commit, two files — no file touched by more than one commit.🤖 Generated with Claude Code