fix(transport): make JDK HTTP streaming non-blocking - #2992
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
CryoThrust
left a comment
There was a problem hiding this comment.
I reviewed the current diff against the PR base and ran the focused transport suite locally:
JdkHttpTransportTest: 52 tests, 0 failures, 0 errors
The tests cover SSE/NDJSON parsing, split UTF-8 buffers, backpressure, cancellation (including [DONE]), first-chunk/idle/response timeouts, non-2xx bodies, and connection failures. The BodyHandlers.ofPublisher() + fromLineSubscriber approach keeps network waiting out of boundedElastic while preserving a demand-aware cancellation path.
I did not find a blocking correctness issue in the focused review. The full repository CI and the opt-in concurrency regression are the remaining validation gates. One useful follow-up for maintainers is to keep the segment/backpressure contract documented, since this replaces a simple blocking reader with an explicit Flow-to-Reactor bridge.
|
I also ran the opt-in concurrency regression with 40 SSE requests, 3 chunks at 200 ms intervals, and a boundedElastic size of 20: This confirms the main starvation scenario is resolved under the reduced pool size. The remaining Codecov report (64.19% patch coverage) is worth improving for maintainability, but it did not expose a functional failure in the focused or concurrency tests. |
|
/check |
AgentScope-Java Version
2.0.3-SNAPSHOT
Description
Closes #2991.
JdkHttpTransportpreviously parsed SSE/NDJSON with a blockingBufferedReaderscheduled on theshared boundedElastic pool. Every slow stream retained one worker while waiting for the next
network chunk, delaying additional streams and unrelated boundedElastic tasks.
This change:
BodyHandlers.ofPublisher();BodySubscribers.fromLineSubscriber(...);The opt-in mock-model test sends 30 chunks at one-second intervals to 40 concurrent requests.
Reproduction and verification
The standalone
reproduction test
is published in
LeePui/agentscope-java, branchcodex/test-jdk-http-bounded-elastic:This branch is expected to report scheduler starvation and fail its assertion after printing the
result. The fixed implementation can be checked independently from branch
codex/perf-jdk-http-async-streaming:The fixed branch should complete with
BUILD SUCCESS, with all 40 streams receiving their firstchunk in the early window, zero blocked readers, and prompt canary execution.
Before, with a boundedElastic size of 20:
After:
The 40-worker control changed from 40 blocked readers and a 29-second canary delay to zero blocked
readers and immediate canary execution.
Tests cover split UTF-8 byte buffers, CRLF/LF decoding, backpressure,
[DONE]cancellation,first-chunk timeout cancellation, late responses, SSE/NDJSON regression, and OpenAI/Ollama model
streaming integration.
Local validation
Validated on Oracle JDK 25.0.3 with Java sources compiled for
--release 17:agentscope-coreverification: 2,318 tests, 0 failures, 0 errors, 10 skipped;git diff --checkall passed.The repository CI is expected to run the full module and JDK matrix.
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test) — affected modules pass locally; full repository CI ispending
changes)