From ec6006a943a263ebb31058cf4896dce1ab3d0a50 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Wed, 20 Sep 2023 12:44:47 +0200 Subject: [PATCH] Removed force_close from all downloaders This flag causes large syncs to fail under certain circumstances, because the port range is used up too fast and reusing ports in time_wait is not appreciated by servers. We cannot possibly fix all broken http servers in the wild. fixes #4452 --- CHANGES/4452.bugfix | 2 ++ pulpcore/download/factory.py | 22 +++++++++------------- pulpcore/download/http.py | 6 +----- 3 files changed, 12 insertions(+), 18 deletions(-) create mode 100644 CHANGES/4452.bugfix diff --git a/CHANGES/4452.bugfix b/CHANGES/4452.bugfix new file mode 100644 index 0000000000..54d44c2a0f --- /dev/null +++ b/CHANGES/4452.bugfix @@ -0,0 +1,2 @@ +Removed a workaround to force close all tcp connections in sync leading to an exhaustion of port +numbers and their reuse while in time_wait state. diff --git a/pulpcore/download/factory.py b/pulpcore/download/factory.py index 7df150d3eb..0bd71391ea 100644 --- a/pulpcore/download/factory.py +++ b/pulpcore/download/factory.py @@ -48,10 +48,6 @@ class DownloaderFactory: http://aiohttp.readthedocs.io/en/stable/client_quickstart.html#timeouts Behaviorally, it should allow for an active download to be arbitrarily long, while still detecting dead or closed sessions even when TCPKeepAlive is disabled. - - Also for http and https urls, even though HTTP 1.1 is used, the TCP connection is setup and - closed with each request. This is done for compatibility reasons due to various issues related - to session continuation implementation in various servers. """ def __init__(self, remote, downloader_overrides=None): @@ -102,7 +98,7 @@ def _make_aiohttp_session_from_remote(self): Returns: :class:`aiohttp.ClientSession` """ - tcp_conn_opts = {"force_close": True} + tcp_conn_opts = {} sslcontext = None if self._remote.ca_cert: @@ -133,17 +129,17 @@ def _make_aiohttp_session_from_remote(self): headers["User-Agent"] = f"{headers['User-Agent']}, {user_agent_header}" headers.extend(header_dict) - conn = aiohttp.TCPConnector(**tcp_conn_opts) - total = self._remote.total_timeout - sock_connect = self._remote.sock_connect_timeout - sock_read = self._remote.sock_read_timeout - connect = self._remote.connect_timeout - timeout = aiohttp.ClientTimeout( - total=total, sock_connect=sock_connect, sock_read=sock_read, connect=connect + total=self._remote.total_timeout, + sock_connect=self._remote.sock_connect_timeout, + sock_read=self._remote.sock_read_timeout, + connect=self._remote.connect_timeout, ) return aiohttp.ClientSession( - connector=conn, timeout=timeout, headers=headers, requote_redirect_url=False + connector=aiohttp.TCPConnector(**tcp_conn_opts), + timeout=timeout, + headers=headers, + requote_redirect_url=False, ) def build(self, url, **kwargs): diff --git a/pulpcore/download/http.py b/pulpcore/download/http.py index a918e41de2..c66a7d9a60 100644 --- a/pulpcore/download/http.py +++ b/pulpcore/download/http.py @@ -71,10 +71,6 @@ class HttpDownloader(BaseDownloader): allow for an active download to be arbitrarily long, while still detecting dead or closed sessions even when TCPKeepAlive is disabled. - If a session is not provided, the one created will force TCP connection closure after each - request. This is done for compatibility reasons due to various issues related to session - continuation implementation in various servers. - `aiohttp.ClientSession` objects allows you to configure options that will apply to all downloaders using that session such as auth, timeouts, headers, etc. For more info on these options see the `aiohttp.ClientSession` docs for more information: @@ -165,7 +161,7 @@ def __init__( self._close_session_on_finalize = False else: timeout = aiohttp.ClientTimeout(total=None, sock_connect=600, sock_read=600) - conn = aiohttp.TCPConnector({"force_close": True}) + conn = aiohttp.TCPConnector() self.session = aiohttp.ClientSession( connector=conn, timeout=timeout, headers=headers, requote_redirect_url=False )