Skip to content

fix(transport): make JDK HTTP streaming non-blocking - #2992

Open
LeePui wants to merge 2 commits into
agentscope-ai:mainfrom
LeePui:codex/perf-jdk-http-async-streaming
Open

fix(transport): make JDK HTTP streaming non-blocking#2992
LeePui wants to merge 2 commits into
agentscope-ai:mainfrom
LeePui:codex/perf-jdk-http-async-streaming

Conversation

@LeePui

@LeePui LeePui commented Sep 5, 2026

Copy link
Copy Markdown

AgentScope-Java Version

2.0.3-SNAPSHOT

Description

Closes #2991.

JdkHttpTransport previously parsed SSE/NDJSON with a blocking BufferedReader scheduled on the
shared boundedElastic pool. Every slow stream retained one worker while waiting for the next
network chunk, delaying additional streams and unrelated boundedElastic tasks.

This change:

  • receives streaming bodies with BodyHandlers.ofPublisher();
  • incrementally decodes UTF-8 lines with BodySubscribers.fromLineSubscriber(...);
  • bridges JDK Flow demand and cancellation to Reactor without unbounded request demand;
  • retains the boundedElastic downstream signal boundary without blocking it on network reads;
  • reads non-2xx bodies asynchronously;
  • preserves SSE, NDJSON, timeout, error mapping, and synchronous request behavior; and
  • adds no dependency or public API change.

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, branch codex/test-jdk-http-bounded-elastic:

git clone --single-branch \
  --branch codex/test-jdk-http-bounded-elastic \
  https://github.com/LeePui/agentscope-java.git \
  agentscope-java-jdk-http-repro
cd agentscope-java-jdk-http-repro

mvn -pl agentscope-core \
  -Dtest=JdkHttpTransportBoundedElasticConcurrencyTest \
  -Dagentscope.test.sse.concurrency.enabled=true \
  -Dagentscope.test.sse.requests=40 \
  -Dagentscope.test.sse.chunks=30 \
  -Dagentscope.test.sse.chunkIntervalMillis=1000 \
  -Dreactor.schedulers.defaultBoundedElasticSize=20 \
  -Dreactor.schedulers.defaultBoundedElasticOnVirtualThreads=false \
  -DforkCount=1 -DreuseForks=false test

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:

cd ..
git clone --single-branch \
  --branch codex/perf-jdk-http-async-streaming \
  https://github.com/LeePui/agentscope-java.git \
  agentscope-java-jdk-http-fixed
cd agentscope-java-jdk-http-fixed

mvn -pl agentscope-core \
  -Dtest=JdkHttpTransportBoundedElasticConcurrencyTest \
  -Dagentscope.test.sse.concurrency.enabled=true \
  -Dagentscope.test.sse.requests=40 \
  -Dagentscope.test.sse.chunks=30 \
  -Dagentscope.test.sse.chunkIntervalMillis=1000 \
  -Dreactor.schedulers.defaultBoundedElasticSize=20 \
  -Dreactor.schedulers.defaultBoundedElasticOnVirtualThreads=false \
  -DforkCount=1 -DreuseForks=false test

The fixed branch should complete with BUILD SUCCESS, with all 40 streams receiving their first
chunk in the early window, zero blocked readers, and prompt canary execution.

Before, with a boundedElastic size of 20:

pool=20 requests=40 slowestFirstChunkMs=30182 canaryDelayMs=29076 blockedReaders=20

After:

pool=20 requests=40 slowestFirstChunkMs=1116 canaryDelayMs=1 blockedReaders=0

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-core verification: 2,318 tests, 0 failures, 0 errors, 10 skipped;
  • OpenAI model extension: 555 tests, 0 failures, 0 errors, 11 skipped;
  • Ollama model extension: 164 tests, 0 failures, 0 errors, 1 skipped;
  • Spotless, JaCoCo, JAR, sources, test-JAR, Javadoc, shade/BOM consistency, and
    git diff --check all 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.

  • Code has been formatted with mvn spotless:apply
  • All tests are passing (mvn test) — affected modules pass locally; full repository CI is
    pending
  • Javadoc comments are complete and follow project conventions
  • Related documentation has been updated (not applicable: no user-facing API or configuration
    changes)
  • Code is ready for review

@CLAassistant

CLAassistant commented Sep 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.18919% with 53 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ntscope/core/model/transport/JdkHttpTransport.java 64.18% 36 Missing and 17 partials ⚠️

📢 Thoughts on this report? Let us know!

@CryoThrust CryoThrust left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@CryoThrust

Copy link
Copy Markdown

I also ran the opt-in concurrency regression with 40 SSE requests, 3 chunks at 200 ms intervals, and a boundedElastic size of 20:

Tests run: 1, Failures: 0, Errors: 0
RESULT pool=20 requests=40 fastestFirstChunkMs=290 slowestFirstChunkMs=328 canaryDelayMs=0 signalThreads=20 blockedReaders=0

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.

@CryoThrust

Copy link
Copy Markdown

/check

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.

[Bug]: JdkHttpTransport SSE streams exhaust the shared boundedElastic scheduler

3 participants