aio-libs / aio-libs/aiohttp

Introduce keepalive_maxage config parameter for BaseConnector

Đang mở
#4,948 0 bình luận 12 reaction 0 người được giao Xem trên GitHub
enhancement
Ngôn ngữ chính
Python
Star
16.5k
Fork
2.4k
Merge trung bình
20 giờ 10 phút
Pull request đã merge (30 ngày)
221

Mô tả

🐣 **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 🙇

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.