hyper icon indicating copy to clipboard operation
hyper copied to clipboard
verified publisher icon hyperium

Supports configuration of all TCP Keepalive parameters per https://en.wikipedia.org/wiki/Keepalive

Open hansonchar opened this issue 3 years ago • 4 comments

According to https://en.wikipedia.org/wiki/Keepalive, TCP Keepalive has three parameters:

  • Keepalive time is the duration between two keepalive transmissions in idle condition.
  • Keepalive interval is the duration between two successive keepalive retransmissions, if acknowledgement to the previous keepalive transmission is not received.
  • Keepalive retry is the number of retransmissions to be carried out before declaring that remote end is not available.

Currently, however, only the first parameter Keepalive time is configurable in hyper.

This CR is a proposed enhancement for a backward compatible change on the 0.14.x branch so that all the three TCP Keepalive parameters would become configurable.

FYI per empirical evidence enabling TCP keepalive with intervals and retries drastically/significantly improves the stability of persistent TCP connections while reducing the number of file descriptors via closing down dead connections.

hansonchar avatar Sep 22 '22 07:09 hansonchar

I believe I've fixed all the stylistic issues reported by the workflow, but it doesn't seem I can trigger a re-run of the workflow. I also observed some workflow failures unrelated to this change, so they are probably existing failures.

Please let me know if I missed anything.

If possible, I'd like to have this change pushed/released asap, so I can make further PR to axum-server based on this change.

hansonchar avatar Sep 22 '22 17:09 hansonchar

I see there are platform specific conditional compilations in the socket2 crate causing some failure. Looking into them.

hansonchar avatar Sep 22 '22 21:09 hansonchar

Could just slap #[cfg(unix)] on the extra methods, and not worry about Windows...

seanmonstar avatar Sep 22 '22 22:09 seanmonstar

Could just slap #[cfg(unix)] on the extra methods, and not worry about Windows...

Right, but I am trying to make it such that the same API's can be provided to the upper layer, so that if a API/method exists for a specific parameter that isn't supported on a specific platform, then calling the API/method would just be a no-op. I've just pushed such changes, and added a few unit tests on it. Hopefully they would pass the platform specific tests now.

I used https://docs.rs/socket2/latest/socket2/struct.TcpKeepalive.html as the reference for the platform specific attributes for interval and retries.

hansonchar avatar Sep 22 '22 22:09 hansonchar

Thanks for merging. Would this change also be merged into the master branch, or is it something that needs a separate PR? Please let me know if I can help.

hansonchar avatar Sep 23 '22 22:09 hansonchar

The master branch has removed both Server and AddrIncoming. Helpers (such as Tokio integration) will now live in the hyper-util crate.

seanmonstar avatar Sep 23 '22 22:09 seanmonstar