Skip to content

Comments

Improve pool connection handling#145

Open
nils-borrmann-tacto wants to merge 5 commits intolong2ice:devfrom
nils-borrmann-tacto:improve-pool-connection-handling
Open

Improve pool connection handling#145
nils-borrmann-tacto wants to merge 5 commits intolong2ice:devfrom
nils-borrmann-tacto:improve-pool-connection-handling

Conversation

@nils-borrmann-tacto
Copy link
Contributor

I think that the current implementation of the connection pool is not ideal, which leads to some subtle problems.

The connection pool always "refreshes" every connection it takes from the pool, and thereby simply reconnects a connection that had been closed. This happens even though there might be another still alive connection in the pool.

Imagine a situation where the pool has two free_connections. The first one is closed, the second on still alive. I would expect the connection pool to test all connections until it finds one that works and acquires that connection. This removes closed connections from the pool and avoids additional latency from unnecessarily re-establishing a new connection.

I therefore changed the behaviour of the connection acquisition in the pool. The pool no longer "refreshes" connections, but only checks if the connection is still live (by sending a ping). It does so until it finds a live connection.

(There was actually another bug in the refreshing: Re-connecting the connection never actually worked, because the check for if self._opened in connect() always returned True, thus exiting the connect method. This only worked because the ExecutionContext inside execute() finally re-established the connection.)

I also added some debug logs, which helps understanding this behaviour.

Comment on lines +174 to 175
conn = await self._create_connection()
self._acquired_connections.append(conn)
Copy link
Contributor Author

@nils-borrmann-tacto nils-borrmann-tacto Oct 18, 2025

Choose a reason for hiding this comment

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

I think there was a race condition lingering here. Directly after our connection was created, it could be acquired by another coroutine. I fixed this by never putting the connection into the free pool.

Comment on lines +64 to +65
if self.writer.is_closing():
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This avoid an exception when closing an already closed connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant