Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[http2] Add experiment to modify behavior of GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA to throttle pings instead of blocking #36374

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yashykt
Copy link
Member

@yashykt yashykt commented Apr 16, 2024

Implements grpc/proposal#429

Currently, the behavior of GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA blocks more pings from being sent if we are sending too many pings without a data/header frame being sent as well. The original intention of this channel arg was to play nice with proxies that have restrictive settings when it comes to pings. This causes awkwardness when configuring keepalive pings for transports with long lived streams with sparse communication. In such a case, gRPC Core would stop sending keepalive pings since no data/header frame is being sent, resulting in a situation where we are unable to detect whether the transport is alive or not.

This change adds an experiment "max_pings_wo_data_throttle" to modify the behavior of GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA to throttle pings to a frequency of 1 minute instead of completely blocking pings when too many pings have been sent without data/header frames.

@ctiller
Copy link
Member

ctiller commented Apr 16, 2024

Can we put the behavior change behind an experiment?

@yashykt yashykt changed the title [http2] Modify behavior of GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA to throttle pings instead of blocking [http2] Add experiment to modify behavior of GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA to throttle pings instead of blocking Apr 22, 2024
@yashykt yashykt added the release notes: yes Indicates if PR needs to be in release notes label Apr 22, 2024
@yashykt
Copy link
Member Author

yashykt commented Apr 22, 2024

@ctiller done

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.

None yet

2 participants