job-list: support jobspec-update event #5408
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Actually might be nice to have this merged so the #4844 implementation could be more easily tested (afaik, there's no way to fetch the redacted jobspec via an RPC at this point). So, I'll try to review this and predecessor sooner rather than later. |
Problem: The internal processing of job data uses a list of "state transition" structs to manage the reading/updating of job data throughout a job's lifecycle. Presently, this is the only way that job data can be updated/changed. In the future, job data can be updated through other means such as the "jobspec-update" event. The current "state transition" struct doesn't support these other means. Solution: Refactor the "state transition" structs to be more flexible for other job data update mechanisms. Rename "struct state_transition" to "struct job_update" and add a type field indicating what type of update is occurring. Currently "state transitions" are the only supported update type. Update all functions and related variables for this change. Most notably the "next_states" list is now "updates" and the "st" variable is now "updt" everywhere.
Problem: In the near future we may need to parse the jobspec tasks list multiple times. Even though it can never be altered, it would be parsed multiple times from the jobspec. Cache the jobspec tasks list in the job data structure.
Problem: In the near future, the jobspec attributes dictionary may need to be parsed multiple times. The jobspec attributes parsing currently sits in a function that is called only when the jobspec is initially loaded. Split out the jobspec attributes parsing into a new internal function.
Problem: In the near future we would like to update the jobspec with new information. Add a new helper function job_jobspec_update() to update the jobspec. Also add an optional "updates" object to job_parse_jobspec() that will add those updates to the jobspec that has just been parsed. Add unit tests.
Problem: In the near future jobs may be able to move from one queue to another. There is no way to adjust queue specific stats when this happens. Support a new stats function job_stats_remove_queue(). Along with job_stats_add_queue(), stats can be migrated from one queue to another when the queue changes.
Problem: jobspec-update events will allow users to update the internally viewed jobspec. Job-list does not handle these events and therefore the jobspec data returned by job-list may not represent what is used by the job-manager and other parts of flux. Support the jobspec-update event in job-list. Fixes flux-framework#4404
Problem: There is no coverage to ensure the job-list module handles jobspec-update events. Add coverage in t2260-job-list.t.
chu11
force-pushed
the
issue4404_job_list_jobspec_update
branch
from
858d632
to
a5c6c88
Compare
Codecov Report
@@ Coverage Diff @@
## master #5408 +/- ##
=======================================
Coverage 83.55% 83.55%
=======================================
Files 470 470
Lines 78582 78669 +87
=======================================
+ Hits 65657 65734 +77
- Misses 12925 12935 +10
|
grondo
approved these changes
Aug 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per #4404, built on top of #5407
There were some tricky parts that took me awhile to refactor through, but overall, the commit series shouldn't look too strange.
I didn't list this as WIP as I think this could go in as is. But perhaps we want to wait until work on #4844 is further along so we can test in tandem??