Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close connections marked as evicted instead of returning them to the pool #2157

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

TAregger
Copy link
Contributor

Description

According to the documentation, connections that were in use during a soft eviction, are effectively evicted when returned to the pool. See Javadoc of com.zaxxer.hikari.HikariPoolMXBean.softEvictConnections

Evict currently idle connections from the pool, and mark active (in-use) connections for eviction when they are
returned to the pool.

That's not how it's currently implemented though. Connections that are marked as evicted are being closed when the connection is tried to be borrowed from the pool.

With this PR, connections marked as evicted are additionally closed when returned to the pool.
Because of race conditions, it's still possible that connections that are marked as evicted are being returned to the pool.
Therefore, I left the logic in com.zaxxer.hikari.pool.HikariPool.getConnection(long) as is.

Why?

We use a database cluster in an active/passive setup. The JDBC connection string ensures that connections are always established with the active node.
On database maintenance we are able to switch the roles of the nodes. We then want to evict all current connections (connected to the now passive node) to ensure that new connections are established with the now active node.

With low volume of SQL executions it can take some time until a connection marked as evicted is being borrowed from the pool.
We would therefore have to adjust the minimumIdleTime to ensure that connections are being re-established in a timely manner.

@brettwooldridge brettwooldridge merged commit a671b6e into brettwooldridge:dev Sep 23, 2024
1 check passed
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.

2 participants