Set TCP_NODELAY #122
Set TCP_NODELAY #122
Conversation
|
Should we enable TCP_NODELAY by default? It seems like more of a footgun than a benefit for mysql connections and there is a solid precedent for it. libmysql appears to set it by default (based on strace), and golang sets it by default for all TCP connections. @byroot do you have any opinions on this? |
|
Makes sense to me to set the flag by default -- although let's ensure that it can be disabled with cc @composerinteralia @jhawthorn @matthewd -- GitHub folks, any opinions here? |
Yes. It's extremely common to set it by default on connections that are expected to be on LAN like a DB connection. I do set it in |
|
I don't think we should have a I don't think the actual protocol would benefit from TCP_NODELAY (it does writes then reads), but it makes sense that with SSL it would. So 👍 from me. I think our code is also well suited to TCP_NODELAY, I believe we always send large constructed buffers (though that is worth double checking that we don't have consecutive writes which could have been batched). |
In that case, it definitely makes sense to omit the option and set it by default - will make things a lot simpler too. |
|
Updated |
|
Here is the effect from this change on our Vitess clients: 
|
| @@ -195,6 +195,14 @@ static int raw_connect_internal(struct trilogy_sock *sock, const struct addrinfo | |||
| return TRILOGY_SYSERR; | |||
| } | |||
|
|
|||
| #ifdef TCP_NODELAY | |||
| if (sock->addr->ai_family != PF_UNIX) { | |||
There was a problem hiding this comment.
Does this make sense?
There was a problem hiding this comment.
Looks good to me! We can't and don't need to apply this to unix sockets.
This PR sets the
TCP_NODELAYflag on sockets.Sometimes, when clients use SSL, Nagle's could cause increased connection latency. We were seeing around 40ms additional latency.