Skip to content

[Bugfix] (grpc): improve GetServerInfo response consistency and accuracy#33070

Open
pacoxu wants to merge 3 commits intovllm-project:mainfrom
pacoxu:fix/grpc-get-server-info-response
Open

[Bugfix] (grpc): improve GetServerInfo response consistency and accuracy#33070
pacoxu wants to merge 3 commits intovllm-project:mainfrom
pacoxu:fix/grpc-get-server-info-response

Conversation

@pacoxu
Copy link
Copy Markdown
Contributor

@pacoxu pacoxu commented Jan 26, 2026

Purpose

Try to fix 2 TODOs.

  • Use single time.time() for last_receive_timestamp and uptime_seconds to avoid drift between the two values
  • Report actual pause state via self.async_llm.is_paused instead of hardcoded False

Test Plan

grpc server endpoint support was added in #30190.
cc @CatherineSue @njhill

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

- Use single time.time() for last_receive_timestamp and uptime_seconds
  to avoid drift between the two values
- Report actual pause state via self.async_llm.is_paused instead of
  hardcoded False

Signed-off-by: Paco Xu <paco.xu@daocloud.io>
@mergify mergify bot added frontend bug Something isn't working labels Jan 26, 2026
@pacoxu pacoxu force-pushed the fix/grpc-get-server-info-response branch from 74b70e6 to e5a874e Compare January 26, 2026 08:06
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request effectively addresses two outstanding TODOs in the GetServerInfo gRPC endpoint. It significantly improves the consistency and accuracy of the server's reported state by ensuring that last_receive_timestamp and uptime_seconds are derived from a single timestamp capture, thereby preventing potential time drift. Additionally, the server's pause state is now correctly reported using the actual self.async_llm.is_paused status, rather than a hardcoded False value.

@CatherineSue
Copy link
Copy Markdown
Contributor

LGTM

Copy link
Copy Markdown
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

Thanks @pacoxu. I'm guessing last_receive_timestamp is meant to be something different though (ie not just current time). Perhaps you could leave a TODO comment there

…time

last_receive_timestamp is meant to be the time of the last received
request, not the current time; leave a TODO to track it when requests
are received.

Signed-off-by: Paco Xu <paco.xu@daocloud.io>
@pacoxu
Copy link
Copy Markdown
Contributor Author

pacoxu commented Jan 27, 2026

Thanks @pacoxu. I'm guessing last_receive_timestamp is meant to be something different though (ie not just current time). Perhaps you could leave a TODO comment there

Added a comment. Thanks for reviewing.

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) February 20, 2026 05:34
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants