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.

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

[BugFix] Fix multiprocessing shutdown errors #7041

Closed
wants to merge 6 commits into from
Closed

[BugFix] Fix multiprocessing shutdown errors #7041

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7558b93
[BugFix] Fix multiprocessing shutdown errors
njhill Aug 1, 2024
17ef6cc
Fix linting
njhill Aug 1, 2024
fa76ccc
Add sys.excepthook for multiproc shutdown
njhill Aug 1, 2024
5432d4e
Add comment per @youkaichao suggestion
njhill Aug 1, 2024
4832bd6
Clean up globals in OpenAI server
njhill Aug 1, 2024
5fe1017
Ensure api server is garbage collected at shutdown post-requests
njhill Aug 2, 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
49 changes: 28 additions & 21 deletions vllm/entrypoints/openai/api_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,28 +308,35 @@ async def run_server(args, llm_engine=None, **uvicorn_kwargs) -> None:
logger.info("vLLM API server version %s", VLLM_VERSION)
logger.info("args: %s", args)

server = await build_server(
args,
llm_engine,
**uvicorn_kwargs,
)

loop = asyncio.get_running_loop()

server_task = loop.create_task(server.serve())

def signal_handler() -> None:
# prevents the uvicorn signal handler to exit early
server_task.cancel()

loop.add_signal_handler(signal.SIGINT, signal_handler)
loop.add_signal_handler(signal.SIGTERM, signal_handler)

try:
await server_task
except asyncio.CancelledError:
print("Gracefully stopping http server")
await server.shutdown()
server = await build_server(
args,
llm_engine,
**uvicorn_kwargs,
)

loop = asyncio.get_running_loop()

server_task = loop.create_task(server.serve())

def signal_handler() -> None:
# prevents the uvicorn signal handler to exit early
server_task.cancel()

loop.add_signal_handler(signal.SIGINT, signal_handler)
loop.add_signal_handler(signal.SIGTERM, signal_handler)

try:
await server_task
except asyncio.CancelledError:
print("Gracefully stopping http server")
await server.shutdown()
finally:
# Clean up globals
for var in ("openai_serving_chat", "openai_serving_completion",
"openai_serving_embedding", "openai_serving_tokenization",
"engine_args", "engine"):
globals().pop(var, None)
Comment on lines +338 to +342
Copy link
Member

Choose a reason for hiding this comment

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

does del work? globals().pop is too hacky.

Copy link
Member Author

Choose a reason for hiding this comment

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

@youkaichao we'd have to check and del each one individually so it would be a lot more code. I want this to work whether or not each is defined, in case some error occurs after setting some and not others.

The way globals are used here is already hacky imo and I think we'll clean it up later. I wanted to keep this change as simple as possible as there are overlapping changes in #6883 which will be merged very soon.

Copy link
Member

Choose a reason for hiding this comment

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

got it. this is a minor concern, and you can skip it if it is difficult to solve.

the most important part is still make the ci pass 🙏



if __name__ == "__main__":
Expand Down
Loading