sdexec: add background task support - #7773
Open
garlick wants to merge 10 commits into
Open
Conversation
Problem: each sdproc is owned by its exec request message (via message aux data), which couples process lifetime to the request and makes it awkward to keep a process alive after its client goes away. Give the module a zlistx_t of sdproc that owns each process directly. The sdproc holds its own reference to the exec request message rather than being attached to it, and stores its list handle for O(1) removal. Assisted-by: Claude:opus-4.8
Problem: the sdexec module does not implement RFC 42 background exec requests. Treat a non-streaming exec request as a background request. It has the following characteristics: - no stdin channel, subproc input is at end-of-file - stdout and stderr are captured and logged to the broker log, matching the rexec server - a single "started" response is sent The existing "unit fully reaped" logic cleans up foreground and background processes through a single code path. The client is detached: disconnecting no longer kills the unit. Validate flags as the rexec server does: reject write-credit and stdio-fallthrough in background mode, and accept the waitable flag only for a background request (its handler is added in a future commit). Future commits add limited output caching, wait support, etc. Assisted-by: Claude:opus-4.8
Problem: sdexec does not implement the wait RPC, as defined in RFC 42. Add an sdexec.wait handler that returns the exit status and the contents of a limited output buffer of a background process started with the waitable flag, looked up by pid or label. If the process has not yet finished the request is parked and answered when the unit is reaped; a disconnecting client's wait is dropped without losing the status. Assisted-by: Claude:opus-4.8
Problem: the sdexec.kill handler does not accept a label as a target process identifier, as described in RFC 42. Accept an optional label in the kill request and look the process up by label when one is given. This works now: flux sproc kill --service sdexec <signal> <label> Assisted-by: Claude:opus-4.8
Problem: the sdexec.list response hardcodes an empty label and a state of "R" for every process, so "flux sproc ps --service sdexec" cannot show process labels or distinguish a running process from a finished one awaiting a wait request. Report the process label from its command and set the state to "Z" once the unit has finished (retained for a wait) or "R" while it is still running, matching the rexec server's list response. Assisted-by: Claude:opus-4.8
Problem: the sdexec guide describes only streaming execution, so the new background mode, wait handler, and process inspection RPCs are undocumented. Add "Background Execution", "Waitable Processes", and "Inspecting and Signaling Processes" sections to the sdexec guide covering non-streaming exec, broker-log output and exit logging, bounded output retention, the wait request semantics (park, disconnect, and post-start error handling), and the list and kill RPCs. Assisted-by: Claude:opus-4.8
Problem: there is no test coverage for sdexec background mode, the sdexec.wait handler, or the label and state fields now reported by sdexec.list and accepted by sdexec.kill. Add t2421-sdexec-bg.t, gated on user systemd like t2409-sdexec.t. It exercises background exec, wait exit status (zero, nonzero, arbitrary, and signal), wait by pid and by label, parking a wait on a running process, retained stdout/stderr returned by wait, kill by pid and by label, ps label and R/Z state reporting, the not-waitable and not-found error paths, and broker-log capture of a detached process's output. Assisted-by: Claude:opus-4.8
Problem: flux-sproc kill --service=sdexec hangs when successful. kill_cb() attaches the asynchronous KillUnit future to the request message as aux data but does not retain the message. The message dispatcher holds the only reference and destroys it when kill_cb() returns, which runs the aux destructor and tears down the future before its reply arrives, so kill_continuation() never runs. The SIGKILL is still delivered because the KillUnit RPC was already sent, which is why the defect went unnoticed: nothing waited synchronously on the response until now. Append the request to ctx->kills so the request is retained until the continuation can run. Assisted-by: Claude:opus-4.8
Problem: the job-exec "bgexec" backend rejects exec.service=sdexec at initialization, but this should now be functional. Drop the gate and the t2418 assertion that the combination fails cleanly. Assisted-by: Claude:opus-4.8
Problem: nothing exercises method=bgexec combined with exec.service=sdexec, including reattach to running jobs across a job-exec module reload. Add a systemd/dbus/flux-security gated test that runs jobs under bgexec+sdexec and reloads job-exec with single- and multi-node jobs running, verifying they reattach to their transient units rather than relaunch and that a reattached job exits cleanly without a spurious exception. This mirrors the rexec reattach coverage in t2418-job-exec-bgexec.t. Assisted-by: Claude:opus-4.8
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7773 +/- ##
==========================================
- Coverage 83.86% 83.81% -0.05%
==========================================
Files 599 599
Lines 102321 102479 +158
==========================================
+ Hits 85808 85895 +87
- Misses 16513 16584 +71
🚀 New features to boost your workflow:
|
grondo
approved these changes
Aug 13, 2026
grondo
left a comment
grondo
left a comment
Contributor
There was a problem hiding this comment.
This LGTM!
I tried to look into why the coverage is so low here, with codecov showing functions like sdproc_lookup_by{proc,label,client} all appearing uncovered. It must be some kind of codecov report merging issue from the system testing 🤷 I'll try to dig into that further later.
However, I did notice a couple missing tests for these corner cases
- Test that a new client can wait for a bg sdexec task after a previous waiter disconnected.
- Test for "already being waited on" error
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This adds waitable background process support to sdexec and enables the new bgexec job-exec back end to use it.
job-exec tests are added for bgexec+sdexec that demonstrate how jobs persist across a job-exec module reload, similar to the tests for bgexec+rexec.
A future PR will add the ability for sdexec and sdmon to re-adopt running units and then we'll be getting close to the goal of full instance restart with running jobs! But not just yet :-)