Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

association_table: add max_cores attribute, send information to plugin #560

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

cmoussa1
Copy link
Member

@cmoussa1 cmoussa1 commented Jan 7, 2025

Problem

The association_table has no way to track the max number of cores an association can have across all of their running jobs. This is needed in order to track the max number of resources an association has across all of their running jobs.


This PR adds a new column to the association_table called max_cores. This field is configurable both when adding or editing an association in the flux-accounting database. This attribute is added to the rest of the information sent per-association to the priority plugin, where it is stored as another member in the Association class.

Existing tests are updated to account for the addition of the new column, and the flux-accounting database from the previous schema version is also added to the testsuite now that the schema has been updated.

Problem: The association_table has no way to track the max number of
cores an association can have across all of their running jobs. This is
needed in order to track the max number of resources an association has
across all of their running jobs.

Add a new column to the association_table called "max_cores".

Update the schema version number for the flux-accounting database to
account for the new column addition.
Problem: max_cores is an attribute for every row in the
association_table, but it cannot be configured with the add-user and
edit-user commands.

Add max_cores as a configurable field in the add-user and edit-user
commands.
Problem: The flux-accounting database from before the addition of the
"max_cores" column is not in the testsuite, but it should be a part of
the update-db tests since the schema has changed.

Add the flux-accounting DB schema version 24 to the testsuite.
Problem: The priority plugin does not store max_cores information
per-association in the plugin, which it will need in order to do
resource tracking across all of an association's running jobs.

Add max_cores to the list of information sent to the priority plugin
per-association.

Add max_cores as an attribute to the Association class so that it can
be stored and accessed per-association in the plugin. Set a default for
this attribute when creating a temporary association in the plugin.

Edit the accounting.cpp unit test to account for the addition of the
new max_cores attribute.
@cmoussa1 cmoussa1 added new feature new feature plugin related to the multi-factor priority plugin database related to the flux-accounting database labels Jan 7, 2025
@cmoussa1 cmoussa1 requested a review from jameshcorbett January 7, 2025 23:46
Copy link
Member

@jameshcorbett jameshcorbett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I don't really understand 9f04c00 but that's probably OK 😊

@cmoussa1
Copy link
Member Author

cmoussa1 commented Jan 8, 2025

Thanks @jameshcorbett - that commit is just giving an admin (or whoever is using flux-accounting) the ability to configure the max_cores for an association in the database (i.e, set their value to some value other than the default). Do you think that commit would make more sense if it was squashed onto 119acdf?

@jameshcorbett
Copy link
Member

Sorry I copied the wrong SHA I meant 6b4210a.

@cmoussa1
Copy link
Member Author

cmoussa1 commented Jan 8, 2025

OH, oh I gotcha - that commit is adding the DB schema before adding the new max_cores column to the association_table to the testsuite so that it can be tested via the update-db command. This is needed because there are versions of the flux-accounting database that are already in production that will not have the most up-to-date DB schema once this PR lands, so I just need to make sure that the current DBs out there will be able to be updated when a new version of flux-accounting gets released. I hope that makes sense??

@jameshcorbett
Copy link
Member

Got it that makes sense!

@cmoussa1
Copy link
Member Author

cmoussa1 commented Jan 8, 2025

Thanks!! Okay, setting MWP here

@mergify mergify bot merged commit 909529b into flux-framework:master Jan 8, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database related to the flux-accounting database merge-when-passing new feature new feature plugin related to the multi-factor priority plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants