Skip to content

[Frontend] Add readiness and liveness endpoints to OpenAI API server #7078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 18 commits into from
Closed

[Frontend] Add readiness and liveness endpoints to OpenAI API server #7078

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
bac2c90
add readiness and liveness k8s probes for openai api_server
mfournioux Aug 2, 2024
d65bf58
update naming for pydantic classes from openai protocol
mfournioux Aug 2, 2024
0c7945d
update naming for pydantic classes from openai protocol and remove aw…
mfournioux Aug 2, 2024
fa1c549
add tests for readiness and liveness endpoints
mfournioux Aug 2, 2024
2fbaa2f
correct syntax pydantic class in protocol
mfournioux Aug 2, 2024
27ef5ac
correct ruff errors
mfournioux Aug 2, 2024
7fa6a37
correct ruff errors
mfournioux Aug 2, 2024
18a9f2c
fixing isort issues
mfournioux Aug 2, 2024
32e030b
update some typo
mfournioux Aug 2, 2024
5127e91
correct some yapf errors
mfournioux Aug 2, 2024
c698d76
correct readiness probe regarding its http status
mfournioux Aug 5, 2024
ea8be80
replace liveness endpoint by health endpoint and renaming readiness e…
mfournioux Aug 5, 2024
c0baaea
clean some imports and configure error response for readiness endpoint
mfournioux Aug 5, 2024
3a8b227
correct model response in readiness endpoint
mfournioux Aug 5, 2024
ac095c1
add return response 500 for readiness if model weights not loaded
mfournioux Aug 5, 2024
14b2b91
Update test_basic.py
mfournioux Aug 5, 2024
b06d686
update the readiness endpoint with a try clause
mfournioux Aug 6, 2024
e950836
add check if KV cache has been set up in readiness endpoint
mfournioux Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions tests/entrypoints/openai/test_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,22 @@ async def test_log_metrics(client: openai.AsyncOpenAI):
response = requests.get(base_url + "/metrics")

assert response.status_code == HTTPStatus.OK


@pytest.mark.asyncio
async def test_get_readiness_ok(client: openai.AsyncOpenAI):
"""Test the technical route /readiness when the model is fully loaded"""
base_url = str(client.base_url)[:-3].strip("/")

response = requests.get(base_url + "/ready")

assert response.status_code == HTTPStatus.OK

@pytest.mark.asyncio
async def test_get_readiness_ok(client: openai.AsyncOpenAI):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you're going to update this test to check when the server is not ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes exactly, I am working on it

Copy link
Contributor Author

@mfournioux mfournioux Aug 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding readiness, I have worked on create a proper unit test when the server is not ready. I was thinking to test that when the model weight were not loaded or KV cache was not set up, readiness endpoints would return an error message.

But when I have checked how VLLM server was launched, I realized that the endpoints would not be callable until the server was properly deployed and model was loaded with KV cache setup. So I don't see how I can test if the model weights are not loaded or KV cache is not set up, because if these conditions are not reached, the endpoint for readiness will not be callable.

So, do you have any other idea how to do this test?

Is it compulsory to add this test for the PR to be merged?

Copy link
Member

@DarkLight1337 DarkLight1337 Aug 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this would basically defeat the purpose of this PR in terms of fulfilling #6073. If the server cannot accept any requests until everything has been fully loaded, then there is essentially no difference between /ready and /health. Instead, we should enable the /health endpoint to respond before the vLLM engine has finished starting up.

Copy link
Contributor Author

@mfournioux mfournioux Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your point. Furthermore, after having checked the new release 0.5.4, I have noticed several updates have been added on the rpc server to check if it is ready. So, I don't think the readiness endpoint implemented in this PR is still useful.

These are the next possible actions I propose :

  • Close this PR for two reasons :
  1. The readiness endpoint implemented in this PR checks if the model weights are loaded and kv cache set up. An audit of the code shows that the server can not be up until these weights et kv cache are properly loaded. In addition, the 0.5.4 release shows updates which determinate is rpc server is ready. So, there is not point to check these as the health endpoint will do it.
  2. As cited in [Feature]: Add readiness endpoint /ready and return /health earlier (vLLM on Kubernetes) #6073, regarding deployment on k8s, there is a need to implement k8s probes (startup, readiness, liveness) to have an autonomous deployment which will wait for model to be loaded and then marks the pod as ready when the health checkpoint return 200. I think this not directly related to the vllm server, but it is more about implementing proper helm chart which will configure it.
  • I can open a new PR which proposes examples of helm chart for VLLM deployment on k8s, including k8s probes.

Copy link
Contributor

@frittentheke frittentheke Aug 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfournioux thanks for all your work on this feature.

I do have a deployment using a startup probe and liveness checks afterwards.

The main issue I have with the startup probe is that it is a workaround for applications with a potentially long startup unable to communicate their readiness. One never knows how much time a pod needs (downloading model, weights, ...) and in the meantime the liveness cannot be checked.

Switching to a liveness check that is available very early during startup would be nice, but that then requires a readiness indicator to not send traffic until vLLM is ready.

I have not looked at the recent changes yet. But there really should be a way to bring up the webserver (endpoint) early and also to indicate when it's ready.

Additionally I would love for some metrics to also be returned during the initialization phase, allowing for that to be observed.

As for the Helm chat idea, I am thrilled for an official chart, so people don't have to individually write their deployments and figure out how to best configure vLLM and its, checks or also storage /caching. Also good liveness and readiness checks is something that could come with it. I still stand behind the proposal, that vLLM should be a better K8s citizen and prove these endpoints as best as possible.

Copy link
Contributor Author

@mfournioux mfournioux Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frittentheke I have opened a PR #9199 to share a chart helm in order to have an example how to deploy vllm on k8s, including probes configuration.

"""Test the technical route /readiness when the model is fully loaded"""
base_url = str(client.base_url)[:-3].strip("/")

response = requests.get(base_url + "/ready")

assert response.status_code == HTTPStatus.OK
16 changes: 16 additions & 0 deletions vllm/entrypoints/openai/api_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,22 @@ async def health() -> Response:
return Response(status_code=200)


@router.get(
"/ready",
name="readiness",
tags=["technical"],
)
async def get_readiness() -> Response:
"""Readiness probe for k8s"""
d_worker = openai_serving_chat.engine.engine.model_executor.driver_worker
model_weights = d_worker.model_runner.model_memory_usage

if model_weights > 0:
return Response(status_code=200)
else:
return Response(status_code=500)


@router.post("/tokenize")
async def tokenize(request: TokenizeRequest):
generator = await openai_serving_tokenization.create_tokenize(request)
Expand Down
2 changes: 1 addition & 1 deletion vllm/entrypoints/openai/protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,4 +719,4 @@ class DetokenizeRequest(OpenAIBaseModel):


class DetokenizeResponse(OpenAIBaseModel):
prompt: str
prompt: str
Copy link
Member

@DarkLight1337 DarkLight1337 Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid deleting the last line here. (Since otherwise, the file remains unchanged)

Loading