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

idea: record bank name in priority event #272

Closed
garlick opened this issue Sep 14, 2022 · 9 comments
Closed

idea: record bank name in priority event #272

garlick opened this issue Sep 14, 2022 · 9 comments
Assignees
Labels
idea An idea for a new feature or change

Comments

@garlick
Copy link
Member

garlick commented Sep 14, 2022

Problem: the job eventlog doesn't record the bank a job used when its priority was assigned, but this may be useful information, e.g. see flux-framework/flux-core#4569

Perhaps we could let the multifactor priority plugin record accounting-specific info, like the bank, in the priority event?

@cmoussa1 cmoussa1 added the idea An idea for a new feature or change label Sep 15, 2022
@chu11
Copy link
Member

chu11 commented Sep 15, 2022

alternately, could it be added via a memo event?

@garlick
Copy link
Member Author

garlick commented Sep 15, 2022

Actually, the user can specify flux mini run --setattr bank=NAME, so in that case the bank is already in the jobspec. Maybe a jobspec-update event is the right way for the priority plugin to set the bank to the user's default?

Alternatively, it could be done at ingest by providing a python plugin for the frobnicator (flux-framework/flux-core#4529). That would have the advantage of being able to immediately reject jobs that don't have bank configurations yet instead of holding them in PRIORITY.

Edit: just remembered that the priority plugin could actually still reject the job immediately via the jobtap job.create or job.validate callbacks, so that point is kind of moot.

@cmoussa1
Copy link
Member

I started taking a look at this this morning, thanks for the suggestion on this idea!

Actually, the user can specify flux mini run --setattr bank=NAME, so in that case the bank is already in the jobspec. Maybe a jobspec-update event is the right way for the priority plugin to set the bank to the user's default?

That's right. In the case where a user sets their bank explicitly, the bank is already in the jobspec under attributes.system.bank. When a user wants to use their default bank, however, I think we'll need to set it manually.

Are there any examples in flux-core anywhere of using jobspec-update? A quick GitHub and docs search didn't pull anything up.
:(

@cmoussa1 cmoussa1 self-assigned this Sep 19, 2022
@garlick
Copy link
Member Author

garlick commented Sep 19, 2022

jobspec-update is defined in RFC 21. It is still handled by the job manager. When the event is posted, the job manager's internal copy of the "redacted jobspec" is updated.

The one example we had was the jobspec-defaults jobtap plugin which was removed in favor of the frobnicator in flux-framework/flux-core#4529. The commit that dropped it was flux-framework/flux-core@96f8897

@cmoussa1
Copy link
Member

Ah, okay, I hadn't realized it was removed! Maybe a dumb question, but is it possible to use the either the memo event or the frobnicator in the priority plugin? I think it would be relatively straightforward to find the bank name in the priority event as you mentioned above; I just don't know know how I would set it.

@garlick
Copy link
Member Author

garlick commented Sep 28, 2022

Sorry to leave you hanging on this one. When I read through it last I was kind of on the fence about what to recommend.

Doing this in the frobnicator would require a frobnicator plugin (python) that runs wherever the job is submitted, e.g. login node. I think right now there is no mechanism for such a thing to directly query the accounting database to grab a default bank for the user, correct? You could add an RPC handler in the mf_priority plugin for this I guess. A benefit of this approach is that the jobspec is modified before it ever hits the KVS and tools like flux jobs should have access to the bank without any special handling of jobspec-update events.

OTOH, having mf_priority post a jobspec-update event to assign the bank when it is not already present in the jobspec would be simpler for you to implement I think. Although we're not currently using that event anywhere, it is defined in an RFC and may yet find other use cases (such as extending the duration of a submitted job), so it should be fine to use.

Maybe @grondo or @chu11 have an opinion here?

@grondo
Copy link
Contributor

grondo commented Sep 28, 2022

Maybe @grondo or @chu11 have an opinion here?

The jobspec-update event sounds best to me. Eventually job-list will have to be extended to support these update events (though I haven't considered if it makes sense to add bank to the list of official attributes returned by this module) I don't have any better ideas. Even if a frobnicator plugin was provided by flux-accounting, sysadmins would have to remember to enable it.

@cmoussa1
Copy link
Member

cmoussa1 commented Dec 8, 2022

I think I made some good progress on this over the last couple days. Followed much of the similar process I proposed in #294 and can add a bank key-value pair via jobspec-update when a user submits a job under a default bank. The same goes for a user who submits a job who does not yet exist in the flux-accounting DB (and/or the plugin). When the information for that user gets updated and the previously-held job re-enters job.state.priority, the newly found bank is also added via jobspec-update.

@cmoussa1
Copy link
Member

This should be fixed by #301 now; closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea An idea for a new feature or change
Projects
Development

No branches or pull requests

4 participants