sksamuel / cohort

Ktor/Vertx spring-actuator style library - healthchecks, logging, database
Apache License 2.0
123 stars 6 forks source link

Explicit lifecycle for HealthChecksRegistry #41

Closed wafna closed 6 months ago

wafna commented 1 year ago

The HikariConnectionsHealthCheck seems to permanently hold connections and prevent the data source from cleaning up.

Below, is a demonstration.

import arrow.continuations.SuspendApp
import arrow.core.Either
import arrow.fx.coroutines.closeable
import arrow.fx.coroutines.continuations.ResourceScope
import arrow.fx.coroutines.resourceScope
import com.sksamuel.cohort.HealthCheckRegistry
import com.sksamuel.cohort.hikari.HikariConnectionsHealthCheck
import com.zaxxer.hikari.HikariConfig
import com.zaxxer.hikari.HikariDataSource
import kotlinx.coroutines.Dispatchers
import kotlin.system.exitProcess
import kotlin.time.DurationUnit
import kotlin.time.toDuration

data class Services(val hikariDataSource: HikariDataSource)

fun createHikariDS(): HikariDataSource =
    HikariConfig().apply {
        jdbcUrl = "jdbc:h2:mem:kjs;DB_CLOSE_DELAY=-1"
        username = "sa"
        password = ""
        maximumPoolSize = 1
    }.let { HikariDataSource(it) }

// Makes all the components into resources and bundles them up.
suspend fun ResourceScope.createServices(dataSource: HikariDataSource): Services =
    Services(
        hikariDataSource = closeable { dataSource }
    )

fun createHealthChecks(ds: HikariDataSource): HealthCheckRegistry =
    HealthCheckRegistry(Dispatchers.Default) {
        register(HikariConnectionsHealthCheck(ds, 1), 5.toDuration(DurationUnit.SECONDS))
    }

fun main() {
    val dataSource = createHikariDS()
    // This causes the server to not exit because it holds a connection on the dataSource.
    val registry = createHealthChecks(dataSource)
    SuspendApp {
        resourceScope {
            Either.catch {
                createServices(dataSource)
                require(false) { "So, does it close?" }
            }.onLeft {
                println("Fatal exception in server: ${it.message}")
            }
        }
    }
    println("DONE")
    // HealthChecks holds a DB connection, which hangs the process.
//    exitProcess(0)
}
sksamuel commented 1 year ago

This health check simply uses ds.hikariPoolMXBean.totalConnections from a HikariDataSource. Maybe you need to call .close() on the source when you're finished and want to shut down.

wafna commented 1 year ago

It does call close on the Hikari CP. You can see it in the debugger. Nevertheless, the CP does not close.

On Sat, Jun 10, 2023 at 5:15 PM Sam @.***> wrote:

This health check simply uses ds.hikariPoolMXBean.totalConnections from a HikariDataSource. Maybe you need to call .close() on the source when you're finished and want to shut down.

— Reply to this email directly, view it on GitHub https://github.com/sksamuel/cohort/issues/41#issuecomment-1585869787, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALGGT3EIG4IILQKGZWNFY3XKUE3TANCNFSM6AAAAAAY7WL2TY . You are receiving this because you authored the thread.Message ID: @.***>

sksamuel commented 1 year ago

Ok it's not the DB pool, it's the registry itself. I'm adding a fix to the pending 2.1.0 release.

sksamuel commented 6 months ago

This was fixed back in 2.1