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

L116: C++-Core: Loosen behavior of GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA #429

Merged
merged 3 commits into from
May 16, 2024

Conversation

yashykt
Copy link
Member

@yashykt yashykt commented Apr 26, 2024

No description provided.

@yashykt
Copy link
Member Author

yashykt commented May 1, 2024

cc @ejona86

@yashykt yashykt requested a review from ejona86 May 1, 2024 18:36
@yashykt yashykt merged commit 4ade47e into grpc:master May 16, 2024
1 check passed
copybara-service bot pushed a commit to grpc/grpc that referenced this pull request May 29, 2024
…_WITHOUT_DATA to throttle pings instead of blocking (#36374)

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.

Closes #36374

COPYBARA_INTEGRATE_REVIEW=#36374 from yashykt:ThrottlePings b5bd42a
PiperOrigin-RevId: 638110795
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants