search

Netflix / dynomite

A generic dynamo implementation for different k-v storage engines
Apache License 2.0
4.2k stars 532 forks source link

added keepalive for dynomite listen connection #692

Closed TysonHeart closed 5 years ago

TysonHeart commented 5 years ago

Issue #691 Enabled keepalive for listen connection of dynomite.

balajivenki commented 5 years ago

@shailesh33 and @ipapapa Can you please review this PR for the ticket #691

smukil commented 5 years ago

@balajivenki Thanks for the contribution. I will have a look at it shortly.

balajivenki commented 5 years ago

@shailesh33 and @ipapapa Can you please review the changes? It will help us if it gets merged in the future releases.

smukil commented 5 years ago

@balajivenki @TysonHeart The code looks good to me and is doing the right thing. One additional thing I would note is whether you want to add an option for TCP_KEEPINTVL. The default on most linux kernels for the interval between 2 keep alive probes is 7200 seconds (2 hours), which I think is a fair interval for the purpose of recycling connections for our use cases. Setting the interval too less can add a lot of load to the server, especially when you're dealing with 50k+ connections, and leaving it too large can leave a lot of un-recycled connections.

If you'd like to add a configurable option for TCP_KEEPINTVL for your use cases, feel free to submit another patch. But this can go in as is for now. Thanks for the contribution again.

http://man7.org/linux/man-pages/man7/tcp.7.html _tcp_keepalive_time` (integer; default: 7200; since Linux 2.2) The number of seconds a connection needs to be idle before TCP begins sending out keep-alive probes. Keep-alives are sent only when the SOKEEPALIVE socket option is enabled. The default value is 7200 seconds (2 hours). An idle connection is terminated after approximately an additional 11 minutes (9 probes an interval of 75 seconds apart) when keep-alive is enabled.

balajivenki commented 5 years ago

@smukil Thanks for merging this. We thought of making it as configurable and decided to honor the values in sysctl.conf

net.ipv4.tcp_keepalive_time net.ipv4.tcp_keepalive_intvl net.ipv4.tcp_keepalive_probes

And we have changed our sysctl.conf to our ideal values. That way there is no confusion and all conf's related to tcp keepalive are at the same place.

murthy-chelankuri commented 5 years ago

Can we merge this change to our latest stable version as well v0.7.0 may be as v0.7.1? We are also facing a similar issue in our prod environment