WIP Add connection pooling for CIO client #4407
Conversation
| @@ -288,9 +287,8 @@ internal class Endpoint( | |||
| return connectTimeout to socketTimeout | |||
| } | |||
|
|
|||
| private fun releaseConnection() { | |||
| val address = connectionAddress ?: return | |||
There was a problem hiding this comment.
Can we remove the field connectionAddress?
| ) { | ||
| private val connectionsLimit: Int, | ||
| private val addressConnectionsLimit: Int, | ||
| private val keepAliveTime: Long = 30_000, // Default keep-alive time in milliseconds |
| private fun getPooledConnection(address: InetSocketAddress): PooledConnection? { | ||
| LOG.trace { "Checking for pooled connection for address: $address" } | ||
| val connections = connectionPool[address] ?: return null | ||
| val currentTime = GMTDate().timestamp |
There was a problem hiding this comment.
Just wondering why we don't use kotlinx.datetime? Are we waiting for a stable version?
There was a problem hiding this comment.
Found the issue: KTOR-2721 Migrate from GMTDate to kotlinx.datetime.Instant
| LOG.trace { "Releasing connection for address: ${socket.remoteAddress}" } | ||
|
|
||
| if (socket.isClosed) { | ||
| LOG.warn("Attempted to release a closed connection for address: ${socket.remoteAddress}") |
There was a problem hiding this comment.
On JVM sun.nio.ch.SocketChannelImpl.getRemoteAddress calls ensureOpen under the hood, so here will be a crash
| fun release(socket: Socket) { | ||
| LOG.trace { "Releasing connection for address: ${socket.remoteAddress}" } | ||
|
|
||
| if (socket.isClosed) { |
There was a problem hiding this comment.
This flag seems out of sync with SocketChannel.isOpen on JVM. As a result, we put closed connections to the pool and get a lot of test failures with java.nio.channels.ClosedChannelException
| it.close() | ||
| return@removeAll true | ||
| } | ||
| false |
There was a problem hiding this comment.
Should we also remove closed connections here? Is it possible to have a closed connection in the pool?
No description provided.