Fix leak detection wrongly closing actively-in-use connections during…#149
Merged
Conversation
… pool reset Problem In production we saw this warning on a read-only pool: Connection [name[...] startTime[...] busySeconds[0] createdBy[null] stmt[select ...]] not found in BusyList? busySeconds[0] shows the connection had just been checked out — it was not a leak, yet it had already been removed from the busy list. Root cause BusyConnectionBuffer.closeBusyConnections(leakTimeMinutes) decided whether a busy connection was a leak using lastUsedTime() — the time the connection was last returned to the pool (intended for trimming idle/free connections). The correct field is startUseTime() (the time the connection was checked out for its current use), which is even documented as "Used to detect busy connections that could be leaks" but was unused. As a result, when reset() runs (read-only failover or a DB up/down heartbeat event), a connection that had sat idle in the free list longer than leakTimeMinutes and was then freshly checked out would be misidentified as a leak and force-closed mid-query. When the owning thread finished and called close(), its busy slot was already gone → "not found in BusyList?". Fix - closeBusyConnections now uses startUseTime() (continuous checkout duration) instead of lastUsedTime(). Connections in genuine active use are left to be closed normally when returned, as the reset() Javadoc already intends. - Made PooledConnection.startUseTime() package-private. - Removed a stray System.out.println left in closeBusyConnection. - Moved the test-only pool == null guard above a TRACE log that dereferenced a null pstmtCache. Tests Added BusyConnectionBufferTest.closeBusyConnections_onlyClosesLeaks_notActiveConnections, which fails on the old behaviour and passes with the fix. Full module suite green (62 passed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… pool reset
Problem
In production we saw this warning on a read-only pool:
Connection [name[...] startTime[...] busySeconds[0] createdBy[null] stmt[select ...]] not found in BusyList?
busySeconds[0] shows the connection had just been checked out — it was not a leak, yet it had already been removed from the busy list.
Root cause
BusyConnectionBuffer.closeBusyConnections(leakTimeMinutes) decided whether a busy connection was a leak using lastUsedTime() — the time the connection was last returned to the pool (intended for trimming idle/free connections). The correct field is startUseTime() (the time the connection was checked out for its current use), which is even documented as "Used to detect busy connections that could be leaks" but was unused.
As a result, when reset() runs (read-only failover or a DB up/down heartbeat event), a connection that had sat idle in the free list longer than leakTimeMinutes and was then freshly checked out would be misidentified as a leak and force-closed mid-query. When the owning thread finished and called close(), its busy slot was already gone → "not found in BusyList?".
Fix
Tests
Added BusyConnectionBufferTest.closeBusyConnections_onlyClosesLeaks_notActiveConnections, which fails on the old behaviour and passes with the fix. Full module suite green (62 passed).