fix(ui): harden Token Router running model and detail views - #5399
fix(ui): harden Token Router running model and detail views#5399m199369309 wants to merge 32 commits into
Conversation
…deepseek-tokenizer-asset # Conflicts: # xinference/core/supervisor.py
…agent-orchestration # Conflicts: # xinference/api/tests/test_token_router_api.py # xinference/core/supervisor.py # xinference/router/app.py
…agent-orchestration
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Token Router' feature to Xinference, enabling token-budget routing between short- and long-context model backends. It adds comprehensive frontend management interfaces, i18n support, RESTful API endpoints, and a backend orchestration controller to manage Router Nodes, Assignments, and Tokenizer Assets. Feedback from the review highlights critical improvements: utilizing Starlette's 'BackgroundTask' to prevent connection leaks during early client disconnects in the streaming proxy, wrapping blocking SQLite operations in 'asyncio.to_thread' to keep the main event loop responsive, and refactoring the monolithic validation in the frontend form dialog to provide more specific and helpful error messages.
| response_headers.setdefault("cache-control", "no-cache") | ||
| response_headers.setdefault("x-accel-buffering", "no") | ||
| return StreamingResponse( | ||
| body_stream(), | ||
| status_code=upstream_response.status_code, | ||
| headers=response_headers, | ||
| ) |
There was a problem hiding this comment.
If the client disconnects before the stream starts iterating, the generator's finally block is never executed, which can leak the upstream_response connection. To prevent this, use a Starlette BackgroundTask to guarantee that upstream_response.aclose() is called upon response completion or early disconnect.
from starlette.background import BackgroundTask
response_headers.setdefault("cache-control", "no-cache")
response_headers.setdefault("x-accel-buffering", "no")
return StreamingResponse(
body_stream(),
status_code=upstream_response.status_code,
headers=response_headers,
background=BackgroundTask(upstream_response.aclose),
)References
- When managing critical concurrency resources in a streaming response, use a Starlette BackgroundTask as an early-disconnect fallback alongside generator finally blocks.
| async def _monitor_token_router_nodes(self) -> None: | ||
| while True: | ||
| try: | ||
| transitions = self._token_router_orchestration.sweep_nodes() |
There was a problem hiding this comment.
sweep_nodes() performs blocking SQLite database operations (such as updating node connectivity status and marking bindings stale). Running this directly on the async event loop can block the loop and cause latency spikes. Use asyncio.to_thread to run it in a separate thread, keeping the event loop responsive.
| transitions = self._token_router_orchestration.sweep_nodes() | |
| transitions = await asyncio.to_thread(self._token_router_orchestration.sweep_nodes) |
| if ( | ||
| typedDraft.backends.length < 1 || | ||
| typedDraft.backends.length > 16 || | ||
| typedDraft.rules.length < 1 || | ||
| typedDraft.rules.length > 64 || | ||
| typedDraft.backends.some( | ||
| (backend) => | ||
| !/^[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}$/.test(backend.id.trim()) || | ||
| !backend.model_uid.trim() || | ||
| backend.max_context_tokens < 1 || | ||
| backend.admission.max_active < 1 || | ||
| backend.admission.max_queue < 0 | ||
| ) || | ||
| new Set(backendIds).size !== backendIds.length || | ||
| typedDraft.rules.some( | ||
| (rule) => | ||
| !/^[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}$/.test(rule.id.trim()) || | ||
| rule.priority < 1 || | ||
| rule.priority > 10000 | ||
| ) || | ||
| new Set(ruleIds).size !== ruleIds.length || | ||
| new Set(priorities).size !== priorities.length || | ||
| invalidRule || | ||
| (typedDraft.defaultAction.type === 'route' | ||
| ? !backendIds.includes(typedDraft.defaultAction.backend_id) | ||
| : !typedDraft.defaultAction.reason.trim()) | ||
| ) { | ||
| toast.error(t('tokenRouter.validation.invalidAdvancedConfig')); | ||
| setSaving(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Instead of a single monolithic if statement with a generic error message, consider performing individual checks and showing specific error messages (e.g., "Duplicate backend IDs", "Duplicate rule priorities", "Invalid token range in rule X", etc.). This will significantly improve the developer and operator experience when configuring complex routing rules.
2 participants
Summary
Dependency
The identifier-label and Tokenizer Asset layout changes from #5393 are intentionally excluded from this PR to avoid duplicate submission.
Validation
git diff --checknpx prettier --check <changed frontend files>npx eslint <changed frontend files>npm run buildpre-commit run --files <changed frontend files>(Python hooks skipped because the change is frontend-only)The build completed successfully. Next.js reported only existing repository warnings in unrelated files.