feat: sync up-to-date health check information with AppProxy #5230
Conversation
…sion status update
github-actions
bot
added
size:M
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the model service health check synchronization with AppProxy by updating session status transition logic to provide real-time health check information. The changes ensure that AppProxy receives up-to-date health check configurations whenever session statuses change.
Key changes include:
- Enhanced health check information storage in Redis with separate enabled flag and configuration
- Updated session filtering to only include RUNNING sessions in route generation
- Consolidated Redis operations for both connection info and health check data
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/ai/backend/manager/registry.py | Updated to use new consolidated Redis storage method and improved variable naming |
| src/ai/backend/manager/models/endpoint.py | Added session status filtering to only include RUNNING sessions in active routes |
| src/ai/backend/common/clients/valkey_client/valkey_live/client.py | Added new method to atomically update both connection info and health check data in Redis |
| changes/.feat.md | Added changelog entry for the feature |
| @@ -543,7 +544,9 @@ async def generate_redis_route_info( | |||
| [ | |||
| r.session | |||
| for r in active_routes | |||
| if r.status in RouteStatus.active_route_statuses() and r.session | |||
| if r.status in RouteStatus.active_route_statuses() | |||
| and r.session | |||
There was a problem hiding this comment.
The code accesses r.session_row.status but there's no guarantee that session_row is loaded or available on the route object r. This could result in an AttributeError if the session relationship isn't properly loaded.
| and r.session | |
| and r.session | |
| and r.session_row |
Copilot uses AI. Check for mistakes.
| await self._client.client.set( | ||
| f"endpoint.{endpoint_id}.route_connection_info", | ||
| json.dumps(connection_info), | ||
| expiry=ExpirySet(ExpiryType.SEC, 3600), | ||
| ) | ||
| await self._client.client.set( | ||
| f"endpoint.{endpoint_id}.health_check_enabled", | ||
| "true" if health_check_config is not None else "false", | ||
| expiry=ExpirySet(ExpiryType.SEC, 3600), | ||
| ) | ||
| if health_check_config: | ||
| await self._client.client.set( | ||
| f"endpoint.{endpoint_id}.health_check_config", | ||
| health_check_config.model_dump_json(), | ||
| expiry=ExpirySet(ExpiryType.SEC, 3600), | ||
| ) |
There was a problem hiding this comment.
This block was originally written in pipeline but I just unwrapped it because there seemed to be no alternative option available for valkey... But I guess batch would fit. Thanks for the heads up.
Follow-up PR of #5134. This PR updates session status transition logic to generate up-to-date health check information upon model service's target session status updates.