Skip to content

[http2] Fix GRPC_ARG_HTTP2_STREAM_LOOKAHEAD_BYTES for when BDP is disabled#39585

Closed
yashykt wants to merge 3 commits intogrpc:masterfrom
yashykt:LookaheadBytes
Closed

[http2] Fix GRPC_ARG_HTTP2_STREAM_LOOKAHEAD_BYTES for when BDP is disabled#39585
yashykt wants to merge 3 commits intogrpc:masterfrom
yashykt:LookaheadBytes

Conversation

@yashykt
Copy link
Copy Markdown
Member

@yashykt yashykt commented May 16, 2025

Fixes b/401598085

GRPC_ARG_HTTP2_STREAM_LOOKAHEAD_BYTES is supposed to influence the initial window size for a stream. This behavior is effectively disabled if BDP is enabled (default on) and we let BDP decide the initial window size.

We have a bug where if BDP is disabled, we use the value provided by GRPC_ARG_HTTP2_STREAM_LOOKAHEAD_BYTES for a very brief period of time. The first settings frame sent out has this overridden value, but we quickly change it back to the default value 65535.

I0000 00:00:1747358562.572890 1361251 frame_settings.cc:224] CHTTP2:SVR:ipv6:%5B::1%5D:33960: got setting ENABLE_PUSH = 0
I0000 00:00:1747358562.572964 1361251 frame_settings.cc:224] CHTTP2:SVR:ipv6:%5B::1%5D:33960: got setting MAX_CONCURRENT_STREAMS = 0
I0000 00:00:1747358562.572989 1361251 frame_settings.cc:224] CHTTP2:SVR:ipv6:%5B::1%5D:33960: got setting INITIAL_WINDOW_SIZE = 1000
I0000 00:00:1747358562.573009 1361251 frame_settings.cc:224] CHTTP2:SVR:ipv6:%5B::1%5D:33960: got setting MAX_HEADER_LIST_SIZE = 16384
I0000 00:00:1747358562.573027 1361251 frame_settings.cc:224] CHTTP2:SVR:ipv6:%5B::1%5D:33960: got setting GRPC_ALLOW_TRUE_BINARY_METADATA = 1
I0000 00:00:1747358562.573636 1361245 frame_settings.cc:224] CHTTP2:SVR:ipv6:%5B::1%5D:33960: got setting INITIAL_WINDOW_SIZE = 65535

The above is a log from an example server. The example client has bdp disabled and stream lookahead bytes set to 1000. We first see that the server receives INITIAL_WINDOW_SIZE = 1000 but then it receives another setting INITIAL_WINDOW_SIZE = 65535 shortly after.

@yashykt yashykt requested a review from ctiller as a code owner May 16, 2025 01:49
@yashykt yashykt added release notes: yes Indicates if PR needs to be in release notes and removed lang/core labels May 16, 2025
@yashykt yashykt requested a review from ananda1066 May 16, 2025 01:49
@yashykt yashykt requested a review from tanvi-jagtap May 20, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants