net: expose keepalive configuration on TcpSocket
#3146
Conversation
This commit adds `set_{send, recv}_buffer_size` methods to `TcpSocket`
for setting the size of the TCP send and receive buffers, and `{send,
recv}_buffer_size` methods for returning the current value. These just
call into similar methods on `mio`'s `TcpSocket` type, which were added
in tokio-rs/mio#1384.
Refs: #3082
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
changed the title
TcpSocket
| /// keepalive parameters after they have been set.. | ||
| /// | ||
| /// Some platforms specify this value in seconds, so sub-second | ||
| /// specifications may be omitted. |
There was a problem hiding this comment.
Rounded down or up? What happens if this rounds to zero?
| @@ -86,6 +87,11 @@ cfg_net! { | |||
| } | |||
| } | |||
|
|
|||
| cfg_net! { | |||
| #[allow(unreachable_pub)] // https://github.com/rust-lang/rust/issues/57411 | |||
| pub use mio::net::TcpKeepalive; | |||
There was a problem hiding this comment.
This makes a mio type part of the public API, which is probably not ideal.
There was a problem hiding this comment.
I'm hesitant to add this API at this time. This goes beyond what Tokio has done to date with regard to socket options. Until now, we have added socket option APIs that are essentially pass-through to the OS call. This attempts to provide a portable API that abstracts differences.
I also see a potential issue with the implementation in Mio where a single call to set_keepalive_params may result in multiple sub calls. If the first sub call succeeds, but the second fails, the socket state is inconsistent. All other socket option setters are atomic.
Perhaps it is best to leave such configuration to other crates or the user to call themselves?
| @@ -86,6 +87,11 @@ cfg_net! { | |||
| } | |||
| } | |||
|
|
|||
| cfg_net! { | |||
| #[allow(unreachable_pub)] // https://github.com/rust-lang/rust/issues/57411 | |||
| pub use mio::net::TcpKeepalive; | |||
There was a problem hiding this comment.
We are not exposing mio as part of Tokio's public API. If we do this, we will need to re-define the type.
| @@ -33,7 +33,7 @@ cfg_net! { | |||
|
|
|||
| pub mod tcp; | |||
| pub use tcp::listener::TcpListener; | |||
| pub use tcp::socket::TcpSocket; | |||
| pub use tcp::socket::{TcpSocket, TcpKeepalive}; | |||
There was a problem hiding this comment.
We probably can keep this type in tokio::net::tcp and not re-export it.
|
Closing as we decided in Discord to punt this. |
|
@carllerche wdyt about exposing just keepalive time (time to the initial probe)? That, at least, can be configured consistently across all platforms. |
|
If it is consistent across platforms then it seems fine to me. |
Darksonn
added
A-tokio
Depends on #3145
Motivation
Tokio v0.3 no longer exposes a way to configure TCP keepalive probes.
Many users need this functionality, so we want to re-expose keepalive
configuration via
TcpSocket.Solution
This commit exposes the TCP keepalive configuration added to
mio'sTcpSocketin tokio-rs/mio#1385. This includes a method for enablingkeepalvie and checking if keepalive is enabled, a builder interface for
configuring keepalive parameters (keepalive time, the amount of time
before keepalive probes are sent, keepalive interval, the time between
each probe, and keepalive retries, the number of failed probes before a
connection is closed), and accessors for the values of keepalive
parameters. Due to Windows API limitations,
miohas to provide abuilder for configuring keepalive parameters, since they must all be set
in one API call. Therefore, we re-export the keepalive builder struct.
If the
mioAPI changes, we should be able to replace this with our owntype transparently, so it didn't seem necessary to wrap this --- if I'm
wrong, please let me know!
Fixes: #3082