aio-libs / aio-libs/aiohttp

Introduce keepalive_maxage config parameter for BaseConnector

Aperta
#4,948 0 commenti 12 reazioni 0 assegnatari Vedi su GitHub
enhancement
Lingua principale
Python
Stelle
16.5k
Fork
2.4k
Merge medio
20h 10m
PR unite (30g)
221

Descrizione

🐣 **Is your feature request related to a problem? Please describe.**
Current `keepalive_timeout` parameter works like a "maximum idle time": if a connection has been idle `> keepalive_timeout` it gets closed. But if the connection keeps being used frequently (say, every 1s while the timeout is 75s), the connection will be kept alive indefinitely.

The problem is that we would also like to configure a maximum length of time a connection should be kept alive (since initial connection), regardless of whether it has been idle or not. For example, consider an upstream service with keep alive enabled and a short DNS TTL (60s) where we use CNAME to direct the traffic between different datacenters A and B. If the service is frequently called, the connections will be kept alive indefinitely and the traffic will never switch from A to B.

💡 **Describe the solution you'd like**
A new BaseConnector config parameter called `keepalive_maxage` that configures the maximum age in seconds a connection will be 'kept alive' before it is closed.

Implementation idea:
Currently, an individual connection is represented by a `tuple(connection, timestamp_lastused)` https://github.com/aio-libs/aiohttp/blob/48bf8c356c7c3bac5d520484ede5c35bd1795afb/aiohttp/connector.py#L213

We introduce a third element to this tuple (timestamp_created) which is set when the connection is made and not updated on subsequent use: `tuple(connection, timestamp_lastused, timestamp_created)`.

To implement closing, when we check the `timeout_lastused`, we also check whether current time (t1) `t1 - timestamp_created > keepalive_maxage` and close the connection if so: https://github.com/aio-libs/aiohttp/blob/48bf8c356c7c3bac5d520484ede5c35bd1795afb/aiohttp/connector.py#L543

❓ **Describe alternatives you've considered**
We could call `_drop_acquired_per_host` every Xs ourselves to refresh but seems nicer to have this as a configuration parameter on the `BaseConnector` class itself.

We could subclass TCPConnector ourselves to implement this functionality but if it's useful for us, this extra config could be useful for others, hence this suggestion to implement upstream.

📋 **Additional context**
I (or one of my colleagues) would be more than happy to work on this feature request but I want to check the proposed feature and implementation with the maintainers before doing so 🙇

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.