fix too much unclosed sockets (file descriptor)

[ailisp]

See #1316
This is a bug of actix_web::client, in short, it will try to close any socket conn that has been opened for conn_lifetime time regardless of conn_keep_alive. The problem is it think it successfully closed connection and happily open a new socket connection, but close isn't actually successful.

My fix is to give it a maximum possible lifetime, this is the simplest fix i can think of (and it works fine locally)
There're likely two other better fixes:

1. Fix in actix-web, https://github.com/actix/actix-web/blob/master/actix-http/src/client/pool.rs#L376
   or https://github.com/actix/actix-web/blob/master/actix-http/src/client/pool.rs#L456
2. use reqwest async client instead

[bowenwang1996]

We should consider submitting an issue to actix if the fix is not too hard.

[ailisp]

Sounds good. If it takes too much time for them to merge, we can keep this fix

…

On Fri, Oct 4, 2019 at 6:34 PM Bowen Wang ***@***.***> wrote: We should consider submitting an issue to actix if the fix is not too hard. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1413>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADFFFCCY4MR2IUGBSOIIJA3QM7VKFANCNFSM4I5VX7XQ> .

[frol]

There is no need to do this. u64::max_value() is available just fine without this use.

[ailisp]

Good point, fixed now. I'm confusing that why std::u64::max_value() => compile error. why it's not equivalent to use std::u64; u64::max_value()?
(u64::max_value() is just fine without this use too, as you said)

[ailisp]

@bowenwang1996 @frol Was able to located the bug in actix-http. So in the if branch they want to close the connection, there're two cases: http/1 and http/2. They only handle http/1 case, via some wrapper to io.shutdown. For http/2 case it's ignored. The tricky part to me: it's not obvious how to close a http2 connection. Their connection type definition:

pub(crate) enum ConnectionType<Io> {
    H1(Io),
    H2(SendRequest<Bytes>),
}

For H1, they simply do if let(io)=ConnectionType::H1 and later on io::close. For H2 actix-web is using this crate: https://github.com/hyperium/h2. There isn't a documented way to close it. And so far I couldn't find a proper undocumented way.

A fix in actix-http I can do is do not close. ignore conn_lifetime and conn_keep_alive param, return Acquire::Acquired(conn.io, conn.created) to indicate http/2 connection is ready to reuse. This basically gives the same result as this PR.

[bowenwang1996]

Maybe opening an issue in the actix repo so that you can discuss it with their devs?

[ailisp]

Sounds good! Reported here: actix/actix-web#1123

[zyc801208]

Is this problem fixed? lots of unclosed connection( with status CLOSED_WAIT) occurred,

Cargo.toml
actix-web = {version="2.0.0", features=["openssl"]}
actix-rt = "1.0.0"
awc = "1.0"

[frol]

@zyc801208 yes, it fixed the issue for us back then. We don't observe CLOSED_WAIT state connections on our nodes. Please, create a new issue with more information about your setup and mention me there.

[zyc801208]

@zyc801208 yes, it fixed the issue for us back then. We don't observe CLOSED_WAIT state connections on our nodes. Please, create a new issue with more information about your setup and mention me there.

Ok, thank you!