Implement TCP Keep-Alives across most Unix-like systems #12782
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
panjf2000
force-pushed
the
anet-keepalive
branch
3 times, most recently
from
9d856b9
to
ed29b3a
Compare
panjf2000
changed the title
hpatro
reviewed
Nov 20, 2023
hpatro
reviewed
Nov 21, 2023
It seems that this function has never been "verified" in any testable way, just like else functions in cc @oranagra |
zuiderkwast
reviewed
Nov 23, 2023
Actually, it's intended to restructure the code and make it more generic to avoid multiple OS-specific macros, that way we don't have to do something like |
panjf2000
force-pushed
the
anet-keepalive
branch
from
6eaf637
to
55d3d28
Compare
panjf2000
force-pushed
the
anet-keepalive
branch
from
55d3d28
to
aa7f457
Compare
|
Do you have a minute for this PR? |
|
Ping @oranagra |
1 similar comment
|
Ping @oranagra |
|
It's been a month, what is holding this back? I'm so confused with the zero response from the maintainer at present. Is this feature not worth the effort? Or this PR will get reviewed, just need to wait a little longer? I'll take any response @oranagra |
|
@panjf2000 sorry for the late reply. I believe oran must be too busy and missed the notifications. |
Thanks! |
oranagra
approved these changes
Dec 26, 2023
oranagra
approved these changes
Dec 26, 2023
oranagra
added
the
release-notes
oranagra
pushed a commit
that referenced
this pull request
Jan 8, 2024
…Alive (#12914) This is a follow-up PR for #12782, in which we introduced nested preprocessor directives for TCP keep-alive on Solaris and added redundant indentation for code. Besides, it could result in unreachable code due to the lack of `#else` on the latest Solaris 11.4 where `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` are available. As a result, this PR does three main things: - To eliminate the redundant indention for C code in nested preprocessor directives - To add `#else` directives and move `TCP_KEEPALIVE_THRESHOLD` + `TCP_KEEPALIVE_ABORT_THRESHOLD` settings under it, avoid unreachable code and compiler warnings when `#if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT)` is met on Solaris 11.4 - To remove a few trailing whitespace in comments
roggervalf
pushed a commit
to roggervalf/redis
that referenced
this pull request
Feb 11, 2024
## TCP Keep-Alives [TCP Keep-Alives](https://datatracker.ietf.org/doc/html/rfc9293#name-tcp-keep-alives) provides a way to detect whether a TCP connection is alive or dead, which can be useful for reducing system resources by cleaning up dead connections. There is full support of TCP Keep-Alives on Linux and partial support on macOS in `redis` at present. This PR intends to complete the rest. ## Unix-like OS's support `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` are not included in the POSIX standard for `setsockopts`, while these three socket options are widely available on most Unix-like systems and Windows. ### References - [AIX](https://www.ibm.com/support/pages/ibm-aix-tcp-keepalive-probes) - [DragonflyBSD](https://man.dragonflybsd.org/?command=tcp§ion=4) - [FreeBSD](https://www.freebsd.org/cgi/man.cgi?query=tcp) - [HP-UX](https://docstore.mik.ua/manuals/hp-ux/en/B2355-60130/TCP.7P.html) - [illumos](https://illumos.org/man/4P/tcp) - [Linux](https://man7.org/linux/man-pages/man7/tcp.7.html) - [NetBSD](https://man.netbsd.org/NetBSD-8.0/tcp.4) - [Windows](https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-tcp-socket-options) ### Mac OS In earlier versions, macOS only supported setting `TCP_KEEPALIVE` (the equivalent of `TCP_KEEPIDLE` on other platforms), but since macOS 10.8 it has supported `TCP_KEEPINTVL` and `TCP_KEEPCNT`. Check out [this mailing list](https://lists.apple.com/archives/macnetworkprog/2012/Jul/msg00005.html) and [the source code](https://github.com/apple/darwin-xnu/blob/main/bsd/netinet/tcp.h#L215-L230) for more details. ### Solaris Solaris claimed it supported the TCP-Alives mechanism, but `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris until the latest version 11.4. Therefore, we need to simulate the TCP-Alives mechanism on other platforms via `TCP_KEEPALIVE_THRESHOLD` + `TCP_KEEPALIVE_ABORT_THRESHOLD`. - [Solaris 11.3](https://docs.oracle.com/cd/E86824_01/html/E54777/tcp-7p.html) - [Solaris 11.4](https://docs.oracle.com/cd/E88353_01/html/E37851/tcp-4p.html) --------- Co-authored-by: Oran Agra <oran@redislabs.com>
roggervalf
pushed a commit
to roggervalf/redis
that referenced
this pull request
Feb 11, 2024
…Alive (redis#12914) This is a follow-up PR for redis#12782, in which we introduced nested preprocessor directives for TCP keep-alive on Solaris and added redundant indentation for code. Besides, it could result in unreachable code due to the lack of `#else` on the latest Solaris 11.4 where `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` are available. As a result, this PR does three main things: - To eliminate the redundant indention for C code in nested preprocessor directives - To add `#else` directives and move `TCP_KEEPALIVE_THRESHOLD` + `TCP_KEEPALIVE_ABORT_THRESHOLD` settings under it, avoid unreachable code and compiler warnings when `#if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT)` is met on Solaris 11.4 - To remove a few trailing whitespace in comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TCP Keep-Alives
TCP Keep-Alives provides a way to detect whether a TCP connection is alive or dead, which can be useful for reducing system resources by cleaning up dead connections.
There is full support of TCP Keep-Alives on Linux and partial support on macOS in
redisat present.This PR intends to complete the rest.
Unix-like OS's support
TCP_KEEPIDLE,TCP_KEEPINTVL, andTCP_KEEPCNTare not included in the POSIX standard forsetsockopts, while these three socket options are widely available on most Unix-like systems and Windows.References
Mac OS
In earlier versions, macOS only supported setting
TCP_KEEPALIVE(the equivalent ofTCP_KEEPIDLEon other platforms), but since macOS 10.8 it has supportedTCP_KEEPINTVLandTCP_KEEPCNT.Check out this mailing list and the source code for more details.
Solaris
Solaris claimed it supported the TCP-Alives mechanism, but
TCP_KEEPIDLE,TCP_KEEPINTVL, andTCP_KEEPCNTwere not available on Solaris until the latest version 11.4. Therefore, we need to simulate the TCP-Alives mechanism on other platforms viaTCP_KEEPALIVE_THRESHOLD+TCP_KEEPALIVE_ABORT_THRESHOLD.