Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give more control on connection tasks #29

Open
programatik29 opened this issue Nov 15, 2021 · 6 comments
Open

Give more control on connection tasks #29

programatik29 opened this issue Nov 15, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@programatik29
Copy link
Owner

programatik29 commented Nov 15, 2021

Currently there is no way to shut a connection down except signaling a global shutdown.

Having this ability can be useful to detect slow clients and prevent some attacks.

@programatik29 programatik29 added the enhancement New feature or request label Nov 15, 2021
@hansonchar
Copy link
Contributor

Related (but orthogonal) to this great enhancement proposal, I see by default the TCP keepalive is disabled in axum-server, as TCP keepalive is disabled by default in hyper. This means dead TCP connections could be left undetected and accumulate over time eventually leading to resource exhaustion with error similar to

ERROR                 main hyper::server::tcp: accept error: Too many open files (os error 24)

To alleviate the issue, one can enable TCP keepalive with code like:

use axum::{routing::get, Router};
use std::net::SocketAddr;

#[tokio::main]
async fn main() {
    let app = Router::new().route("/", get(|| async { "Hello, world!" }));

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    println!("listening on {}", addr);
    axum_server::bind(addr)
        .addr_incoming_config(
            AddrIncomingConfig::default()
                .tcp_keepalive(Some(Duration::from_secs(60)))
                .build(),
        ).serve(app.into_make_service())
        .await
        .unwrap();
}

So far testing on a host that experienced this problem within a day or two shows very promising behavior.

@hansonchar
Copy link
Contributor

hansonchar commented Sep 21, 2022

Furthermore, currently the only configurable property for TCP keepalive in axum_server for incoming/accepted connections is the Keepalive time as defined at https://en.wikipedia.org/wiki/Keepalive. The other two properties,Keepalive interval and Keepalive retry are not currently configurable and are set to None by default .

Specifically, even though all three parameters are supported by the socket2 crate (used by hyper), only Keepalive time is exposed by the hyper crate for configuration via AddrIncoming.html#method.set_keepalive which the axum_server crate depends upon.

Opportunities for improvement on both hyper and axum_server it appears.

@hansonchar
Copy link
Contributor

hansonchar commented Sep 21, 2022

Here is a PR for hyper to make all three TCP Keepalive parameters configurable:

hyperium/hyper#2991

If it was accepted, I would then post the corresponding PR to make this configurable at the axum-server layer.

@hansonchar
Copy link
Contributor

Per empirical evidence enabling TCP keepalive with intervals and retries drastically/significantly improves the stability of persistent TCP connections while reducing the number of file descriptors via closing down dead connections.

@johan-smits
Copy link

FYI hyperium/hyper#2991 was accepted.

@josecelano
Copy link

It seems the AddrIncomingConfig was removed. I don't see any example or documentation to set the tcp_keepalive duration in the latest version. Was that feature removed @programatik29?

Relates to: https://github.com/programatik29/axum-server/pulls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants