job-manager: drop sched.free response requirement #5786
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
Problem: `flux queue status -v` shows a count of pending sched.free requests, but once sched.free no longer requires a response, they will never be "pending". Drop pending free requests from the output. Update sharness tests that expected that in the output. Drop `free_pending` from the job-manager.alloc-query response payload.
grondo
approved these changes
Mar 11, 2024
garlick
added a commit
to garlick/flux-rfc
that referenced
this pull request
Mar 11, 2024
Problem: sched.free requires a response, but this adds complexity to the job manager and has little benefit. Code that drops the sched.free response from flux-core is proposed in flux-framework/flux-core#5786. Drop the free response.
|
Oof just noticed a typo - an extra log message that shouldn't be there. Will force push. Before this, a free error would cause the scheduler interface to be torn down (scheduler stopped). Now we just leave it to schedulers to log something. I think that is probably fine - the scheduler can choose to abort if it wants to, or log. (They both currently do log at LOG_ERR level anyway). |
Problem: the sched.free RPC requires a response but the response is unnecessary for correct job management. Moreover, the coupling between the RPC response and the job state machine makes some anticipated changes harder to design. Make sched.free a "fire and forget" RPC. Simply post the free event immediately instead of deferring it until the response is received. Update libschedutil to make schedutil_free_respond() a no-op. Update sched-simple to not directly respond when there is a free error. This change should be accompanied by an update to RFC 27.
garlick
force-pushed
the
sched_free_noresponse
branch
from
c1d21db
to
a1abce4
Compare
|
Thanks - I'll set MWP. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #5786 +/- ##
===========================================
+ Coverage 54.85% 83.32% +28.47%
===========================================
Files 461 509 +48
Lines 74808 82479 +7671
===========================================
+ Hits 41033 68724 +27691
+ Misses 33775 13755 -20020
|
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: the
sched.freeRPC requires a response but the response is unnecessary for correct job management.Moreover, the coupling between the RPC response and the job state machine makes some anticipated changes harder to design.
Make
sched.freea "fire and forget" RPC. Simply post thefreeevent immediately instead of deferring it until the response is received..Update
flux queue status -voutput to not show pending free rpcs, since they can no longer be pending.This change should be accompanied by an update to RFC 27.