Emit static responses exactly once regardless of subscriber demand shape#636
Merged
slinkydeveloper merged 2 commits intoJul 14, 2026
Merged
Conversation
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
Test Results 29 files + 21 29 suites +21 56s ⏱️ - 2m 36s Results for commit c412475. ± Comparison against base commit a5f770d. This pull request removes 60 and adds 1289 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
nikhiln64
changed the title
… once Two demand-handling fixes surfaced by restatedev#614: 1. HttpRequestFlowAdapter pushed every incoming buffer into an unbounded ArrayDeque regardless of subscriber demand. Pause the request stream on construction and mirror subscriber demand into Vert.x pull-mode fetch(), so buffers are only delivered (and queued) as requested. 2. StaticResponseRequestProcessor (discovery/health responses) re-emitted the whole response body and re-signalled onComplete on every request() call, violating Reactive Streams rules 1.7/2.12. This is currently masked by subscribers requesting Long.MAX_VALUE exactly once: any subscriber using bounded per-item requests duplicates the discovery response until the runtime rejects it as undecodable JSON. Emit exactly once. Verified with the sdk-testing testcontainers suite against a real runtime, plus new unit tests for both classes.
Keep only the StaticResponseRequestProcessor exactly-once fix and its test.
nikhiln64
force-pushed
the
fix/http-vertx-response-backpressure
branch
from
40806ca to
c412475
Compare
Contributor
Author
|
recheck |
Contributor
|
@nikhiln64 can you follow this? #636 (comment) |
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
Contributor
Author
|
recheck |
Contributor
|
this works, great! |
Contributor
|
Thanks for the contribution, we'll release this on the next patch release of the sdk |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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
StaticResponseRequestProcessor(discovery, health responses) re-emits the whole response body and re-signalsonCompleteon everyrequest()call, violating Reactive Streams rules 1.7/2.12.Today this is masked because both current subscribers issue
request(Long.MAX_VALUE)exactly once. The moment any subscriber uses bounded per-item requests, the discovery response gets duplicated hundreds of times — reproduced E2E while prototyping #614: the runtime received a 2.39 MB discovery body (~1000 copies of the JSON) and rejected it withtrailing characters at line 1 column 1180. Fixed with a single-emission guard.Context in #614 (comment) — the response-side backpressure work itself is on hold pending a design decision on
RequestProcessorImpl's output demand semantics.(This PR originally also carried a request-side pause/fetch change in
sdk-http-vertx; reverted per review.)Testing
StaticResponseRequestProcessorTest: exactly-once emission under bounded and unbounded demand, error on non-positive demand.:sdk-testing:test(testcontainers, real runtime) green.Developed with AI assistance (Claude Code), reviewed and driven by a human.