mysql_async
mysql_async copied to clipboard
TCP keepalive definition may be better expressed in seconds
Hello, found maybe a little mistake in how tcp_keepalive
see OptsBuilder::tcp_keepalive() is defined in the lib. Thanks for the lib by the way.
By convention keepalive tcp parameters are seconds, in mysql_async
it's currently defined from_millis()
.
https://github.com/blackbeam/mysql_async/blob/master/src/conn/mod.rs#L913
} else {
let keepalive = opts
.tcp_keepalive()
.map(|x| std::time::Duration::from_millis(x.into()));
Stream::connect_tcp(opts.hostport_or_url(), keepalive).await?
};
From socket2 lib source-code it's expected in seconds (not explicitly said in doc but readable in code).
#[allow(unused_variables)]
pub(crate) fn set_tcp_keepalive(fd: Socket, keepalive: &TcpKeepalive) -> io::Result<()> {
#[cfg(not(any(target_os = "haiku", target_os = "openbsd", target_os = "nto")))]
if let Some(time) = keepalive.time {
let secs = into_secs(time);
unsafe { setsockopt(fd, libc::IPPROTO_TCP, KEEPALIVE_TIME, secs)? }
}
And From man 7 tcp
man7.org tcp or linux.die.org man 7 tcp
TCP_KEEPCNT (since Linux 2.4)
The maximum number of keepalive probes TCP should send before dropping the connection. This option should not be used in code intended to be portable.
TCP_KEEPIDLE (since Linux 2.4)
The time (in seconds) the connection needs to remain idle before TCP starts sending keepalive probes, if the socket option SO_KEEPALIVE has been set on this socket. This option should not be used in code intended to be portable.
TCP_KEEPINTVL (since Linux 2.4)
The time (in seconds) between individual keepalive probes. This option should not be used in code intended to be portable.
Warning: it may be a breaking change for users who expected milliseconds.
If i am wrong don't hesitate to correct me. :)
Hi. This is the reason, I believe. I'll consider changing this to seconds or at least adding a note in the docs.
Thanks for the reply, it's more clear!
(from tokio doc you linked)
If None is specified then keepalive messages are disabled, otherwise the number of milliseconds specified will be the time to remain idle before sending a TCP keepalive probe.
Some platforms specify this value in seconds, so sub-second millisecond specifications may be omitted.
Edit hit enter too soon.
Either documenting or change is fine for me, if you go for the doc I can do a PR today/tomorrow.