Skip to content

[chttp2] Bound write sizes based on observed write performance#34665

Merged
ctiller merged 13 commits intogrpc:masterfrom
ctiller:💌
Oct 12, 2023

Hidden character warning

The head ref may contain hidden characters: "\ud83d\udc8c"
Merged

[chttp2] Bound write sizes based on observed write performance#34665
ctiller merged 13 commits intogrpc:masterfrom
ctiller:💌

Conversation

@ctiller
Copy link
Copy Markdown
Member

@ctiller ctiller commented Oct 11, 2023

Instead of fixing a target size for writes, try to adapt it a little to observed bandwidth.

The initial algorithm tries to get large writes within 100-1000ms maximum delay - this range probably wants to be tuned, but let's see.

The hope here is that on slow connections we can not back buffer so much and so when we need to send a ping-ack it's possible without great delay.

@ctiller ctiller requested review from Vignesh2208, ananda1066 and yashykt and removed request for gnossen, jtattermusch and veblush October 11, 2023 23:18
@ctiller ctiller merged commit 7c59c09 into grpc:master Oct 12, 2023
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Oct 13, 2023
ctiller added a commit to ctiller/grpc that referenced this pull request Oct 20, 2023
…34665)

Instead of fixing a target size for writes, try to adapt it a little to
observed bandwidth.

The initial algorithm tries to get large writes within 100-1000ms
maximum delay - this range probably wants to be tuned, but let's see.

The hope here is that on slow connections we can not back buffer so much
and so when we need to send a ping-ack it's possible without great
delay.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bloat/none imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants