Conversation
get_matched_urls() returns None when no node matches the requested model and role, but the RANDOM and MIN_EXPECTED_LATENCY branches unpack its return value directly, raising "TypeError: cannot unpack non-iterable NoneType object" and returning HTTP 500 instead of an "unavailable model" response. Return an empty pair instead so the existing len() == 0 guard is reachable.
This branch has not been deployed
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
NodeManager.get_node_urlcrashes withTypeError: cannot unpack non-iterable NoneType objectwhen no node matches the requested model + role, returning HTTP 500 instead of an "unavailable model" response.Root cause
get_matched_urls()(nested inget_node_url) returnsNonewhen there is no matching node:But both the
RANDOMandMIN_EXPECTED_LATENCYbranches unpack the return value directly:so the
NoneraisesTypeErrorbefore theif len(all_matched_urls) == 0: return Noneguard is ever reached (it is dead code).When this happens
In DistServe (PD-disaggregation) mode, a model can be registered on a node of one role while the engine of the requested role is unavailable (e.g. the Prefill engine is down or removed by the heartbeat check).
check_request_modelstill passes because the model is in the global model list, butget_node_url(model, EngineRole.Prefill)finds no Prefill node and crashes instead of returning an "unavailable model" response.Fix
Make
get_matched_urlsalways return a(urls, speeds)pair (an empty pair instead ofNone), so the existinglen(...) == 0guard becomes reachable andget_node_urlreturnsNoneas intended.Tests
Added
tests/test_lmdeploy/serve/test_proxy.pycovering all three routing strategies for both "no node matches" (returnsNone) and "node matches" (returns the URL).