Skip to content

[Bugfix][Router] Run KvawareRouter /tokenize fallback in executor to avoid blocking event loop - #1069

Open
Asthenia0412 wants to merge 2 commits into
vllm-project:mainfrom
Asthenia0412:fix/kvaware-router-sync-requests
Open

[Bugfix][Router] Run KvawareRouter /tokenize fallback in executor to avoid blocking event loop#1069
Asthenia0412 wants to merge 2 commits into
vllm-project:mainfrom
Asthenia0412:fix/kvaware-router-sync-requests

Conversation

@Asthenia0412

Copy link
Copy Markdown

Description

KvawareRouter.route_request() uses a synchronous requests.post call for the remote /tokenize fallback directly on the event loop. Under load, this blocking call stalls the event loop and can delay health checks and other concurrent requests.

The same pattern was already fixed in tokenize_prompt() by running the blocking HTTP call via asyncio.get_running_loop().run_in_executor(). This PR applies the same fix to the route_request() fallback for consistency.

Change

src/vllm_router/routers/routing_logic.py

# before
body = requests.post(remote_url, headers=headers, json=data, timeout=10).json()
token_ids = body["tokens"]

# after
loop = asyncio.get_running_loop()
response = await loop.run_in_executor(
    None,
    lambda: requests.post(remote_url, headers=headers, json=data, timeout=10),
)
token_ids = response.json()["tokens"]

Testing

  • Syntax verified with ast.parse
  • No behavior change; only moves the blocking call off the event loop

…ng event loop

The remote /tokenize fallback in KvawareRouter.route_request() used a
synchronous requests.post call directly on the event loop. Under load,
this blocks the loop and can stall health checks and other requests.

Mirror the tokenize_prompt() fallback pattern by running the blocking
HTTP call via run_in_executor.

Signed-off-by: Asthenia <asthenia0412@gmail.com>

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request wraps a blocking HTTP POST request in an executor to prevent stalling the event loop in route_request. The reviewer suggests leveraging the shared aiohttp client session instead of running requests.post in an executor, which avoids thread-pool overhead and prevents socket exhaustion through connection reuse.

Comment on lines +405 to +414
# Run the blocking HTTP call in an executor so it does not
# stall the event loop (mirrors tokenize_prompt fallback).
loop = asyncio.get_running_loop()
response = await loop.run_in_executor(
None,
lambda: requests.post(
remote_url, headers=headers, json=data, timeout=10
),
)
token_ids = response.json()["tokens"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since route_request is an asynchronous function and has access to the request object, we can leverage the shared aiohttp client session from request.app.state.aiohttp_client_wrapper() instead of using requests.post in an executor.

Using requests.post without a session creates a new TCP connection for every fallback request, which is highly inefficient under load and can lead to socket exhaustion. Utilizing the shared aiohttp client session enables connection reuse and avoids the overhead of thread-pool execution.

            client = request.app.state.aiohttp_client_wrapper()
            async with client.post(
                remote_url, headers=headers, json=data, timeout=10
            ) as response:
                response_json = await response.json()
                token_ids = response_json["tokens"]

…ze fallback

Per review feedback, replace the executor-wrapped requests.post with the
shared aiohttp client session from request.app.state.aiohttp_client_wrapper().
This reuses TCP connections (avoiding socket exhaustion under load) and
keeps the call off the event loop.

Signed-off-by: Asthenia <asthenia0412@gmail.com>
@Asthenia0412

Copy link
Copy Markdown
Author

Thanks for the review! I updated the fallback to use the shared aiohttp client session from request.app.state.aiohttp_client_wrapper() instead of requests.post in an executor. This reuses TCP connections and keeps the call off the event loop. Could you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant