hjwsm1989 opened a new pull request, #3690:
URL: https://github.com/apache/brpc/pull/3690
### What problem does this PR solve?
Issue Number: Related to #3418 / #3430
Problem Summary:
PR #3430 implemented flow-control segmentation and deferred `WINDOW_UPDATE`
flushing for HTTP/2 & gRPC **client requests** (`H2UnsentRequest`). However,
**server responses** (`H2UnsentResponse`) still lacked HTTP/2 flow-control
support:
1. **No stream/connection flow-control check on server responses**: When a
server sends a response body exceeding the peer's stream window
(`sctx->_remote_window_left`) or connection window
(`ctx->_remote_window_left`), `H2UnsentResponse::AppendAndDestroySelf` packs
all `DATA` frames at once, causing standard gRPC/HTTP2 clients (or the server
itself when window runs out) to fail with `FLOW_CONTROL_ERROR`.
2. **Server-side `H2StreamContext` destroyed too early**: On the server
side, `OnEndStream` removes `sctx` from `_pending_streams` as soon as the
request finishes arriving, before the business bthread finishes executing and
sends the response. Without retaining `sctx` in `_pending_streams` until the
response is completely sent, incoming stream-level `WINDOW_UPDATE` frames for
an in-flight response cannot find the stream context.
3. **Deferred gRPC trailer HPACK dynamic table ordering**: When a gRPC
response body is partially deferred due to flow control, the trailing `HEADERS`
(`grpc-status`, `grpc-message`) must be sent only after all `DATA` frames are
flushed. Pre-encoding trailers with the connection's dynamic HPACK table during
`H2UnsentResponse::New` and sending them later upon `WINDOW_UPDATE` would
corrupt the HPACK dynamic table state if other streams encode `HEADERS` in
between.
### What is changed and the side effects?
Changed:
1. **Server-side `H2StreamContext` lifecycle retention (`Stage 1 + Stage
2`)**:
- Keep server-side `H2StreamContext` in `H2Context::_pending_streams`
across `OnEndStream` (`_stream_ended = true`) until the server finishes sending
all response frames (or the stream is reset via `RST_STREAM` /
`ClearPendingStreamData`).
- Protect against premature destruction during concurrent `OnResetStream`
by holding a reference-counted `butil::intrusive_ptr<H2StreamContext>` in
`H2HttpMessage`.
2. **Stateless HPACK encoding for deferred gRPC trailers**:
- Add `HPacker::EncodeWithoutDynamicTable` (`H2HeaderStatelessAppend`
using RFC 7541 static table index when available + literal header field without
indexing `0000xxxx`) so deferred gRPC trailing `HEADERS` never mutate the
connection's HPACK dynamic table out of order.
3. **Flow-controlled segmentation & deferred flushing in
`H2UnsentResponse`**:
- Split response `DATA` frames according to `min(stream_window,
conn_window)` and `max_frame_size`.
- Defer remaining `DATA` and trailing `HEADERS` in
`sctx->_pending_response_data`, accounted under `Socket::_unwritten_bytes` with
overcrowding protection (`FLAGS_socket_max_unwritten_bytes`).
- Resume sending pending server responses upon stream-level or
connection-level `WINDOW_UPDATE` via round-robin `H2Context::FlushPendingData`
(`_last_flushed_stream_id`).
4. **Improved client `RST_STREAM` diagnostics & release-mode `H2Bvars`**:
- Include `stream_id` and `H2ErrorToString(frame_head.stream_id)` in
client-side `OnResetStream` error message.
- Enable `H2Bvars` outside `DEBUG` builds to provide observability into
`h2_stream_context_count` and `h2_unsent_request_count`.
5. **Unit tests**:
- Added unit tests in `test/brpc_h2_unsent_message_unittest.cpp`,
`test/brpc_grpc_protocol_unittest.cpp`, and
`test/brpc_http_rpc_protocol_unittest.cpp` covering response window splitting,
deferred gRPC trailers, cross-stream HPACK isolation, `RST_STREAM` cleanup,
overcrowded buffer protection, round-robin flushing, and end-to-end 4MB gRPC
response under a 64KB initial window.
Side effects:
- Performance effects: Negligible when window is sufficient (fast path sends
headers/data/trailers in one pass without queuing); prevents
`FLOW_CONTROL_ERROR` when responses exceed peer window.
- Breaking backward compatibility: None.
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of
Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
