JetBrains / Exposed

Kotlin SQL Framework
http://jetbrains.github.io/Exposed/
Apache License 2.0
8.24k stars 686 forks source link

Connection leak. #1167

Closed neetkee closed 3 years ago

neetkee commented 3 years ago

Hello, I'm is using some spring libraries, that call queries via jdbcTemplate and transactionTemplate. I have encountered a problem. If exposed transactions are used with Spring transactionTemplate there is a connection leak.

Issue project: https://github.com/neetkee/exposed-jdbc-template-demo. You can just run tests to reproduce the problem. Error:

Could not open JDBC Connection for transaction; nested exception is java.sql.SQLTransientConnectionException: HikariPool-1 - Connection is not available, request timed out after 30005ms.
org.springframework.transaction.CannotCreateTransactionException: Could not open JDBC Connection for transaction; nested exception is java.sql.SQLTransientConnectionException: HikariPool-1 - Connection is not available, request timed out after 30005ms.
    at org.springframework.jdbc.datasource.DataSourceTransactionManager.doBegin(DataSourceTransactionManager.java:309)
    at org.jetbrains.exposed.spring.SpringTransactionManager.doBegin(SpringTransactionManager.kt:41)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.startTransaction(AbstractPlatformTransactionManager.java:400)
    at
Omitted
dtbullock commented 3 years ago

What the tests don't show is that when using Spring transactionTemplate is used by itself, there is not a leak, and that the addition of Exposed is the cause of the leaking.

Also, testWithoutTransactions() is not executed the same number of times that testWithTransactions() is.

But if your test-results hold even when repeating each test the same number of time, then, since even in testWithoutTransactions(), Exposed is still using transactions, it would suggest that the connection-hogging behaviour is due to Spring!

Can you address these matters in your otherwise excellent test repro?

neetkee commented 3 years ago

Updated tests to precisely indicate leak conditions.

neetkee commented 3 years ago

Additional debug logging:

2021-02-24 23:10:20.441 DEBUG 29094 --- [    Test worker] o.j.e.spring.SpringTransactionManager    : Could not reset JDBC Connection after transaction

java.sql.SQLException: Connection is closed
    at com.zaxxer.hikari.pool.ProxyConnection$ClosedConnection.lambda$getClosedConnection$0(ProxyConnection.java:515) ~[HikariCP-3.4.5.jar:na]
    at com.sun.proxy.$Proxy64.setAutoCommit(Unknown Source) ~[na:na]
    at com.zaxxer.hikari.pool.ProxyConnection.setAutoCommit(ProxyConnection.java:414) ~[HikariCP-3.4.5.jar:na]
    at com.zaxxer.hikari.pool.HikariProxyConnection.setAutoCommit(HikariProxyConnection.java) ~[HikariCP-3.4.5.jar:na]
    at org.springframework.jdbc.datasource.DataSourceTransactionManager.doCleanupAfterCompletion(DataSourceTransactionManager.java:378) ~[spring-jdbc-5.3.3.jar:5.3.3]
    at org.jetbrains.exposed.spring.SpringTransactionManager.doCleanupAfterCompletion(SpringTransactionManager.kt:52) [spring-transaction-0.29.1.jar:na]
    at org.jetbrains.exposed.spring.SpringTransactionManager$SpringTransaction.close(SpringTransactionManager.kt:137) [spring-transaction-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.Transaction.close(Transaction.kt) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.closeStatementsAndConnection(ThreadLocalTransactionManager.kt:247) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt$inTopLevelTransaction$1.invoke(ThreadLocalTransactionManager.kt:185) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt$inTopLevelTransaction$2.invoke(ThreadLocalTransactionManager.kt:191) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.keepAndRestoreTransactionRefAfterRun(ThreadLocalTransactionManager.kt:199) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.inTopLevelTransaction(ThreadLocalTransactionManager.kt:190) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt$transaction$1.invoke(ThreadLocalTransactionManager.kt:148) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.keepAndRestoreTransactionRefAfterRun(ThreadLocalTransactionManager.kt:199) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.transaction(ThreadLocalTransactionManager.kt:120) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.transaction(ThreadLocalTransactionManager.kt:118) [exposed-core-0.29.1.jar:na]
    at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.transaction$default(ThreadLocalTransactionManager.kt:117) [exposed-core-0.29.1.jar:na]
    at com.example.demo.BookService.testWithSpringAndExposedTransactions(BookService.kt:20) [main/:na]
    at com.example.demo.DemoApplicationTests.testWithSpringAndExposedTransactions(DemoApplicationTests.kt:37) [test/:na]
AleksanderBrzozowski commented 3 years ago

We had the same issue in our project, everything worked for a small load, but when we did performance tests, connection leaks started to occur.

We have created something like this to fix this:

// this class is exposed as a bean
class TransactionOperations(datasource: Datasource) {
    private val db = Database.connect(datasource)

    operator fun <T> invoke(block: () -> T): T {
        //exposed transaction
        return transaction(..., db = db) { callback() }
    }
}

Since you don't use @Transactional annotation, you could try it to :)

Tapac commented 3 years ago

@neetkee , thank you very much for providing such a complex sample. Looks like I was able to debug and fix that tricky case. I copy-pasted your test-sample into Exposed project. I will try to release the fix by the end of the week