job-list: initialize queue stats #5712
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #5712 +/- ##
===========================================
+ Coverage 55.38% 83.46% +28.08%
===========================================
Files 439 486 +47
Lines 75169 82933 +7764
===========================================
+ Hits 41631 69219 +27588
+ Misses 33538 13714 -19824
|
chu11
force-pushed
the
issue5688_job_list_missing_queue_stats
branch
from
4e0bef5 to
613eee0
Compare
grondo
approved these changes
Feb 1, 2024
| @@ -45,18 +45,19 @@ static void free_wrapper (void **item) | |||
| } | |||
|
|
|||
| static struct job_stats *queue_stats_lookup (struct job_stats_ctx *statsctx, | |||
| struct job *job) | |||
There was a problem hiding this comment.
Commit message: looksup -> looks up
Also, 3 spaces between the period and "In a few cases..."
Problem: The internal queue_stats_lookup() function looks up a stats structure for a queue and creates one if it is missing. But in a few cases, this makes no sense. For example, when a job is being purged, it should be (generally) impossible that queue stats don't exist. Add a boolean indicating if the queue stats should be created or not in queue_stats_lookup(). In a few cases where it's undesireable for queue_stats_lookup() to return no queue stats, output some debugging error info. Also, have queue_stats_lookup() take a queue name instead of a "struct job" as input, as the queue name is what is actually necessary.
Problem: Job queue stats are generated in job-list when a job is submitted into it. If a job queue never has a job submitted to it, no job stats exist for the queue. This isn't what users would expect, they should expect queue stats to be 0 for the queue. Solution: Read the flux config and initialize queue stats to 0 for any queues that have been configured. Fixes flux-framework#5688
Problem: A test in t2260-job-list.t assumes there will be no job queue stats if a job has never been submitted to it. That behavior was recently changed. Update the test to check for non-empty queue stats for queues that should have no jobs "submitted" to it. Also add a new "nosubmitqueue" to the test for additional coverage.
Problem: CodeQL outputs a warning that an except clause is empty without a comment. Add a comment to eliminate the warning.
chu11
force-pushed
the
issue5688_job_list_missing_queue_stats
branch
from
e9ad433 to
7958552
Compare
|
thanks! fixed, rebased, and setting MWP |
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.
Problem: Job queue stats are generated in job-list when a job is submitted into it. If a job queue never has a job submitted to it, no job stats exist for the queue. This isn't what users would expect, they should expect queue stats to be 0 for the queue.
Solution: Read the flux config and initialize queue stats to 0 for any queues that have been configured.
Fixes #5688
Uhhh I clearly messed up my branch with rebasing somehow ... will fix
I'm not sure why a CodeQL warning showed up in this PR. I figured the "not a warning" in original PR was enough, but I guess not?? Or maybe b/c it was due to my messed up rebasing earlier. Anyways, I just added a tiny fix to fix it.
Oh yeah, side note. Implementation may look a little weird. Why have config_reload() call down into stats.[ch], why not put everything in stats.c? I needed config reload setup for another PR, so decided to just solve this issue first along the way.