Skip to content

Conversation

@sjpb
Copy link
Collaborator

@sjpb sjpb commented Jan 6, 2021

Currently node memory defaults to 1MB if ram_mb not set in role vars. This changes the default to the real free memory as given from ansible facts. Given the number of cpu cores etc is set from facts this seems uncontrovertial.

@sjpb sjpb requested a review from jovial January 6, 2021 11:38
Co-authored-by: jovial <will@stackhpc.com>
@sjpb
Copy link
Collaborator Author

sjpb commented Jan 6, 2021

@jovial
Copy link
Contributor

jovial commented Jan 6, 2021

Sorry, checked the slurm docs and it seems that RealMemory is in megabytes :-/

Size of real memory on the node in megabytes (e.g. "2048"). The default value is 1. Lowering RealMemory with the goal of setting aside some amount for the OS and not available for job allocations will not work as intended if Memory is not set as a consumable resource in SelectTypeParameters. So one of the *_Memory options need to be enabled for that goal to be accomplished. Also see MemSpecLimit.

(https://slurm.schedmd.com/slurm.conf.html)

So not sure if we need to do a conversion from base 2 to base 10. Any idea?

@sjpb
Copy link
Collaborator Author

sjpb commented Jan 6, 2021

I thought "megabytes" tended to get used generically for either unit, esp. in older docs? So is the 2048 example a hint that it's expecting base2?

We're not doing a conversion really, it's just that testing free vs ansible showed ansible was using mebibytes so wanted to document that.

Leave code as-is and just revert docs to say "megabytes"?

@jovial
Copy link
Contributor

jovial commented Jan 7, 2021

I thought "megabytes" tended to get used generically for either unit, esp. in older docs? So is the 2048 example a hint that it's expecting base2?

Think you right about that. From what I can tell, slurm seems to be using base2 internally: https://github.com/SchedMD/slurm/blob/23cbe39d98cfacd9434f10c19a415c6092e4c61c/src/slurmd/slurmd/get_mach_stat.c#L115 and referring to it as MB

@sjpb sjpb merged commit 83ff39f into master Jan 7, 2021
@sjpb sjpb deleted the feature/realmemory branch January 7, 2021 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants