[Frontend] Fix tcp port reservation for api server #10012
Conversation
PR vllm-project#8537 changed this code to bind the TCP port prior to the engine starting to ensure that port isn't used unexpectedly by something launched during the engine startup. (ray is mentioned in the discussino history and the comment in the code) This change introduced some new unexpected behavior as discussed in issue vllm-project#9737. Restarting vllm within a short time after handling an API request where the client hasn't closed its end of the connection would cause vllm to fail to start with a "port in use" error. The primary issue was the use of the `fd` option to the uvicorn config. This option does not actually do what we want. The relevant code can be found here: https://github.com/encode/uvicorn/blob/fe3910083e3990695bc19c2ef671dd447262ae18/uvicorn/config.py#L496-L501 The important line to note is this one: sock = socket.fromfd(self.fd, socket.AF_UNIX, socket.SOCK_STREAM) Note that `uvicorn` is expecting the fd to be for an `AF_UNIX` socket. We are passing in a TCP (`AF_INET`) socket. We seem to be lucky that this mostly works anyway, though it's surprising it works at all! Instead of using this `fd` option, set the `SO_REUSEADDR` option on the socket, which will allow `uvicorn` to bind to the same address and port. When reserving the port, we previously always specified `""` for the host. I fixed that too so that we bind to the host that we pass to `uvicorn` for it to bind to, as well. Finally, explicitly `close()` the socket as a final step of cleanup. Closes vllm-project#9737 Signed-off-by: Russell Bryant <rbryant@redhat.com>
|
👋 Hi! Thank you for contributing to the vLLM project. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can do one of these:
🚀 |
Thanks for the pointer! The revert was #9852. Hopefully that test can run on this PR so we can check? @khluu |
I marked |
|
Everything seems to be passing 🎉 |
Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: Loc Huynh <jc1da.3011@gmail.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: Sumit Dubey <sumit.dubey2@ibm.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: Maxime Fournioux <55544262+mfournioux@users.noreply.github.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: Tyler Michael Smith <tyler@neuralmagic.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
PR #8537 changed this code to bind the TCP port prior to the engine
starting to ensure that port isn't used unexpectedly by something
launched during the engine startup. (ray is mentioned in the
discussino history and the comment in the code)
This change introduced some new unexpected behavior as discussed in
issue #9737. Restarting vllm within a short time after handling an API
request where the client hasn't closed its end of the connection would
cause vllm to fail to start with a "port in use" error.
The primary issue was the use of the
fdoption to the uvicornconfig. This option does not actually do what we want. The relevant
code can be found here:
https://github.com/encode/uvicorn/blob/fe3910083e3990695bc19c2ef671dd447262ae18/uvicorn/config.py#L496-L501
The important line to note is this one:
Note that
uvicornis expecting the fd to be for anAF_UNIXsocket.We are passing in a TCP (
AF_INET) socket. We seem to be lucky thatthis mostly works anyway, though it's surprising it works at all!
Instead of using this
fdoption, set theSO_REUSEADDRoption onthe socket, which will allow
uvicornto bind to the same address andport.
When reserving the port, we previously always specified
""for thehost. I fixed that too so that we bind to the host that we pass to
uvicornfor it to bind to, as well.Finally, explicitly
close()the socket as a final step of cleanup.Closes #9737
Signed-off-by: Russell Bryant rbryant@redhat.com