fix(http): drain SSE stream for connection reuse#790
Open
Conversation
70e959d to
475ddeb
Compare
475ddeb to
1f675de
Compare
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.
Fixes #789
Motivation and Context
Subsequent tool calls via
StreamableHttpClientTransporttook ~42ms each on Linux, roughly 10x slower than the Python SDK. The root cause is twofold: the server kept per-request SSE channels open after sending a response, and the client dropped POST SSE response bodies without consuming them. This left stale connections in the pool that reqwest would stall on (~40ms TCP Delayed ACK) before falling back to a fresh connection.This PR fixes both sides. On the server, request-wise channels now close immediately after sending a Response or Error, and the associated resource_router entries are cleaned up to prevent leaks. On the client, per-request POST SSE streams use a lightweight
raw_sse_to_jsonrpcadapter instead ofSseAutoReconnectStream, andexecute_sse_streamdrains remaining bytes after receiving a response so the connection returns to the pool cleanly. The default reqwest client also setspool_max_idle_per_host(0)because TCP Delayed ACK on Linux still prevents reliable connection reuse even with the drain. Users who provide a custom client viawith_client()are unaffected. Also fixedis_responseto matchErrorin addition toResponse, since JSON-RPC errors are equally terminal for request-wise streams.How Has This Been Tested?
bench.rssomewhere outside the workspace (e.g./tmp/bench/src/main.rs) with aCargo.tomlpointing at the local rmcp crate:On Linux before this fix, calls 2-5 consistently show ~42ms. After the fix, all calls should be under 1ms. The issue does not reproduce on macOS due to different TCP Delayed ACK behavior.
Breaking Changes
None
Types of changes
Checklist
Additional context