search

flux-framework / flux-accounting

bank/accounting interface for the Flux resource manager
https://flux-framework.readthedocs.io/projects/flux-accounting/en/latest/index.html
GNU Lesser General Public License v3.0
3 stars 10 forks source link

plugin: add estimation of cores-per-node count on system during initialization #469

Open cmoussa1 opened 4 months ago

cmoussa1 commented 4 months ago

Problem

The priority plugin does not know about basic system information it will need in order to enforce a max-cores limit per association, such as the number of cores on a node.

This PR adds an estimation of a cores-per-node count estimate during the initialization of the priority plugin by fetching resource.R from the KVS. It stores this estimate in a global variable in the plugin. The plan is to use this count when calculating the number of cores used by a job when only nnodes are specified. I think that this count might not be exactly right for systems where the core count per-node might be different throughout the system, but I figure this could at least be a good estimate and a start for tracking and enforcing resource limit per-association across all of their running jobs (see conversation in flux-framework/flux-core#6091).

This count is also added to the list of information returned in the callback for plugin.query.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.

Project coverage is 83.08%. Comparing base (a6bd897) to head (5747337). Report is 6 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #469 +/- ## ========================================== - Coverage 83.30% 83.08% -0.22% ========================================== Files 23 23 Lines 1557 1573 +16 ========================================== + Hits 1297 1307 +10 - Misses 260 266 +6 ``` | [Files](https://app.codecov.io/gh/flux-framework/flux-accounting/pull/469?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | Coverage Δ | | |---|---|---| | [src/plugins/mf\_priority.cpp](https://app.codecov.io/gh/flux-framework/flux-accounting/pull/469?src=pr&el=tree&filepath=src%2Fplugins%2Fmf_priority.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-c3JjL3BsdWdpbnMvbWZfcHJpb3JpdHkuY3Bw) | `82.19% <64.70%> (-0.87%)` | :arrow_down: |
cmoussa1 commented 4 months ago

Thank you both for the first pass! I've force-pushed up some changes to drop FLUX_KVS_WATCH, use libflux-idset, and change how the test is structured to instead use flux resource R --include=0 | flux R decode --count=core (this check makes a lot more sense than what I had).

I've also added some comments in code where suggested.