Repository navigation
fix(model): serialise backend loading and refuse unload mid-generation - #572
Merged
Merged
Conversation
ensure_backend returned early whenever _backend was set, but initialize sets it before load_model finishes, so after an idle unload a second request could start generating on a half-loaded model. A failed load also left that object in place, so later requests kept using it. /dev/unload and /dev/reload took the lifecycle lock but ignored _active_requests and could unload the model under a generator paused between chunks. Always take the lock in ensure_backend, clear _backend when loading fails, and have unload and reload raise ModelBusyError while requests are generating, which the dev routes return as 409.
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
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.
ensure_backend returned early whenever _backend was set, but initialize
sets it before load_model finishes, so after an idle unload a second
request could start generating on a half-loaded model. A failed load
also left that object in place, so later requests kept using it.
/dev/unload and /dev/reload took the lifecycle lock but ignored
_active_requests and could unload the model under a generator paused
between chunks.
Always take the lock in ensure_backend, clear _backend when loading
fails, and have unload and reload raise ModelBusyError while requests
are generating, which the dev routes return as 409.
Testing
pytest: full suite passes.test_model_unload.py: a caller arriving mid-load waits for it, a failed load leaves no backend and the next call retries, and unload/reload refuse (and the routes 409) while a request is generating. The first four fail on master.docker/gpu/Dockerfile.optimizedwith this and the sibling PRs merged withALLOW_DEV_UNLOAD=true:/dev/unloadduring a long generation returns 409 and that generation completes; after an idle unload, three concurrent speech requests all return 200 with no errors in the log.Follows #562-#568 from the same review. Based on master on its own; the only overlap with those PRs is the CHANGELOG
[Unreleased]section.