-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Connections kept forever in connection pool #1301
Comments
Adding actual removal of connections would be a pretty substantial change to the codebase. I would consider a PR to add the feature if it could be done in a minimally-invasive manner, without creating additional threads inside urllib3, and without causing too much extra work to normally-processed requests, but don't believe that it would justify major changes, especially when a pool's maximum number of connections can be set relatively easily. |
It does not need to use threads but at least we will need to do a check for all connections in the pool when a connection is requested, that will add some work and probably will need to add some locking to keep it thread safe. With both solutions it will free connections as you keep using some of them but if you stop using them at all they will stay there forever, unless you delete the connection pool altogether. |
LIFO is far more optimal for what we want. Most HTTP usage will make repeated requests to the same host and that means the last connection in will most likely be the next one needed. That said, if we get one and it is expired, we do discard it. |
Closing this issue as it's gone stale. Comment within if you'd like to reopen or continue discussion. |
Maybe this is intended as otherwise it would add complexity to the connection pool but connections are never removed from the pool, so long lived apps with a burst of requests will keep those connection objects in memory forever.
The solution could be adding a stale timeout parameter to the connection pool and removing the connections from the pool after that timeout.
The text was updated successfully, but these errors were encountered: