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

[Postgres] JdbcDriver is not closing connection in case of sql exception #2306

Closed
dbacinski opened this issue Apr 22, 2021 · 1 comment · Fixed by #2319
Closed

[Postgres] JdbcDriver is not closing connection in case of sql exception #2306

dbacinski opened this issue Apr 22, 2021 · 1 comment · Fixed by #2319
Labels
Milestone

Comments

@dbacinski
Copy link

dbacinski commented Apr 22, 2021

Runtime Environment
SQLDelight version: 1.5.0-SNAPSHOT
Application OS: Kotlin + Hikari + Postgres

Describe the bug

  1. I have a query witch is failing with sqlexception like, which is totally fine, error happens:
ERROR: ba1e72af-7a2d-4af6-8b3a-5b613964abb7: org.postgresql.util.PSQLException: ERROR: duplicate key value violates unique constraint "idx_productpriceandstock_productid"
  Detail: Key (product_id)=(34655) already exists.
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2433)
	at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2178)
	at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:306)
	at org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:441)
	at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:365)
	at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:155)
	at org.postgresql.jdbc.PgPreparedStatement.execute(PgPreparedStatement.java:144)
	at com.zaxxer.hikari.pool.ProxyPreparedStatement.execute(ProxyPreparedStatement.java:44)
	at com.zaxxer.hikari.pool.HikariProxyPreparedStatement.execute(HikariProxyPreparedStatement.java)
	at com.squareup.sqldelight.sqlite.driver.SqliteJdbcPreparedStatement.execute(JdbcDriver.kt:180)
	at com.squareup.sqldelight.sqlite.driver.JdbcDriver.execute(JdbcDriver.kt:108)
	at gt.kk.catalog.schema.catalog.ProductPriceAndStockQueriesImpl.insert(ProductDatabaseImpl.kt:4827)
  1. But when it happens connection is never being released to the pool and finally, when more such errors occur, the whole pool gets filled in and blocked.
  2. Hikari is also reporting connection leak as:
2021-04-22 11:03:10 [Catalog-Pool housekeeper] com.zaxxer.hikari.pool.ProxyLeakTask.run()
WARN: Connection leak detection triggered for org.postgresql.jdbc.PgConnection@16c3ca31 on thread HTTP-worker-1, stack trace follows: java.lang.Exception: Apparent connection leak detected
	at com.zaxxer.hikari.HikariDataSource.getConnection(HikariDataSource.java:100)
	at com.squareup.sqldelight.sqlite.driver.JdbcDrivers$asJdbcDriver$1.getConnection(JdbcDriver.kt:18)
	at com.squareup.sqldelight.sqlite.driver.JdbcDriver.connectionAndClose(JdbcDriver.kt:93)
	at com.squareup.sqldelight.sqlite.driver.JdbcDriver.execute(JdbcDriver.kt:104)
	at gt.kk.catalog.schema.catalog.ProductPriceAndStockQueriesImpl.insert(ProductDatabaseImpl.kt:4827)

In the sqldelight JdbcDriver I see code like this:

  override fun execute(
    identifier: Int?,
    sql: String,
    parameters: Int,
    binders: (SqlPreparedStatement.() -> Unit)?
  ) {
    val (connection, onClose) = connectionAndClose()
    connection.prepareStatement(sql).use { jdbcStatement ->
      SqliteJdbcPreparedStatement(jdbcStatement)
        .apply { if (binders != null) this.binders() }
        .execute()
    }
    onClose()
  }

This is a closing connection when all if fine but in case of exception, there is no onClose call in here.

I see use block that should close the connection but apparently, it is not working.

If use would work correctly we probably wouldn't need to call onClose() explicitly here at all.

@dbacinski dbacinski added the bug label Apr 22, 2021
@dbacinski
Copy link
Author

what we probably need is something like:

    try {
        return block(this)
        onClose()
    } catch (e: Throwable) {
        onClose()
        throw e
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants