Fix "Event loop Closed" error #188
Conversation
|
📦 Build Artifacts Available |
There was a problem hiding this comment.
@sjmonson good find on the issue! With the default for it set to 20, though, I am worried about the potential performance implications. It looks like we can call .aclose / .close on shutdown and that would release the connection pools. Could we see if there's a simple way to add that in?
|
Some other suggestions including Mark's:
Call await self._async_client.aclose() inside a del or explicit shutdown() hook
Instantiate the client in the backend constructor or in a FastAPI lifespan hook and share it across tasks; close it when the loop ends
Expose --httpx-max-keepalive CLI flag or GUIDELLM_HTTPX_KEEPALIVE env-var; default to 20, let users drop to 0 if they see the bug.
httpx.Limits(max_keepalive_connections=1, keepalive_expiry=1) keeps a single hot connection per host but is immune to cross-loop reuse in most cases. |
|
📦 Build Artifacts Available |
|
📦 Build Artifacts Available |
|
I just realized that |
|
📦 Build Artifacts Available |
This reverts commit e6a908a.
|
📦 Build Artifacts Available |
|
New patch did not work from some reason. Reverting to the last patch because its known working and has minimal performance impact. Since this is a high priority bug we just need to get this in and can investigate why |
|
📦 Build Artifacts Available |
|
📦 Build Artifacts Available |
3 participants
By default httpx will keep the last few connections open in a pool for re-use. Since we create the httpx connection inside an asyncio task, it is bound to the lifetime of the task rather than the httpx client. Thus when the client attempts to reuse the connection in a new task it fails. There may be performance implications for this change, but since the default is only to re-use the last 5 connections, its likely minor. (ref)
Fixes: #147