[Serve] Expose replica and controller urls on load balancer #3351
Conversation
| async def _get_urls(self, request: fastapi.Request): | ||
| del request # Unused | ||
|
|
||
| ready_replica_urls = self._load_balancing_policy.ready_replicas |
There was a problem hiding this comment.
Maybe we should cache this in this SkyServeLoadBalancer class whenever we've finished a sync. Seems appropriate.
Alternatively, we should make abstract class LoadBalancingPolicy expose a get_ready_replicas() and use it here? Subclasses may want to have R/W locks to protect this field.
| @@ -91,7 +91,21 @@ async def _redirect_handler(self, request: fastapi.Request): | |||
| logger.info(f'Redirecting request to {path}') | |||
| return fastapi.responses.RedirectResponse(url=path) | |||
|
|
|||
| async def _get_urls(self, request: fastapi.Request): | |||
There was a problem hiding this comment.
nit: how about _get_ready_replicas?
nit: return type
| ready_replica_urls = self._load_balancing_policy.ready_replicas | ||
| for i, ready_replica_url in enumerate(ready_replica_urls): | ||
| if not ready_replica_url.startswith('http'): | ||
| ready_replica_url = 'https://' + ready_replica_url |
There was a problem hiding this comment.
Why do we want to prepend this? E.g., Caddy / Prometheus's target lists do not require this.
| ready_replica_url = 'https://' + ready_replica_url | ||
| ready_replica_urls[i] = ready_replica_url | ||
| return fastapi.responses.JSONResponse(content={ | ||
| 'controller': self._controller_url, |
There was a problem hiding this comment.
Oh I see the reason for this naming now. Shall we separate them out?
|
This PR is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR was closed because it has been stalled for 10 days with no activity. |
We now expose replica URLs from the load balancer for better metrics support.
Future TODO:
Tested (run the relevant ones):
bash format.shsky serve up -n test examples/serve/http_server/task.yamlcurl $(sky serve status --endpoint test)/-/urlspytest tests/test_smoke.pypytest tests/test_smoke.py::test_fill_in_the_namebash tests/backward_comaptibility_tests.sh