Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idle timeout #3275

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Idle timeout #3275

wants to merge 4 commits into from

Conversation

ecerulm
Copy link
Contributor

@ecerulm ecerulm commented Jan 10, 2024

Implements idle_timeout where connections that have been idle for more than idle_timeout seconds 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

@ecerulm ecerulm changed the title WIP: Idle timeout Idle timeout Jan 10, 2024
@ecerulm ecerulm force-pushed the idle_timeout branch 2 times, most recently from 7685c83 to 8e6284e Compare January 11, 2024 08:55
@ecerulm ecerulm marked this pull request as draft January 11, 2024 08:56
@ecerulm ecerulm marked this pull request as ready for review January 11, 2024 11:34
@ecerulm
Copy link
Contributor Author

ecerulm commented Jan 11, 2024

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 assert 0.4999978542327881 >= 0.5 that is caused by float precision loss when doing delta = time.time() - now. The PR #3276 adds some tolerance for that.

src/urllib3/connectionpool.py Outdated Show resolved Hide resolved
@ecerulm
Copy link
Contributor Author

ecerulm commented Jan 12, 2024

The CI for pypy-3.9 failed (cancelled after 30
minutes) I don’t think it’s related to the PR either.

src/urllib3/connection.py Show resolved Hide resolved
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why key on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@ecerulm
Copy link
Contributor Author

ecerulm commented Jan 17, 2024

@sethmlarson , @sigmavirus24 , the CI seems to fail for pypy-3.9 , I tried on my laptop nox -rs test-pypy and that passed. Is there any know issue with the CI?

@sethmlarson sethmlarson closed this May 1, 2024
@sethmlarson sethmlarson reopened this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants