From ebad877c527be71e1ac8df5c7ab58389dea95fb2 Mon Sep 17 00:00:00 2001 From: Morten Svanaes Date: Wed, 7 Oct 2026 16:36:00 +0800 Subject: [PATCH] fix: apply Hikari pool sizing and lifetime settings [2.42] The Hikari pool was built before its sizing was set, so minimumIdle was fixed at 10 and connection.pool.min_idle/max_idle_time/max_lifetime_seconds were ignored. Integration tests set min_idle=3, but every cached Spring test context still kept 10 connections, exhausting the Postgres test container (max_connections=100) and failing context loads with "too many clients" (e.g. CacheInvalidationMessagePublisherTest). Configure HikariConfig before constructing the data source and add the connection.pool.min_idle, keep_alive_time_seconds and max_lifetime_seconds keys as on 2.43 (#21261, #23015). Defaults match the previous effective Hikari values; max_idle_time is only applied to Hikari when set explicitly because its 7200s default is sized for c3p0. --- .../dhis/external/conf/ConfigurationKey.java | 15 ++++ .../dhis/datasource/DatabasePoolUtils.java | 35 +++++++-- .../datasource/DatabasePoolUtilsTest.java | 74 +++++++++++++++---- 3 files changed, 104 insertions(+), 20 deletions(-) diff --git a/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/ConfigurationKey.java b/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/ConfigurationKey.java index 69bfbd90dae7..a051fee66ca5 100644 --- a/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/ConfigurationKey.java +++ b/dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/ConfigurationKey.java @@ -199,6 +199,21 @@ public enum ConfigurationKey { */ ANALYTICS_CONNECTION_POOL_MAX_IDLE_TIME("analytics.connection.pool.max_idle_time", "7200", false), + /** Minimum number of idle connections to maintain, Hikari only (default: 10). */ + CONNECTION_POOL_MIN_IDLE("connection.pool.min_idle", "10", false), + + /** + * Interval to keep idle connections alive. Does not reset idle timeout, Hikari only (default: 2 + * minutes). + */ + CONNECTION_POOL_KEEP_ALIVE_TIME_SECONDS("connection.pool.keep_alive_time_seconds", "120", false), + + /** + * Connection max lifetime. An in-use connection will never be retired, only when it is idle will + * it be removed. Hikari only (default: 30 minutes). + */ + CONNECTION_POOL_MAX_LIFETIME_SECONDS("connection.pool.max_lifetime_seconds", "1800", false), + /** * Number of seconds that Connections in excess of minPoolSize should be permitted to remain idle * in the pool before being culled (default: 0). diff --git a/dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/datasource/DatabasePoolUtils.java b/dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/datasource/DatabasePoolUtils.java index 8523e1a34ed9..49e4b7a8df95 100644 --- a/dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/datasource/DatabasePoolUtils.java +++ b/dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/datasource/DatabasePoolUtils.java @@ -32,6 +32,7 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static java.lang.Integer.parseInt; import static java.lang.Long.parseLong; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.hisp.dhis.external.conf.ConfigurationKey.ANALYTICS_CONNECTION_PASSWORD; import static org.hisp.dhis.external.conf.ConfigurationKey.ANALYTICS_CONNECTION_POOL_ACQUIRE_INCR; import static org.hisp.dhis.external.conf.ConfigurationKey.ANALYTICS_CONNECTION_POOL_ACQUIRE_RETRY_ATTEMPTS; @@ -57,9 +58,12 @@ import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_ACQUIRE_RETRY_DELAY; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_IDLE_CON_TEST_PERIOD; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_INITIAL_SIZE; +import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_KEEP_ALIVE_TIME_SECONDS; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_MAX_IDLE_TIME; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_MAX_IDLE_TIME_EXCESS_CON; +import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_MAX_LIFETIME_SECONDS; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_MAX_SIZE; +import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_MIN_IDLE; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_MIN_SIZE; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_NUM_THREADS; import static org.hisp.dhis.external.conf.ConfigurationKey.CONNECTION_POOL_TEST_ON_CHECKIN; @@ -239,6 +243,14 @@ private static DataSource createHikariDbPool( dhisConfig.getProperty(mapper.getConfigKey(CONNECTION_POOL_MAX_SIZE)))); final String connectionTestQuery = dhisConfig.getProperty(mapper.getConfigKey(CONNECTION_POOL_TEST_QUERY)); + final int minIdleConnections = + parseInt(dhisConfig.getProperty(mapper.getConfigKey(CONNECTION_POOL_MIN_IDLE))); + final int keepAliveTimeSeconds = + parseInt( + dhisConfig.getProperty(mapper.getConfigKey(CONNECTION_POOL_KEEP_ALIVE_TIME_SECONDS))); + final int maxLifetimeSeconds = + parseInt(dhisConfig.getProperty(mapper.getConfigKey(CONNECTION_POOL_MAX_LIFETIME_SECONDS))); + final ConfigurationKey maxIdleTimeKey = mapper.getConfigKey(CONNECTION_POOL_MAX_IDLE_TIME); HikariConfig hc = new HikariConfig(); hc.setPoolName(config.getDataSourceName()); @@ -250,6 +262,22 @@ private static DataSource createHikariDbPool( hc.addDataSourceProperty("prepStmtCacheSize", "250"); hc.addDataSourceProperty("prepStmtCacheSqlLimit", "2048"); hc.setConnectionTestQuery(connectionTestQuery); + + // Configure the pool before constructing the data source: Hikari validates and starts the pool + // in the constructor, so pool sizing set afterwards is applied to an already filled pool. + hc.setConnectionTimeout(connectionTimeout); + hc.setValidationTimeout(validationTimeout); + hc.setMaximumPoolSize(maxPoolSize); + hc.setMinimumIdle(minIdleConnections); + hc.setKeepaliveTime(SECONDS.toMillis(keepAliveTimeSeconds)); + hc.setMaxLifetime(SECONDS.toMillis(maxLifetimeSeconds)); + + // The max idle time default (7200s) is sized for c3p0 and exceeds Hikari's max lifetime, which + // would disable idle eviction. Only apply it to Hikari when configured explicitly. + if (dhisConfig.hasProperty(maxIdleTimeKey)) { + hc.setIdleTimeout(SECONDS.toMillis(parseLong(dhisConfig.getProperty(maxIdleTimeKey)))); + } + final String leakThresholdStr = dhisConfig.getProperty(mapper.getConfigKey(CONNECTION_POOL_WARN_MAX_AGE)); @@ -280,12 +308,7 @@ private static DataSource createHikariDbPool( hc.setMetricsTrackerFactory(new MicrometerMetricsTrackerFactory(meterRegistry)); } - HikariDataSource ds = new HikariDataSource(hc); - ds.setConnectionTimeout(connectionTimeout); - ds.setValidationTimeout(validationTimeout); - ds.setMaximumPoolSize(maxPoolSize); - - return ds; + return new HikariDataSource(hc); } /** Create a data source based on a C3p0 connection pool. */ diff --git a/dhis-2/dhis-support/dhis-support-hibernate/src/test/java/org/hisp/dhis/datasource/DatabasePoolUtilsTest.java b/dhis-2/dhis-support/dhis-support-hibernate/src/test/java/org/hisp/dhis/datasource/DatabasePoolUtilsTest.java index 3a76253604ad..ec03eb284ea2 100644 --- a/dhis-2/dhis-support/dhis-support-hibernate/src/test/java/org/hisp/dhis/datasource/DatabasePoolUtilsTest.java +++ b/dhis-2/dhis-support/dhis-support-hibernate/src/test/java/org/hisp/dhis/datasource/DatabasePoolUtilsTest.java @@ -29,11 +29,14 @@ */ package org.hisp.dhis.datasource; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import com.mchange.v2.c3p0.ComboPooledDataSource; +import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; @@ -118,6 +121,42 @@ void testCreateDbPoolWhenDbPoolTypeIsC3P0() throws PropertyVetoException, SQLExc @Test void testCreateDbPoolWhenDbPoolTypeIsHikari() throws PropertyVetoException, SQLException { + DhisConfigurationProvider mockDhisConfigurationProvider = mockHikariConfiguration(); + given(mockDhisConfigurationProvider.hasProperty(ConfigurationKey.CONNECTION_POOL_MAX_IDLE_TIME)) + .willReturn(true); + given(mockDhisConfigurationProvider.getProperty(ConfigurationKey.CONNECTION_POOL_MAX_IDLE_TIME)) + .willReturn("25"); + + DataSource dataSource = + DatabasePoolUtils.createDbPool( + hikariPoolConfig(mockDhisConfigurationProvider), meterRegistry); + + try (HikariDataSource hikari = assertInstanceOf(HikariDataSource.class, dataSource)) { + assertEquals(5, hikari.getMaximumPoolSize()); + assertEquals(3, hikari.getMinimumIdle()); + assertEquals(SECONDS.toMillis(60), hikari.getKeepaliveTime()); + assertEquals(SECONDS.toMillis(1200), hikari.getMaxLifetime()); + assertEquals(SECONDS.toMillis(25), hikari.getIdleTimeout()); + } + } + + @Test + void testCreateDbPoolWhenDbPoolTypeIsHikariKeepsDefaultIdleTimeoutWhenNotConfigured() + throws PropertyVetoException, SQLException { + DhisConfigurationProvider mockDhisConfigurationProvider = mockHikariConfiguration(); + given(mockDhisConfigurationProvider.getProperty(ConfigurationKey.CONNECTION_POOL_MAX_IDLE_TIME)) + .willReturn("7200"); + + DataSource dataSource = + DatabasePoolUtils.createDbPool( + hikariPoolConfig(mockDhisConfigurationProvider), meterRegistry); + + try (HikariDataSource hikari = assertInstanceOf(HikariDataSource.class, dataSource)) { + assertEquals(new HikariConfig().getIdleTimeout(), hikari.getIdleTimeout()); + } + } + + private static DhisConfigurationProvider mockHikariConfiguration() { DhisConfigurationProvider mockDhisConfigurationProvider = mock(DhisConfigurationProvider.class); given(mockDhisConfigurationProvider.getProperty(ConfigurationKey.CONNECTION_DRIVER_CLASS)) .willReturn("org.hisp.dhis.datasource.StubDriver"); @@ -127,20 +166,27 @@ void testCreateDbPoolWhenDbPoolTypeIsHikari() throws PropertyVetoException, SQLE mockDhisConfigurationProvider.getProperty( ConfigurationKey.CONNECTION_POOL_VALIDATION_TIMEOUT)) .willReturn("250"); + given(mockDhisConfigurationProvider.getProperty(ConfigurationKey.CONNECTION_POOL_MIN_IDLE)) + .willReturn("3"); + given( + mockDhisConfigurationProvider.getProperty( + ConfigurationKey.CONNECTION_POOL_KEEP_ALIVE_TIME_SECONDS)) + .willReturn("60"); + given( + mockDhisConfigurationProvider.getProperty( + ConfigurationKey.CONNECTION_POOL_MAX_LIFETIME_SECONDS)) + .willReturn("1200"); + return mockDhisConfigurationProvider; + } - DbPoolConfig.DbPoolConfigBuilder poolConfigBuilder = - DbPoolConfig.builder("test_hikari") - .dbPoolType(DatabasePoolUtils.DbPoolType.HIKARI.name()) - .jdbcUrl("jdbc:fake:db") - .username("") - .password("") - .maxPoolSize("1") - .acquireIncrement("1") - .maxIdleTime(String.valueOf(ThreadLocalRandom.current().nextInt())) - .dhisConfig(mockDhisConfigurationProvider); - - DataSource dataSource = - DatabasePoolUtils.createDbPool(poolConfigBuilder.build(), meterRegistry); - assertInstanceOf(HikariDataSource.class, dataSource); + private static DbPoolConfig hikariPoolConfig(DhisConfigurationProvider dhisConfig) { + return DbPoolConfig.builder("test_hikari") + .dbPoolType(DatabasePoolUtils.DbPoolType.HIKARI.name()) + .jdbcUrl("jdbc:fake:db") + .username("") + .password("") + .maxPoolSize("5") + .dhisConfig(dhisConfig) + .build(); } }