Offload request body stream reads#2273
Closed
pavel-ptashyts wants to merge 8 commits into
Closed
Conversation
InputStream request bodies were still read by the channel event loop through both the HTTP/1 chunked writer and the HTTP/2 body pump. A slow or blocking producer could therefore stall unrelated work on the same loop. Add a client-owned blocking-read executor and have NettyInputStreamBody suspend the event-loop writer while one InputStream read runs off-loop. Completed bytes are copied into ByteBufs back on the event loop before the HTTP/1 or HTTP/2 writer resumes, keeping buffer ownership unchanged. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
Expose request body stream read offload controls on the client config so callers can disable the worker pool or tune its worker count and queue capacity. Keep the default behavior enabled and preserve automatic thread and queue sizing when count or queue values are not configured. Add focused tests for property defaults, custom worker naming, and disabled inline event-loop reads. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
Preserve the original synchronous stream writers when request body read offload is disabled, and remove an unnecessary event-loop scheduling hop from the enabled path. Convert unchecked InputStream failures into IOExceptions so suspended HTTP/1 and HTTP/2 uploads fail instead of hanging. Propagate the original HTTP/2 streaming failure, make close visibility explicit, and update the stale InputStream behavior expectation. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
Guard the HTTP/2 streaming pump against synchronous writability callbacks fired from flush, and keep it parked while the terminal DATA frame is pending. This prevents concurrent pump state mutation and duplicate endStream frames under constrained flow control. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
Keep request body stream read offloading opt-in so existing clients retain inline reads unless the worker pool is explicitly enabled. Update the HTTP/1.1 and HTTP/2 tests to distinguish default behavior from the enabled offload path. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
Keep the invalid InputStream regression focused on the new body-read implementation after making that implementation opt-in. A default client correctly retains the legacy response behavior and should not be used for this assertion. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
Skip worker reads when cancellation closes a body stream before its queued task starts. This prevents closed streams from being touched and keeps cancelled uploads from consuming executor capacity. Add deterministic HTTP/1.1 and HTTP/2 queue tests, plus an HTTP/2 sibling stream test that proves a blocked upload does not stall another stream. Document executor-taking constructors as internal client wiring. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
pavel-ptashyts
force-pushed
the
perf/offload-body-stream-reads
branch
from
July 20, 2026 15:08
bb6c48f to
6f51656
Compare
Reserve a second queue slot so the active upload can submit its EOF read while the cancelled task is still being drained. This keeps the tests focused on preventing read-after-close behavior instead of exercising queue saturation. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
pavel-ptashyts
force-pushed
the
perf/offload-body-stream-reads
branch
from
July 20, 2026 15:52
fa4eddc to
b3664b9
Compare
Contributor
Author
|
Closing by decision; performance-plan item 1.2 will be marked skipped. |
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.
Summary
InputStreamrequest body reads from Netty event-loop threads for HTTP/1.1 and HTTP/2 uploads when enabledConfiguration
requestBodyStreamReadOffloadEnableddefaults tofalseand must be enabled explicitlyrequestBodyStreamReadThreadsCountdefaults to-1, which uses the configured I/O thread countrequestBodyStreamReadQueueSizedefaults to0, which selects an automatic bounded capacityMotivation
Blocking
InputStream.read(...)calls can stall Netty event-loop threads while streaming request bodies. Under HTTP/2, one blocked upload can also delay sibling streams on the same connection. The feature is opt-in so existing clients retain inline stream reads. When enabled, it keeps stream reads away from the event loop and exposes controls for tuning the worker pool.The hardened implementation avoids an extra event-loop scheduling hop, makes worker-thread close visibility explicit, and ensures runtime stream failures and cancellation cannot leave an upload suspended indefinitely. A queued task checks cancellation before invoking
InputStream.read, preventing a read-after-close race.A matrix run exposed a flow-control race:
flush()can synchronously firechannelWritabilityChanged, re-entering the pump, and another callback can arrive while the terminal write is pending. Explicit pump and terminal-write states prevent duplicateendStreamframes.The executor-aware support constructors are public only because client wiring crosses package boundaries. Applications should enable and tune the feature through
AsyncHttpClientConfig.Validation
./mvnw -pl client -am -Dtest='BasicHttpTest#cancelledQueuedInputStreamIsNotReadOverHttp1,Http2StreamingBodyFlowControlTest#blockedInputStreamDoesNotStallSiblingStreamOverHttp2+cancelledQueuedInputStreamIsNotReadOverHttp2' -Dsurefire.failIfNoSpecifiedTests=false test./mvnw -pl client -Dtest=org.asynchttpclient.BasicHttpTest#postInputStreamRuntimeFailureFailsRequest,org.asynchttpclient.Http2StreamingBodyFlowControlTest#inputStreamRuntimeFailureFailsRequestOverHttp2,org.asynchttpclient.request.body.InputStreamTest#testInvalidInputStream test./mvnw -pl client -Dtest='AsyncHttpClientDefaultsTest#testDefaultRequestBodyStreamReadOffloadEnabled,BasicHttpTest#postInputStreamBodyGeneratorReadsOffEventLoop+postInputStreamBodyGeneratorReadsOnEventLoopByDefault+postInputStreamRuntimeFailureFailsRequest,Http2StreamingBodyFlowControlTest#inputStreamBodyReadsOffEventLoopOverHttp2+inputStreamRuntimeFailureFailsRequestOverHttp2+largeUploadCompletesUnderConstrainedFlowControlWindow' test./mvnw -pl client -Dtest='org.asynchttpclient.Http2StreamingBodyFlowControlTest#largeUploadCompletesUnderConstrainedFlowControlWindow' test(3 consecutive passes)./mvnw -pl client -Dtest='InputStreamTest,AsyncHttpClientDefaultsTest#testDefaultRequestBodyStreamReadOffloadEnabled' test./mvnw -pl client -DskipTests -Dgpg.skip=true verifyAttribution
Codex on behalf of Pavel Ptashyts