Skip to content

Commit 6422799

Browse files
MarkusSintonenMarkusSintonen
committed
Move connection poll check away from pool expiry checks
1 parent da86ca4 commit 6422799

File tree

12 files changed

+359
-81
lines changed

12 files changed

+359
-81
lines changed

‎httpcore/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
ReadError,
3030
ReadTimeout,
3131
RemoteProtocolError,
32+
ServerDisconnectedError,
3233
TimeoutException,
3334
UnsupportedProtocol,
3435
WriteError,
@@ -114,6 +115,7 @@ def __init__(self, *args, **kwargs): # type: ignore
114115
"SOCKET_OPTION",
115116
# exceptions
116117
"ConnectionNotAvailable",
118+
"ServerDisconnectedError",
117119
"ProxyError",
118120
"ProtocolError",
119121
"LocalProtocolError",

‎httpcore/_async/connection_pool.py‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ def _assign_requests_to_connections(self) -> List[AsyncConnectionInterface]:
240240
closing_connections = []
241241

242242
# First we handle cleaning up any connections that are closed,
243-
# have expired their keep-alive, or surplus idle connections.
243+
# have expired, or surplus idle connections.
244244
for connection in list(self._connections):
245245
if connection.is_closed():
246246
# log: "removing closed connection"
@@ -267,15 +267,12 @@ def _assign_requests_to_connections(self) -> List[AsyncConnectionInterface]:
267267
for connection in self._connections
268268
if connection.can_handle_request(origin) and connection.is_available()
269269
]
270-
idle_connections = [
271-
connection for connection in self._connections if connection.is_idle()
272-
]
273270

274271
# There are three cases for how we may be able to handle the request:
275272
#
276273
# 1. There is an existing connection that can handle the request.
277274
# 2. We can create a new connection to handle the request.
278-
# 3. We can close an idle connection and then create a new connection
275+
# 3. We can close an idle/expired connection and then create a new connection
279276
# to handle the request.
280277
if available_connections:
281278
# log: "reusing existing connection"
@@ -286,15 +283,19 @@ def _assign_requests_to_connections(self) -> List[AsyncConnectionInterface]:
286283
connection = self.create_connection(origin)
287284
self._connections.append(connection)
288285
pool_request.assign_to_connection(connection)
289-
elif idle_connections:
290-
# log: "closing idle connection"
291-
connection = idle_connections[0]
292-
self._connections.remove(connection)
293-
closing_connections.append(connection)
294-
# log: "creating new connection"
295-
connection = self.create_connection(origin)
296-
self._connections.append(connection)
297-
pool_request.assign_to_connection(connection)
286+
else:
287+
purged_connection = next(
288+
(c for c in self._connections if c.is_idle() or c.has_expired()),
289+
None,
290+
)
291+
if purged_connection is not None:
292+
# log: "closing idle connection"
293+
self._connections.remove(purged_connection)
294+
closing_connections.append(purged_connection)
295+
# log: "creating new connection"
296+
connection = self.create_connection(origin)
297+
self._connections.append(connection)
298+
pool_request.assign_to_connection(connection)
298299

299300
return closing_connections
300301

‎httpcore/_async/http11.py‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
ConnectionNotAvailable,
2222
LocalProtocolError,
2323
RemoteProtocolError,
24+
ServerDisconnectedError,
2425
WriteError,
2526
map_exceptions,
2627
)
@@ -45,6 +46,7 @@ class HTTPConnectionState(enum.IntEnum):
4546
ACTIVE = 1
4647
IDLE = 2
4748
CLOSED = 3
49+
SERVER_DISCONNECTED = 4
4850

4951

5052
class AsyncHTTP11Connection(AsyncConnectionInterface):
@@ -59,7 +61,7 @@ def __init__(
5961
) -> None:
6062
self._origin = origin
6163
self._network_stream = stream
62-
self._keepalive_expiry: Optional[float] = keepalive_expiry
64+
self._keepalive_expiry = keepalive_expiry
6365
self._expire_at: Optional[float] = None
6466
self._state = HTTPConnectionState.NEW
6567
self._state_lock = AsyncLock()
@@ -76,13 +78,7 @@ async def handle_async_request(self, request: Request) -> Response:
7678
f"to {self._origin}"
7779
)
7880

79-
async with self._state_lock:
80-
if self._state in (HTTPConnectionState.NEW, HTTPConnectionState.IDLE):
81-
self._request_count += 1
82-
self._state = HTTPConnectionState.ACTIVE
83-
self._expire_at = None
84-
else:
85-
raise ConnectionNotAvailable()
81+
await self._update_state()
8682

8783
try:
8884
kwargs = {"request": request}
@@ -142,6 +138,29 @@ async def handle_async_request(self, request: Request) -> Response:
142138
await self._response_closed()
143139
raise exc
144140

141+
async def _update_state(self) -> None:
142+
async with self._state_lock:
143+
# If the HTTP connection is idle but the socket is readable, then the
144+
# only valid state is that the socket is about to return b"", indicating
145+
# a server-initiated disconnect.
146+
server_disconnected = (
147+
self._state == HTTPConnectionState.IDLE
148+
and self._network_stream.get_extra_info("is_readable")
149+
)
150+
if (
151+
server_disconnected
152+
or self._state == HTTPConnectionState.SERVER_DISCONNECTED
153+
):
154+
self._state = HTTPConnectionState.SERVER_DISCONNECTED
155+
raise ServerDisconnectedError()
156+
157+
if self._state in (HTTPConnectionState.NEW, HTTPConnectionState.IDLE):
158+
self._request_count += 1
159+
self._state = HTTPConnectionState.ACTIVE
160+
self._expire_at = None
161+
else:
162+
raise ConnectionNotAvailable()
163+
145164
# Sending the request...
146165

147166
async def _send_request_headers(self, request: Request) -> None:
@@ -279,18 +298,13 @@ def is_available(self) -> bool:
279298
return self._state == HTTPConnectionState.IDLE
280299

281300
def has_expired(self) -> bool:
282-
now = time.monotonic()
283-
keepalive_expired = self._expire_at is not None and now > self._expire_at
284-
285-
# If the HTTP connection is idle but the socket is readable, then the
286-
# only valid state is that the socket is about to return b"", indicating
287-
# a server-initiated disconnect.
288-
server_disconnected = (
289-
self._state == HTTPConnectionState.IDLE
290-
and self._network_stream.get_extra_info("is_readable")
291-
)
301+
if self._state == HTTPConnectionState.SERVER_DISCONNECTED:
302+
# Connection that is disconnected by the server is considered expired.
303+
# Pool then cleans up this connection by closing it.
304+
return True
292305

293-
return keepalive_expired or server_disconnected
306+
now = time.monotonic()
307+
return self._expire_at is not None and now > self._expire_at
294308

295309
def is_idle(self) -> bool:
296310
return self._state == HTTPConnectionState.IDLE

‎httpcore/_async/http2.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,7 @@ async def handle_async_request(self, request: Request) -> Response:
9393
f"to {self._origin}"
9494
)
9595

96-
async with self._state_lock:
97-
if self._state in (HTTPConnectionState.ACTIVE, HTTPConnectionState.IDLE):
98-
self._request_count += 1
99-
self._expire_at = None
100-
self._state = HTTPConnectionState.ACTIVE
101-
else:
102-
raise ConnectionNotAvailable()
96+
await self._update_state()
10397

10498
async with self._init_lock:
10599
if not self._sent_connection_init:
@@ -184,6 +178,15 @@ async def handle_async_request(self, request: Request) -> Response:
184178

185179
raise exc
186180

181+
async def _update_state(self) -> None:
182+
async with self._state_lock:
183+
if self._state in (HTTPConnectionState.ACTIVE, HTTPConnectionState.IDLE):
184+
self._request_count += 1
185+
self._expire_at = None
186+
self._state = HTTPConnectionState.ACTIVE
187+
else:
188+
raise ConnectionNotAvailable()
189+
187190
async def _send_connection_init(self, request: Request) -> None:
188191
"""
189192
The HTTP/2 connection requires some initial setup before we can start

‎httpcore/_exceptions.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,17 @@ def map_exceptions(map: ExceptionMapping) -> Iterator[None]:
1616

1717

1818
class ConnectionNotAvailable(Exception):
19-
pass
19+
"""
20+
This error is handled by the connection pool.
21+
Users should not see this error directly when using connection pool.
22+
"""
23+
24+
25+
class ServerDisconnectedError(ConnectionNotAvailable):
26+
"""
27+
This error is handled by the connection pool.
28+
Users should not see this error directly when using connection pool.
29+
"""
2030

2131

2232
class ProxyError(Exception):

‎httpcore/_sync/connection_pool.py‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ def _assign_requests_to_connections(self) -> List[ConnectionInterface]:
240240
closing_connections = []
241241

242242
# First we handle cleaning up any connections that are closed,
243-
# have expired their keep-alive, or surplus idle connections.
243+
# have expired, or surplus idle connections.
244244
for connection in list(self._connections):
245245
if connection.is_closed():
246246
# log: "removing closed connection"
@@ -267,15 +267,12 @@ def _assign_requests_to_connections(self) -> List[ConnectionInterface]:
267267
for connection in self._connections
268268
if connection.can_handle_request(origin) and connection.is_available()
269269
]
270-
idle_connections = [
271-
connection for connection in self._connections if connection.is_idle()
272-
]
273270

274271
# There are three cases for how we may be able to handle the request:
275272
#
276273
# 1. There is an existing connection that can handle the request.
277274
# 2. We can create a new connection to handle the request.
278-
# 3. We can close an idle connection and then create a new connection
275+
# 3. We can close an idle/expired connection and then create a new connection
279276
# to handle the request.
280277
if available_connections:
281278
# log: "reusing existing connection"
@@ -286,15 +283,19 @@ def _assign_requests_to_connections(self) -> List[ConnectionInterface]:
286283
connection = self.create_connection(origin)
287284
self._connections.append(connection)
288285
pool_request.assign_to_connection(connection)
289-
elif idle_connections:
290-
# log: "closing idle connection"
291-
connection = idle_connections[0]
292-
self._connections.remove(connection)
293-
closing_connections.append(connection)
294-
# log: "creating new connection"
295-
connection = self.create_connection(origin)
296-
self._connections.append(connection)
297-
pool_request.assign_to_connection(connection)
286+
else:
287+
purged_connection = next(
288+
(c for c in self._connections if c.is_idle() or c.has_expired()),
289+
None,
290+
)
291+
if purged_connection is not None:
292+
# log: "closing idle connection"
293+
self._connections.remove(purged_connection)
294+
closing_connections.append(purged_connection)
295+
# log: "creating new connection"
296+
connection = self.create_connection(origin)
297+
self._connections.append(connection)
298+
pool_request.assign_to_connection(connection)
298299

299300
return closing_connections
300301

‎httpcore/_sync/http11.py‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
ConnectionNotAvailable,
2222
LocalProtocolError,
2323
RemoteProtocolError,
24+
ServerDisconnectedError,
2425
WriteError,
2526
map_exceptions,
2627
)
@@ -45,6 +46,7 @@ class HTTPConnectionState(enum.IntEnum):
4546
ACTIVE = 1
4647
IDLE = 2
4748
CLOSED = 3
49+
SERVER_DISCONNECTED = 4
4850

4951

5052
class HTTP11Connection(ConnectionInterface):
@@ -59,7 +61,7 @@ def __init__(
5961
) -> None:
6062
self._origin = origin
6163
self._network_stream = stream
62-
self._keepalive_expiry: Optional[float] = keepalive_expiry
64+
self._keepalive_expiry = keepalive_expiry
6365
self._expire_at: Optional[float] = None
6466
self._state = HTTPConnectionState.NEW
6567
self._state_lock = Lock()
@@ -76,13 +78,7 @@ def handle_request(self, request: Request) -> Response:
7678
f"to {self._origin}"
7779
)
7880

79-
with self._state_lock:
80-
if self._state in (HTTPConnectionState.NEW, HTTPConnectionState.IDLE):
81-
self._request_count += 1
82-
self._state = HTTPConnectionState.ACTIVE
83-
self._expire_at = None
84-
else:
85-
raise ConnectionNotAvailable()
81+
self._update_state()
8682

8783
try:
8884
kwargs = {"request": request}
@@ -142,6 +138,29 @@ def handle_request(self, request: Request) -> Response:
142138
self._response_closed()
143139
raise exc
144140

141+
def _update_state(self) -> None:
142+
with self._state_lock:
143+
# If the HTTP connection is idle but the socket is readable, then the
144+
# only valid state is that the socket is about to return b"", indicating
145+
# a server-initiated disconnect.
146+
server_disconnected = (
147+
self._state == HTTPConnectionState.IDLE
148+
and self._network_stream.get_extra_info("is_readable")
149+
)
150+
if (
151+
server_disconnected
152+
or self._state == HTTPConnectionState.SERVER_DISCONNECTED
153+
):
154+
self._state = HTTPConnectionState.SERVER_DISCONNECTED
155+
raise ServerDisconnectedError()
156+
157+
if self._state in (HTTPConnectionState.NEW, HTTPConnectionState.IDLE):
158+
self._request_count += 1
159+
self._state = HTTPConnectionState.ACTIVE
160+
self._expire_at = None
161+
else:
162+
raise ConnectionNotAvailable()
163+
145164
# Sending the request...
146165

147166
def _send_request_headers(self, request: Request) -> None:
@@ -279,18 +298,13 @@ def is_available(self) -> bool:
279298
return self._state == HTTPConnectionState.IDLE
280299

281300
def has_expired(self) -> bool:
282-
now = time.monotonic()
283-
keepalive_expired = self._expire_at is not None and now > self._expire_at
284-
285-
# If the HTTP connection is idle but the socket is readable, then the
286-
# only valid state is that the socket is about to return b"", indicating
287-
# a server-initiated disconnect.
288-
server_disconnected = (
289-
self._state == HTTPConnectionState.IDLE
290-
and self._network_stream.get_extra_info("is_readable")
291-
)
301+
if self._state == HTTPConnectionState.SERVER_DISCONNECTED:
302+
# Connection that is disconnected by the server is considered expired.
303+
# Pool then cleans up this connection by closing it.
304+
return True
292305

293-
return keepalive_expired or server_disconnected
306+
now = time.monotonic()
307+
return self._expire_at is not None and now > self._expire_at
294308

295309
def is_idle(self) -> bool:
296310
return self._state == HTTPConnectionState.IDLE

‎httpcore/_sync/http2.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,7 @@ def handle_request(self, request: Request) -> Response:
9393
f"to {self._origin}"
9494
)
9595

96-
with self._state_lock:
97-
if self._state in (HTTPConnectionState.ACTIVE, HTTPConnectionState.IDLE):
98-
self._request_count += 1
99-
self._expire_at = None
100-
self._state = HTTPConnectionState.ACTIVE
101-
else:
102-
raise ConnectionNotAvailable()
96+
self._update_state()
10397

10498
with self._init_lock:
10599
if not self._sent_connection_init:
@@ -184,6 +178,15 @@ def handle_request(self, request: Request) -> Response:
184178

185179
raise exc
186180

181+
def _update_state(self) -> None:
182+
with self._state_lock:
183+
if self._state in (HTTPConnectionState.ACTIVE, HTTPConnectionState.IDLE):
184+
self._request_count += 1
185+
self._expire_at = None
186+
self._state = HTTPConnectionState.ACTIVE
187+
else:
188+
raise ConnectionNotAvailable()
189+
187190
def _send_connection_init(self, request: Request) -> None:
188191
"""
189192
The HTTP/2 connection requires some initial setup before we can start

0 commit comments

Comments
 (0)