Customize Keep-Alive timeout

[jborean93]

Checklist

• There are no similar issues or pull requests for this yet.
• I discussed this idea on the community chat and feedback is positive.

Is your feature related to a problem? Please describe.

The change #627 added a 5 second timeout for Keep-Alive connections. Once that timeout is reached and no new requests are made in that timeframe then the connection in the pool is dropped.

I'm working with an endpoint that has a longer timeout before the server sends the RST/ACK on an idle connection (~2 minutes). While there should usually be a steady stream of requests being made there can be a time where nothing was sent in that 5 second window. Once the connection is dropped from the pool then the client needs to create a new connection and do any authentication that is required. This authentication process could be quite expensive as depending on the auth protocol used, multiple round trips may be required.

Describe the solution you would like.

Potentially a way to define the timoeut in httpx.Timeout(keep_alive=x) or maybe under httpx.Limits(keep_alive=x).

Describe alternatives you considered

I'm no expert on the HTTP spec but I would have assumed we could just rely on the server to send the RST, ACK as a way to determine if the socket has been closed and to only remove it from the pool then.

[florimondmanca]

Hello!

This idea was actually mentioned and quickly discussed on the original PR, see #627 (comment) :-)

Should this be made configurable, e.g. as a new option on Timeout()?
We might want to name it "expiry" rather than "timeout". Yes ideally configurable, perhaps on the PoolLimits, but we could probably treat that as a second pass.

Then this second pass came in in the HTTPCore repo… encode/httpcore#32

And so as it turns out, there's already a way to achieve this, which is actually advertised in the Custom transports: Usage docs:

import httpcore
import httpx

ssl_context = httpx.create_ssl_context()
transport = httpcore.SyncConnectionPool(
    ssl_context=ssl_context,
    max_connections=100,
    max_keepalive_connections=20,
    keepalive_expiry=10.0,  # <---
)

with httpx.Client(transport=transport):
    ...

Yes, that's verbose because currently one needs to pass all options that HTTPX passes through otherwise, but we have an open ticket to address this: #1302

There's still the option of updating httpx.Limits to also accept keepalive_expiry=..., but we'd also like to promote the transport API more (so that all options of the default HTTP transports don't have to be mirrored at the client level). Once something like httpx.HTTPTransport() lands you'd be able to do it this way:

import httpx

transport = httpx.HTTPTransport(keepalive_expiry=10.0)
with httpx.Client(transport=transport):
    ...

---

As for your point about RST/ACK — I don't know enough to tell if this is something we could rely upon. I guess keepalive expiry is here so that in case servers don't send that kind of shutdown, then we can be sure we're not keeping stale connections for too long. Besides I'm not sure how common that RST/ACK mechanism is for us to consider supporting it. Do other HTTP clients support something like this?

I'm renaming this issue to more specifically address this RST/ACK discussion, since I think for keepalive_expiry you should be all set now. :-)

[jborean93]

Thanks for the suggestion, I'll definitely give it a try as redoing the authentication every 1-2 minute of idle connections is a lot more desirable than every 5 seconds of an idle connection. Sorry I can't help anymore on the discussion on the RST/ACK, I also don't know enough about the protocol to really make a decision as to what is the proper course of action.

[tomchristie]

Okay, so...

We do already support "server-initiated RST/ACK as a complement to client-side keep-alive timeouts", in that we check the aliveness of the connection when re-acquiring from the pool, so we have a "either expired or disconnected policy".

Tho our current keepalive timeout is fairly low, yup.

We ended up with that as a default largely because asyncio isn't terribly resilient at gracefully detecting/handling malformed SSL connection closes, so we needed a short keepalive to mitigate against the chance of re-using an SSL connection that had closed improperly. The sync and trio versions do not have this issue.

Once encode/httpcore#167 is pulled in, it's possible we might be able to increase the default value. (10 or 30 seconds both sound reasonable to me)

[florimondmanca]

"either expired or disconnected policy"

True. I had not directly made the link that this RST/ACK thing is just the server hanging up on the connection, and we already discard connections that are obviously closed because of that.

Right, then off to renaming this again to focus back on keep-alive… I'd agree the default could probably be a bit higher once we're confident enough it would work in the asyncio case.