transport: add data_frame_budget to client Endpoint - #2834
Draft
LukasKorba wants to merge 1 commit into
Draft
Conversation
## Motivation h2 0.4.18 added a client-side budget (client::Builder::data_frame_budget) that guards a connection against floods of small DATA frames, closing it with GOAWAY(ENHANCE_YOUR_CALM) once the budget is exhausted. This is a correctness fix in h2 (see hyperium/h2 CHANGELOG.md 0.4.16-0.4.18), but its default of 25,600 bytes can be exhausted by well-behaved peers: a server-streaming RPC that emits one small message per frame -- one HTTP/2 DATA frame per item in a long stream -- can trip the guard partway through, even though both sides are individually within the HTTP/2 spec. tonic already exposes sibling h2 client settings on Endpoint (max_frame_size, initial_stream_window_size, http2_header_table_size, ...), but had no way to raise this new budget, so callers hitting the guard had no mitigation short of pinning h2 below 0.4.16. This depends on hyperium/hyper exposing the setting on client::conn::http2::Builder, which it does not yet as of hyper 1.11.0 (see the sibling `expose-data-frame-budget` branch on hyperium/hyper); this change should land after that one merges and releases. ## Solution Add a new `data_frame_budget` field to `Endpoint`, mirroring the `max_frame_size` pattern: - Add `data_frame_budget: Option<usize>` field to `Endpoint` - Add public `pub fn data_frame_budget(self, budget: impl Into<Option<usize>>) -> Self` builder method - Wire the field through to hyper's `Builder::data_frame_budget()` in connection.rs - Include an integration test verifying the setting is accepted and a basic RPC still completes end-to-end (h2's own test suite covers the budget's flood-detection behavior; this test only proves tonic's plumbing) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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.
⚠ Blocked on hyperium/hyper#4164 — draft until it merges and releases.
Motivation
h2 0.4.18 added a client-side budget (client::Builder::data_frame_budget)
that guards a connection against floods of small DATA frames, closing it
with GOAWAY(ENHANCE_YOUR_CALM) once the budget is exhausted. This is a
correctness fix in h2 (see hyperium/h2 CHANGELOG.md 0.4.16-0.4.18), but
its default of 25,600 bytes can be exhausted by well-behaved peers: a
server-streaming RPC that emits one small message per frame -- one
HTTP/2 DATA frame per item in a long stream -- can trip the guard
partway through, even though both sides are individually within the
HTTP/2 spec.
tonic already exposes sibling h2 client settings on Endpoint
(max_frame_size, initial_stream_window_size, http2_header_table_size,
...), but had no way to raise this new budget, so callers hitting the
guard had no mitigation short of pinning h2 below 0.4.16.
This depends on hyperium/hyper exposing the setting on
client::conn::http2::Builder, which it does not yet as of hyper 1.11.0
(see the sibling
expose-data-frame-budgetbranch on hyperium/hyper);this change should land after that one merges and releases.
Solution
Add a new
data_frame_budgetfield toEndpoint, mirroring themax_frame_sizepattern:data_frame_budget: Option<usize>field toEndpointpub fn data_frame_budget(self, budget: impl Into<Option<usize>>) -> Selfbuilder methodBuilder::data_frame_budget()inconnection.rs
basic RPC still completes end-to-end (h2's own test suite covers the
budget's flood-detection behavior; this test only proves tonic's
plumbing)
Evidence
hyper,
cargo test -p tonic --all-featuresis green (212 tests incl. a new doctest), a newintegration test (
tests/data_frame_budget.rs) proves the plumbing end-to-end, and against areal public gRPC server that emits one small DATA frame per message (Zcash lightwalletd,
10,000-message range): default budget trips h2's
too_many_data_framesguard on the releasedstack;
endpoint.data_frame_budget(256_000)completes the same range;data_frame_budget(1024)fails immediately — the knob verifiably drives h2 through the whole chain. Motivating context:
zcash/lightwalletd#593.