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

drop default queue configuration and assignment #279

Closed
garlick opened this issue Sep 30, 2022 · 6 comments
Closed

drop default queue configuration and assignment #279

garlick opened this issue Sep 30, 2022 · 6 comments
Assignees

Comments

@garlick
Copy link
Member

garlick commented Sep 30, 2022

In #278 a test of flux-accounting assigning a default queue was removed because any default queue should already be assigned before the mf_priority plugin sees the job. This issue is a reminder to remove any now unused functionality in flux-accounting for default queue configuration and assignment.

@cmoussa1 cmoussa1 self-assigned this Sep 30, 2022
@cmoussa1
Copy link
Member

Thanks for opening this @garlick! I think I could use some more clarification before I make some progress on this, so I'm sorry in advance if I am misunderstanding anything, but perhaps with the changes proposed in flux-framework/flux-core#4627, I should begin work on dropping most queue references throughout flux-accounting to remove confusion on where exactly where to define queues and their respective properties. Currently in flux-accounting, this is where queues are referenced and can be interacted with:

flux-accounting DB

There currently exists both a queue_table that holds queues and their associated integer priority. There is also a queues column in the association_table which specifies which queues an association (i.e a user/bank combo) is allowed to submit jobs under. Commands exist that allow an admin to add, remove, and edit both the queue_table and the queues column for each association.

If the job manager is already reading queue information defined somewhere (TOML file? Something else? I'm not sure), then perhaps flux-accounting should instead just read from this file, parse the queue information, and self-populate the information it needs to the database. Then, I could also just drop all of the commands that interact with the queue_table, and an admin would only need to define all this queue information in just one place. Does that make sense? Sorry if I'm off base here.

multi-factor priority plugin

You've already opened a PR on this and stated in your description above that the job-manager is now responsible for validating queues, which the multi-factor priority plugin currently does. So I think I can just drop this validation from the plugin all together. The only thing I think would be left to consider is how the plugin is to associate an integer priority with each queue when calculating a job's integer priority.

To my understanding, some queues are higher priority than others and there needs to be a way to take that into account when a job is submitted. Currently, the plugin factors each queue's respective priority when calculating the priority for an individual job, but if the job-manager does this after #4627, then I should drop this support in the plugin too. If not, perhaps this information can also be read from wherever queue information is defined in flux-core and self-populate so that it does not need to be defined in both flux-core and flux-accounting.

I hope I made at least a little sense and didn't ramble, but let me know your thoughts and what you think might be best!

@garlick
Copy link
Member Author

garlick commented Sep 30, 2022

Thanks for taking the time to describe this so clearly, and I apologize for not tracking flux-accounting closely enough to know this already!

My main point is that flux-accounting should not assign a queue to a job, as seemed to be expected in those tests that I proposed to remove. By the time a job reaches mf_priority it will already have a queue (if configured) and the queue will have been validated against the TOML [queues] config. Calculating the priority is still entirely flux-accounting's job.

I think for now you should keep the queue_table as a way to assign the queue priority factor in mf_priority, and the queues column in the association_table for access control, but think about any fallout that might result from this getting out of sync with the TOML config. Maybe one way to avoid requiring all TOML queues to appear in the database would be to have a default priority and a default allow/deny rule for unknown queues, and then you would only need to explicitly list the ones that are different?

I don't know if it's feasible to transition flux-accounting to using TOML for those tables, or if there should be some way for the db to track the TOML config as you mentioned? That's maybe a topic for a coffee discussion?

I think to close this issue, just make sure flux-accounting isn't trying to assign queues to jobs. Does that make sense?

@cmoussa1
Copy link
Member

Yes, I believe that makes sense. As of now, I do not believe flux-accounting is assigning any queues to jobs, it is just using a queue's associated priority to help further calculate the priority of the job. But you are right and make a great point, that if the information present in both the flux-accounting DB and the TOML config get out of sync, things could get wonky, so I think there's definitely something to think about how we might sync up the two!

@garlick
Copy link
Member Author

garlick commented Sep 30, 2022

Maybe nothing to do in this issue then? Please close if you think not!

@cmoussa1
Copy link
Member

cmoussa1 commented Sep 30, 2022

I think perhaps I should open a new issue on dropping the queue validation part in the priority plugin since that is now part of the job-manager's responsibility (I think?) - does that sound okay?

@garlick
Copy link
Member Author

garlick commented Sep 30, 2022

Yep!

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

No branches or pull requests

2 participants