Implement Keepalive #145
Implement Keepalive #145
Conversation
caleblloyd
commented
Dec 7, 2016
caleblloyd
commented
- Fixes Implement Keepalive Connection String Options #132
- A value of 0 indicates that the OS Default keepalive settings are used
- On Windows, a value greater than 0 is the idle connection time, measured in seconds, before the first keepalive packet is sent.
- Unix-based Operating Systems will always use the OS Default keepalive settings.
|
I've included a basic test, but I couldn't think of a better way to unit test this since everything happens at the network driver layer. Here's a screenshot of a packet capture running this commit with
As you can see, the keepalive runs every 3 seconds from time 792-810. At time 812, I shutdown the MySql server, and the server sends FIN. The client does not send FIN, so the client believes the connection is still open. The next keepalive packet is not ACK'd, so the client retries keepalives every 1s, which is the proper interval. Eventually 10 keepalives are missed, so the client RST the connection. I think that this demonstrates correct behavior. |
caleblloyd
force-pushed
the
f_keepalive
branch
from
b114911 to
3611699
Compare
| var stopwatch = Stopwatch.StartNew(); | ||
| await Task.Delay(3000); | ||
| stopwatch.Stop(); | ||
| Assert.InRange(stopwatch.ElapsedMilliseconds, 2500, 3500); |
There was a problem hiding this comment.
What is this testing specifically? That no exception is thrown?
There was a problem hiding this comment.
Correct. I guess the stopwatch is not necessary, I'll just remove that
| @@ -148,6 +148,13 @@ These are the other options that MySqlConnector supports. They are set to sensi | |||
| <td>True to have MySqlDataReader.GetValue() and MySqlDataReader.GetDateTime() return DateTime.MinValue for date or datetime columns that have disallowed values.</td> | |||
| </tr> | |||
| <tr> | |||
| <td>Keep Alive, Keepalive</td> | |||
| <td>0</td> | |||
| <td>TCP Keepalive idle time. A value of 0 indicates that the OS Default keepalive settings are used. | |||
There was a problem hiding this comment.
Connector/Net documents "A value of 0 indicates that keepalive is not used."; do we want to be different by default?
| public static void SetKeepalive(Socket socket, uint keepAliveTimeSeconds) | ||
| { | ||
| // Always use the OS Default Keepalive settings | ||
| socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); |
There was a problem hiding this comment.
Following up on question above, should this option only be set if user has set keepAlive != 0?
If you think that the benefits of turning it on by default are worthwhile, then I have no objection. (I'm in favour of changing to use sensible defaults.)
There was a problem hiding this comment.
I think that always enabling it to use the OS Default would be sensible. It allows the OS to clean up old sockets should the connection go away. If data is consistently being sent across the network, Keepalive packets will never need to be sent.
Windows and Linux both default to send the first Keepalive packet after 2 hours of inactivity on the socket.
caleblloyd
force-pushed
the
f_keepalive
branch
from
3611699 to
873dd53
Compare
caleblloyd
force-pushed
the
f_keepalive
branch
from
873dd53 to
07a72b7
Compare
