Repository navigation
pref: Improved _watch_disconnect CPU impact - #315
Merged
Merged
Conversation
FieldMarshallObvious
force-pushed
the
pref/watch_disconnect
branch
from
November 6, 2026 20:10
2ece408 to
e9ad6d3
Compare
FieldMarshallObvious
marked this pull request as ready for review
November 6, 2026 20:21
Contributor
Author
|
/build-pr |
noahpodgurski
approved these changes
Nov 9, 2026
Contributor
Author
|
Performed small scale tests on exa. and mistral in dev to confirm build performs as expected. When doing mistral test experienced some errors on the litellm side, these where errors were not related to the change. MLPA Errors: Dev Load Tests: |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR reduces the CPU cost of
_watch_disconnectinstream_completion. Before, the watcher polledrequest.is_disconnected()every 100 ms for every open stream, so CPU grew with the number of open streams rather than with throughput. Under multiturn load, pods hit their CPU limit while completing very few requests per second._watch_disconnectnow awaitsrequest.receive(), so the task sleeps until the ASGI server delivershttp.disconnectwhen the client disconnects. When a stream finishes normally, stream cleanup cancels the waiting watcher, as before. Polling remains only as a fallback ifreceive()returns an unexpected message type. Additionally I disabled sentrymiddleware_spansas it added cost when switching between middleware, and it is unused.Unit tests where failing under the new code, I changed these tests to more closely match the ASGI behavior expected in actual runtime behavior.
QA
To validate this change I ran some small load tests on my local machine measuring CPU performance with
py-spyin a docker container which mirrors stage. Additionally I ran a small test harness with a mocked litellm, in a docker container, and measure CPU consupmtion of different parts of thecompletions.py.In flame charts on a load test
_watch_disconnectwent from 32.25% to 0.98%Flame Chart Performances
Full flamecharts are attached in a google drive link below, these flame charts show reducing impact of
_watch_disconnecton observed CPU cycles.Watch Disconnect Without Any Changes
Watch Disconnect Disabling Sentry
Watch Disconnect Waiting Instead of Polling
Mocked MLPA Harness
This test was configured with the following setup, and demonstrates the improvement of CPU load when simulating load on MLPA.Setup:
stream_completion.cpu.statover a 15 s window once all streams were open, and is reported as % of one core.HTTPX_MAX_CONNECTIONSwas raised to 1000 so all 500 streams could reach the upstream.Related Tickets & Documents
AIPLAT-1599
22.10.2026 - Stage MLPA - multiturn - sim-gpt
MLPA Flame Charts - 05.11.2025