Stop hanging on dead processes and make seeded image edits repeatable - #256
Merged
Merged
Conversation
merceod
requested review from
Gaurav-Shah05,
NSagan271,
stephen-dwq and
vasilevklart
September 16, 2026 06:07
(1) upon stop, the data worker iterated tensor_uuid_to_metadata_per_request, which is only populated when result chunks appear. Replaced it with an explicit set in_flight_requests. (2) has the health check return a 503 when the API server detects that the conductor is down (3) Set up the dynamo integration api server to also check for the dead conductor (4) added a watchdog thread on the conductor that checks api server health (like the worker -> conductor watchdog
| req.error = message | ||
| req.error_status = 503 | ||
| req.event.set() | ||
| if self.on_fatal is not None: |
Collaborator
There was a problem hiding this comment.
If the conductor dies after finalize_setup() starts the message thread but before on_fatal is assigned, the thread records the failure and exits without stopping the server. The later callback assignment does not trigger shutdown
Collaborator
There was a problem hiding this comment.
I think you could move the fatal_error check inside the existing request_lock, alongside the pending request registration? That should closes the race
| since the conductor's per-request seed is derived from | ||
| ``hash(request_id)``); otherwise a fresh uuid4 is generated. | ||
| """ | ||
| if self.fatal_error is not None: |
Collaborator
There was a problem hiding this comment.
submit_request() checks fatal_error before taking request_lock. If the fatal handler runs before this request is registered, it misses the request, and nothing signals its completion
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
3 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.
Two problems on main that came out of the same test runs, fixed together.
A dead worker or conductor hung the server
Nothing watched the process handles, so a worker that failed to initialize or got killed left the server up but unusable, with clients waiting for the 600 s timeout and /health still green, and a killed conductor left orphaned workers holding their GPUs. A dead worker or conductor is now fatal. The conductor and the API server poll their child handles, waiting requests get a 503 that names the dead process, new ones are refused, the remaining processes are torn down and the entrypoint exits 1. Each worker also runs a small watchdog and leaves on its own if the conductor is gone, and the data worker drops its in-flight requests when it stops so nothing is left in /dev/shm. SIGINT stops still exit 0, and SIGTERM still reports 143 as on main (uvicorn re-raises the signal).
Seeded image-to-image was not repeatable
Two runs of /v1/images/edits with the same seed gave the same picture with about one gray level of noise on most pixels, while text-to-image was bitwise. Stage hashes pointed at the VAE encoder, which samples its posterior with randn_like on the global RNG. It keeps sampling, but the noise now comes from the request seed (offset so it is a different stream from the diffusion noise) and is passed in as a tensor input, so torch.compile and CUDA graphs stay out of it.
Tests
13 CPU tests for the liveness paths and 3 for the seeded VAE draw. The existing API server stub in test_request_failure_propagation.py gained two attributes.
Checked on H200s