Idle timeout #3275
Idle timeout #3275
Conversation
7685c83 to
8e6284e
Compare
|
The testcase failing on Window 3.10 https://github.com/urllib3/urllib3/actions/runs/7486491572/job/20376966639?pr=3275 will be solved by #3276. It complains that |
|
The CI for pypy-3.9 failed (cancelled after 30 |
| @@ -91,6 +92,7 @@ class PoolKey(typing.NamedTuple): | |||
| key_assert_fingerprint: str | None | |||
| key_server_hostname: str | None | |||
| key_blocksize: int | None | |||
| key_idle_timeout: float | datetime.timedelta | None | |||
There was a problem hiding this comment.
I'm not sure I understand. I need to add key_idle_timeout to PoolKey otherwise PoolManager(idle_timeout=x) will raise an error, this is tested on test/with_dummyserver/test_poolmanager.py::TestPoolManager::test_key_idle_timeout, if I remove key_idle_timeout from PoolKey it will fail like this:
FAILED test/with_dummyserver/test_poolmanager.py::TestPoolManager::test_key_idle_timeout - TypeError: PoolKey.__new__() got an unexpected keyword argument 'key_idle_timeout'
Co-authored-by: Ian Stapleton Cordasco <graffatcolmingov@gmail.com>
|
@sethmlarson , @sigmavirus24 , the CI seems to fail for pypy-3.9 , I tried on my laptop |
| if idle_timeout: | ||
| if isinstance(idle_timeout, datetime.timedelta): | ||
| self.idle_timeout = idle_timeout | ||
| else: | ||
| self.idle_timeout = datetime.timedelta(seconds=idle_timeout) | ||
| self.last_activity = datetime.datetime.now() |
There was a problem hiding this comment.
Would it be preferable to use monotonic time for this? datetime introduces the complexity of timezones and clock jumps, and keeping it all as floats keeps typing consistent with the other timeouts. eg.
| if idle_timeout: | |
| if isinstance(idle_timeout, datetime.timedelta): | |
| self.idle_timeout = idle_timeout | |
| else: | |
| self.idle_timeout = datetime.timedelta(seconds=idle_timeout) | |
| self.last_activity = datetime.datetime.now() | |
| if idle_timeout: | |
| if isinstance(idle_timeout, datetime.timedelta): | |
| self.idle_timeout = idle_timeout.total_seconds() | |
| else: | |
| self.idle_timeout = idle_timeout | |
| self.last_activity = time.monotonic() |
| Idle timeout in seconds after which a connection is considered not | ||
| usable. Useful if connecting via NAT that silently drops connections | ||
| after inactivity period like `AWS NAT gateway <https://docs.aws.amazon.com/vpc/latest/userguide/nat-gateway-troubleshooting.html#nat-gateway-troubleshooting-timeout>`_. | ||
| 8000 |
|
There was a problem hiding this comment.
You may want to add as well: https://learn.microsoft.com/en-us/azure/nat-gateway/faq#what-is-the-idle-timeout-for-a-nat-gateway
|
As far as I can see abandoning valid connections can also result in a memory leak. I'd silently try to close those connections to avoid leaks. |
Implements idle_timeout where connections that have been idle for more than
idle_timeoutseconds are considered stale.The connection pool will check the idleness before returning a connection, and if will create a new connection if the first candidate connection already "expired". In particular, it won't try all the connections in the pool until it finds one.
The idle status is reset at every call to
request().Closes #3184
Closes #2498