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

feat(transport): Enable TCP_NODELAY. #120

Merged
merged 1 commit into from
Nov 6, 2019
Merged

feat(transport): Enable TCP_NODELAY. #120

merged 1 commit into from
Nov 6, 2019

Conversation

dominiquelefevre
Copy link
Contributor

The combination of the Nagle algorithm on a client, and delayed ack on a server
can introduce up to 200ms latency if a small gRPC messages is sent with multiple
writes.

Enable TCP_NODELAY to make sure that neither client, nor server buffer their
messages for too long when exchaning small requests.

The combination of the Nagle algorithm on a client, and delayed ack on a server
can introduce up to 200ms latency if a small gRPC messages is sent with multiple
writes.

Enable TCP_NODELAY to make sure that neither client, nor server buffer their
messages for too long when exchaning small requests.
@LucioFranco
Copy link
Member

@dominiquelefevre I think this is a good idea to add but I'd like to look into what other grpc libraries do as defaults. I am curious if grpc-go's http2 impl sets nodelay by default, same with netty?

@dominiquelefevre
Copy link
Contributor Author

grpc-go sets nodelay by default, and I do not know of any way to disable that.

@LucioFranco
Copy link
Member

@dominiquelefevre ok that makes sense, I am happy to merge this but maybe in a follow up PR we should probably provide this as a config option as well.

@LucioFranco LucioFranco merged commit 0299509 into hyperium:master Nov 6, 2019
@dominiquelefevre
Copy link
Contributor Author

@LucioFranco, I am opposed to a config option.

TCP_NODELAY is required for services that exchange small requests. If requests are big, sending one more incomplete TCP packet with a http request prologue introduces no noticeable overhead, thus it makes no sense to disable TCP_NODELAY in such scenarios. So, just keep the API minimal.

@LucioFranco
Copy link
Member

@dominiquelefevre no I agree, what I mean't was to enable it by default and let users disable it. Not a high priority and we don't have to do anything until someone needs it. Thanks for all the help! :)

rabbitinspace pushed a commit to satelit-project/tonic that referenced this pull request Jan 1, 2020
The combination of the Nagle algorithm on a client, and delayed ack on a server
can introduce up to 200ms latency if a small gRPC messages is sent with multiple
writes.

Enable TCP_NODELAY to make sure that neither client, nor server buffer their
messages for too long when exchaning small requests.
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.

2 participants