Give Callers the Ability to set TCP_NODELAY#223
Give Callers the Ability to set TCP_NODELAY#223jcalabro wants to merge 1 commit intotatethurston:mainfrom jcalabro:main
Conversation
|
Thanks @jcalabro, I’ll take a look. Just to make sure I’m understanding your current usage, you’re seeing degraded network performance for server to server communication? |
|
I am having degraded performance on server -> server communication, yeah. Though I just ran a test and it turns out setting TCP_NODELAY made no significant difference in our particular workload. That being said, I would still like to turn this on for all my RPCs. I typically write Go code, which has TCP_NODELAY on by default (some explanation here as to the reasoning), and I'd love to be able to do the same with my node twirp clients. |
|
@jcalabro Thanks for the PR. Identical to golang,
Closing because of this. LMK if you disagree. |
|
Oh interesting, thank you! Apologies, I didn't dig deep enough there. I do agree that we should close this PR. Thank you, apologies again! |
|
No apologies necessary, I appreciate the MR. |
Hey there! I've been using this library and having some performance issues for this specific application and network I'm on. I believe the issue may be due to the fact that TCP_NODELAY is not enabled.
I'm sure that I'm not the only one who may want this feature, which is enabled in node's http library, but not fetch (as far as I can tell)?
This PR also adds type annotations to the
nodeHttpTransportand fixes up the associated linter errors that occurred as a result.It is a bit unfortunate that it's asymmetric in nature (i.e. a caller could set
noDelay: true, but not actually set it if they use thefetchRpcTransport). Thoughts on how to potentially improve that?Thanks! Let me know if there are any questions I can answer!