feat: add health_generate route to openai serving

[dsingal0]

Adds /health_generate for kubernetes to detect when the runtime is hung so it can restart the container/pod even if the server /health returns a 200.
This is helpful in cases when a long context request from a user has caused the llm runtime to hang or die but the fastapi server is still running.

[dsingal0]

@kaiyux wdyt?

[kaiyux]

@kaiyux wdyt?

@dsingal0 Thanks for the suggestion. Some members in the team are on public holidays and will return next week, we will keep you posted.

At first glance I think we should be careful when introducing a new API that is not within OpenAI API scope, and I'll take a closer look on that.

[LinPoly]

For the added entrypoint function, it is acceptable to check if the runtime/executor/engine is alive. But similar logic can also be added to the client code instead of the server side, set a timeout for a simple request, then check if there is any response returned by the server and the response status. Do we have any reason to implement the logic in the server side? @dsingal0

[LinPoly]

FYI: vLLM monitors executor health with daemon thread, it is more structured and reliable for me, but we need to implement similar checking from executor level. @kaiyux

[dsingal0]

@LinPoly

re implementing the check server side vs client side
for deployment in kubernetes we need a health and liveness probe to detect when a pod is healthy and ready to be sent traffic during autoscaling and when its unhealthy and needs to be restarted. There is no client code to add this logic to in that case. Currently trtllm-serve exposes /health but that only checks if the server is up, not the health of the runtime. Would be great to have it at the executor/runtime level if possible

[LinPoly]

@LinPoly

re implementing the check server side vs client side
for deployment in kubernetes we need a health and liveness probe to detect when a pod is healthy and ready to be sent traffic during autoscaling and when its unhealthy and needs to be restarted. There is no client code to add this logic to in that case. Currently trtllm-serve exposes /health but that only checks if the server is up, not the health of the runtime. Would be great to have it at the executor/runtime level if possible

So you mean K8S is using server side health check for error detection and auto-scaling? If so I think it is reasonable to add such a WAR, we can work on a more structured solution afterwards. @kaiyux for opinions.

[Superjomn]

@kaiyux @penli9 , as we have no such check yet, I think a /health_generate is reasonable; we can refine the implementation during further iterations. BTW, we are going to introduce the heartbeat mechanism into the LLM-API level, and that could facilitate the implementation for /health -- it will check and update the status of the system periodically in a cheap way, but I think the /health_generate can always return a real-time status once it is invoked.

[kaiyux]

@dsingal0 Can you also help fix the DCO check? https://github.com/NVIDIA/TensorRT-LLM/pull/3856/checks?check_run_id=41920082141

Summary
Commit sha: 2cd150e, Author: Dhruv Singal, Committer: Dhruv Singal; The sign-off is missing.
Commit sha: a12154a, Author: Dhruv Singal, Committer: Dhruv Singal; The sign-off is missing.
Commit sha: 06f3d21, Author: Dhruv Singal, Committer: Dhruv Singal; The sign-off is missing.

You can follow the guidance here to sign off https://github.com/NVIDIA/TensorRT-LLM/blob/main/CONTRIBUTING.md#signing-your-work.

Please let us know for any questions, thanks!

[dsingal0]

@kaiyux done

[kaiyux]

@dsingal0 Thanks for fixing the DCO check. Mind squashing the commits? Not sure why there were a lot of un-related commits introduced, the diff looks good though.

[dsingal0]

@kaiyux I think rebasing to add signoff added those commits to the PR, can we squash on merge instead? I added the test and removed the mtp.py change.

[dsingal0]

alternatively, I could open a new PR

[kaiyux]

@kaiyux I think rebasing to add signoff added those commits to the PR, can we squash on merge instead? I added the test and removed the mtp.py change.

Thanks a lot! I think we should be fine merge this one since we will squash the commits when merge it, @chzblych do you think differently?

[kaiyux]

/bot run

[tensorrt-cicd]

PR_Github #4694 [ run ] triggered by Bot

[kaiyux]

/bot run

[tensorrt-cicd]

PR_Github #4705 [ run ] triggered by Bot

[tensorrt-cicd]

PR_Github #4694 [ run ] completed with state ABORTED

[tensorrt-cicd]

PR_Github #4705 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3393 completed with status: 'FAILURE'