feat core: restore response body streaming for HTTP/2.0#1301
Open
SSE4 wants to merge 3 commits into
Open
Conversation
Body streaming over h2 was disabled (SetStreamBody() hit a UINVARIANT whose throw escaped into std::terminate) after the connection refactor dropped the streaming-event wiring from the connection loop, and because the old serial loop head-of-line-blocked concurrent streams. Restore it on top of the concurrent WaitAnyContext loop: * Http2Connection additionally waits on the session's streaming SingleConsumerEvent (NoAutoReset, reset only at the wakeup point where its awaitable has been dropped out of the context) and drains the event queue into the per-stream chunk buffers, resuming deferred nghttp2 data providers. * The response of a streaming handler is submitted upon the first streaming event of its stream (SetHeadersEnd() always precedes the first PushBodyChunk()), so headers go out early while the handler keeps producing; task completion only finalizes stats/access logs. * Events for streams already reset by the client are dropped instead of tearing down the connection; submit errors fail only that request. * An h2c-upgrade first request with a streaming handler (no stream id at SetStreamBody() time, so it used the HTTP/1.1 queue) degrades to a buffered send instead of hanging on a forever-deferred provider. Head-of-line blocking is gone by construction: handler tasks already run concurrently and NGHTTP2_ERR_DEFERRED starves only the chunk-less stream, which nghttp2 skips while serving the other multiplexed streams. Un-skip the three TAXICOMMON-10258 functional tests and add a head-of-line-blocking guard test (slow streamed response must not delay fast requests on the same connection).
…race New functional tests for the restored h2 streaming: * client reset mid-stream: the handler keeps producing after RST_STREAM; its events must be dropped and the connection must stay usable; * h2c upgrade whose first response is streamed: must degrade to a buffered send, not hang; * flow-control backpressure: a 100 KiB streamed body behind a 1 KiB stream window (and past the connection window) must arrive intact, WINDOW_UPDATE-gated. New unit tests (there were none for the streaming machinery): event queue FIFO/signal contract, WaitAny compatibility of the streaming event, dropped events for unknown streams, and SetStreamBody() picking the h2 producer (previously a process-fatal UINVARIANT). Under TSan the old PushBodyChunk() assert read the response data while the connection coroutine's ExtractData() moved it (the response is now submitted concurrently with the producing handler). Check the data only before the first chunk is announced - after that SetBody() is already impossible and asserts in SetBody() itself.
The concurrent streaming tests used identical payloads on every stream, so bytes leaking between concurrently multiplexed streams (or chunks reordered within one stream) could not be detected. Echo a payload whose every block encodes the stream number and block position, 20 streams multiplexed on one connection, and require byte-exact responses.
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
1 participant
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.
Body streaming (
response-body-stream: true) currently aborts the wholeprocess over HTTP/2:
SetStreamBody()hits aUINVARIANTwhose throw escapesinto a
noexceptcontext. The feature existed (d6f6925), was disabled underTAXICOMMON-10258 because it broke after an update and head-of-line-blocked
concurrent streams, and the HttpReader refactor (f753790) then dropped the
connection-loop wiring while the producer machinery stayed in the tree.
This restores it on top of the current concurrent connection loop:
Http2Connectionadditionally waits on the session's streamingSingleConsumerEventin itsWaitAnyContextand drains the event queue onwakeup, resuming deferred nghttp2 data providers. The event has to be
NoAutoReset(auto-reset is rejected by WaitAny) and may only beReset()right after its wakeup fired, while it has no registered awaiter.
event of its stream —
SetHeadersEnd()always precedes the firstPushBodyChunk(), so headers go out early while the handler keepsproducing. Task completion then only finalizes stats and access logs.
tearing down the connection; submit errors fail only that request. An
h2c-upgrade first request with a streaming handler (no stream id at
SetStreamBody()time) degrades to a buffered send instead of hanging.The original head-of-line blocking doesn't reapply: handler tasks already run
concurrently, and
NGHTTP2_ERR_DEFERREDstarves only the chunk-less streamwhile nghttp2 keeps serving the other multiplexed ones. A new functional test
guards exactly that (a slow ~5s stream must not delay fast requests on the
same connection).
Tests: the three skipped TAXICOMMON-10258 tests are re-enabled and pass, plus
new ones — client reset mid-stream, h2c upgrade with a streamed response,
flow-control backpressure (100 KiB body through a 1 KiB window), and a
byte-exact integrity check with per-stream unique payloads across multiplexed
streams. First unit tests for the streaming machinery are added as well. The
suite is green under ASan/UBSan and TSan (clang + the tsan suppressions
file); TSan caught one debug-only race between
PushBodyChunk()'s assert andthe concurrent response submission, fixed here.
Load-tested with h2load: 150k streamed 8 KiB responses/s (1.15 GB/s) over
8 connections × 100 streams with zero failures, including a run with 4 KiB
flow-control windows forcing constant defer/resume.
closes: #1290