Skip to content
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.

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

Pass param for pre-created socket to uvicorn #224

Closed
wants to merge 2 commits into from
Closed

Conversation

fialhocoelho
Copy link
Contributor

During the integration tests, the built image triggered the error below:

[rank0]:[W215 15:13:14.568778632 ProcessGroupNCCL.cpp:1496] Warning: WARNING: destroy_process_group() was not called before program exit, which can leak resources. For more info, please see https://pytorch.org/docs/stable/distributed.html#shutdown (function operator())
Traceback (most recent call last):
  File "/opt/vllm/lib64/python3.12/site-packages/vllm_tgis_adapter/http.py", line 66, in run_http_server
    shutdown_coro = await serve_http(app, **serve_kwargs)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: serve_http() missing 1 required positional argument: 'sock'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/opt/vllm/lib64/python3.12/site-packages/vllm_tgis_adapter/__main__.py", line 114, in <module>
    run_and_catch_termination_cause(loop, task)
  File "/opt/vllm/lib64/python3.12/site-packages/vllm_tgis_adapter/__main__.py", line 90, in run_and_catch_termination_cause
    loop.run_until_complete(task)
  File "uvloop/loop.pyx", line 1518, in uvloop.loop.Loop.run_until_complete
  File "/opt/vllm/lib64/python3.12/site-packages/vllm_tgis_adapter/__main__.py", line 80, in start_servers
    raise RuntimeError(f"Failed task={name} ({coro_name})") from exception
RuntimeError: Failed task=http_server (run_http_server)
WARNING: All log messages before absl::InitializeLog() is called are written to STDERR
E0000 00:00:1739632399.252718       1 init.cc:232] grpc_wait_for_shutdown_with_timeout() timed out.

Looking for the function serve_http() in vllm/entrypoints/launcher.py, I found that a recent modification in PR #13113 (post vLLM v0.7.2) added an optional parameter sock to allow uvicorn to use a pre-created socket.

We actually use the serve_http() function in /src/vllm_tgis_adapter/http.py. Because of this, I added a parameter sock=None to maintain the same function signature we used before. I'm not sure if we need to use a pre-created socket for multi-GPU support or for something else.

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.32%. Comparing base (197f101) to head (81deb93).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #224      +/-   ##
==========================================
+ Coverage   63.26%   63.32%   +0.05%     
==========================================
  Files          28       28              
  Lines        1764     1764              
  Branches      218      218              
==========================================
+ Hits         1116     1117       +1     
+ Misses        542      541       -1     
  Partials      106      106              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fialhocoelho fialhocoelho marked this pull request as ready for review February 16, 2025 20:35
@fialhocoelho fialhocoelho added the kind/bug Something isn't working label Feb 16, 2025
Signed-off-by: Jefferson Fialho <jfialho@ibm.com>
Co-authored-by: Daniele <36171005+dtrifiro@users.noreply.github.com>
@fialhocoelho
Copy link
Contributor Author

@dtrifiro I will add your suggestion (editing the current commit) to keep the function backwards compatible with 0.7.2 ASAP

@fialhocoelho fialhocoelho reopened this Feb 21, 2025
@fialhocoelho fialhocoelho marked this pull request as draft February 21, 2025 20:30
@dtrifiro
Copy link
Contributor

Thanks @fialhocoelho, I'm moving this over to #233

@dtrifiro dtrifiro closed this Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants