Repository navigation
fix(worker): pop launch_ts in recover_model to avoid strict-constructor crashes - #5012
Merged
qinxuye merged 1 commit intoJun 9, 2026
Merged
Conversation
…or crashes launch_ts is an internal timestamp stamped onto the launch snapshot at the entry of launch_builtin_model. recover_model splats the whole snapshot back via launch_builtin_model(**launch_args), which injects launch_ts into the model's self._kwargs. Models that forward the full self._kwargs into a strict constructor (e.g. jina-reranker-v3 -> AutoModelForCausalLM.from_pretrained) then crash with TypeError: unexpected keyword argument 'launch_ts'. The cross-session recovery path (recover_models_on_startup at line 788) already pops launch_ts before relaunch. This commit applies the same cleanup to the in-process recovery path (recover_model), copying the dict first to avoid mutating the cached snapshot in self._model_uid_to_launch_args. Fixes OOM auto-recovery for strict-constructor models. Without this fix, recover_sub_pool detects the crash and triggers recover_model, but the recreated model fails to load, leaving the replica permanently down. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the internal timestamp launch_ts was passed to model constructors during recovery, causing a TypeError in models with strict constructors. The fix ensures launch_args is copied and launch_ts is popped before launching the model. A unit test has been added to verify this fix. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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.
Summary
recover_sub_pool→recover_model) crashes strict-constructor models (e.g., jina-reranker-v3) withTypeError: unexpected keyword argument 'launch_ts'. The bug is thatrecover_modelsplats the cached launch snapshot (which includeslaunch_ts, an internal timestamp) back intolaunch_builtin_model(**launch_args), injectinglaunch_tsinto the model'sself._kwargs. Models that forward the fullself._kwargsinto a strict constructor (likeAutoModelForCausalLM.from_pretrained(**model_kwargs)) then crash.launch_tsfromlaunch_argsinrecover_modelbefore the splat, mirroring the existing cleanup inrecover_models_on_startup(line 788). Copy the dict first to avoid mutating the cached snapshot inself._model_uid_to_launch_args(which legitimately useslaunch_tsascreated_ts).recover_sub_pooldetects the crash and triggers recovery.Root cause analysis (from production logs)
Why embedding v3/v4 survived: They use
.get()to selectively extract kwargs (not a full splat), solaunch_tswas never passed to the constructor. Only models that forward the entireself._kwargsvia**are affected.Test plan
test_recover_model_pops_launch_ts_from_kwargsinxinference/core/tests/test_worker.py— verifieslaunch_tsis removed from kwargs passed tolaunch_builtin_model, and that the original cached snapshot still containslaunch_ts(copy isolation).XINFERENCE_MODEL_ACTOR_AUTO_RECOVER_LIMIT=2, trigger OOM, verify auto-recovery completes within 30s (should seeModelActor loadedinstead ofTypeError).self._kwargs).Related PRs
monitor_sub_poolsruns sorecover_sub_pooldetects crashes.os._exit(1)so OOM crashes are detectable.launch_tsleakage.Blast radius
recover_modelonly. Does not affectrecover_models_on_startup(already had the cleanup),launch_builtin_model(no changes), or model-side code.self._model_uid_to_launch_argsis unchanged (copy isolation), so any code that readslaunch_tsfrom the cache (e.g.,created_tscomputation) is unaffected.recover_models_on_startuptorecover_model.🤖 Generated with Claude Code