axum icon indicating copy to clipboard operation
axum copied to clipboard

Feature: `Serve::tcp_nodelay`

Open Clueliss opened this issue 4 months ago • 3 comments

Fixes: #2521

Motivation

See #2521 and linked issue in hyper

Solution

Adds the tcp_nodelay method to Serve (and WithGracefulShutdown) to enable the user to instruct the server to set TCP_NODELAY on incoming connections.

Regarding tests: I wasn't really sure what you would like to have there (since with_graceful_shutdown also doesn't seem to have tests or I didn't find them, not sure), so pointers are appreciated.

Clueliss avatar Mar 15 '24 10:03 Clueliss

Hi, this looks good to me. Can you just add an entry to the changelog please?

mladedav avatar Mar 16 '24 21:03 mladedav

Is TCP_NODELAY ever on by default, such that .tcp_nodelay(false) would make sense to have? I was originally thinking that the method would be parameter-less.

The idea was that there is maybe some configuration or OS where it could be enabled by default that I'm just not aware of. Just wanted to make sure that we don't mess with that default value and users can explicitly disable it.

Again, I personally couldn't give you an example of such an OS or config. I just had https://github.com/hyperium/hyper/issues/3269#issuecomment-1887653439 in mind where it was mentioned that MacOS probably has a slightly different implementation.

("[..] On Mac OS there was no difference in performance between /stream and /regular and this probably has to do with a slight difference in the implementation details or settings tuning of Nagle's algorithm across the two OSes. [..]" - @huntharo)

The comment doesn't imply that its not enabled tho. I can make it parameter-less if you prefer

Clueliss avatar Mar 18 '24 06:03 Clueliss

Hey, I'll try to get to this again soon. Your reasoning for having the bool parameter makes sense, let's keep that.

jplatte avatar Mar 20 '24 07:03 jplatte