From 4f4b454c47e4f94475cd33cd243a688130d6ceec Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Fri, 27 Sep 2024 11:08:48 +0800 Subject: [PATCH] Allow to configure `closeNetworkTimeout` instead of hardcoding 15s See https://github.com/brettwooldridge/HikariCP/issues/2231 --- README.md | 4 +++ .../java/com/zaxxer/hikari/HikariConfig.java | 20 +++++++++++++ .../com/zaxxer/hikari/HikariConfigMXBean.java | 21 +++++++++++++- .../java/com/zaxxer/hikari/pool/PoolBase.java | 7 +++-- .../com/zaxxer/hikari/pool/TestMBean.java | 6 +++- .../zaxxer/hikari/pool/TestValidation.java | 28 +++++++++++++++++++ 6 files changed, 81 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b1e390aa..74c1b07d 100644 --- a/README.md +++ b/README.md @@ -274,6 +274,10 @@ connection attempt, and the pool will start immediately while trying to obtain c in the background. Consequently, later efforts to obtain a connection may fail. *Default: 1* +⏳``closeNetworkTimeout``
+This property set network timeout in milliseconds on connection close if the JDBC driver supports. +*Default: 15000* + ❎``isolateInternalQueries``
This property determines whether HikariCP isolates internal pool queries, such as the connection alive test, in their own transaction. Since these are typically read-only diff --git a/src/main/java/com/zaxxer/hikari/HikariConfig.java b/src/main/java/com/zaxxer/hikari/HikariConfig.java index f0cc081c..edc55236 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfig.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfig.java @@ -50,6 +50,7 @@ public class HikariConfig implements HikariConfigMXBean private static final char[] ID_CHARACTERS = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ".toCharArray(); private static final long CONNECTION_TIMEOUT = SECONDS.toMillis(30); + private static final long CLOSE_NETWORK_TIMEOUT = SECONDS.toMillis(15); private static final long VALIDATION_TIMEOUT = SECONDS.toMillis(5); private static final long SOFT_TIMEOUT_FLOOR = Long.getLong("com.zaxxer.hikari.timeoutMs.floor", 250L); private static final long IDLE_TIMEOUT = MINUTES.toMillis(10); @@ -64,6 +65,7 @@ public class HikariConfig implements HikariConfigMXBean private volatile String catalog; private volatile long connectionTimeout; private volatile long validationTimeout; + private volatile long closeNetworkTimeout; private volatile long idleTimeout; private volatile long leakDetectionThreshold; private volatile long maxLifetime; @@ -121,6 +123,7 @@ public class HikariConfig implements HikariConfigMXBean maxPoolSize = -1; maxLifetime = MAX_LIFETIME; connectionTimeout = CONNECTION_TIMEOUT; + closeNetworkTimeout = CLOSE_NETWORK_TIMEOUT; validationTimeout = VALIDATION_TIMEOUT; idleTimeout = IDLE_TIMEOUT; initializationFailTimeout = 1; @@ -199,6 +202,23 @@ public class HikariConfig implements HikariConfigMXBean } } + /** {@inheritDoc} */ + @Override + public long getCloseNetworkTimeout() + { + return closeNetworkTimeout; + } + + /** {@inheritDoc} */ + @Override + public void setCloseNetworkTimeout(long closeNetworkTimeoutMs) + { + if (closeNetworkTimeoutMs < 0) { + throw new IllegalArgumentException("closeNetworkTimeout cannot be negative"); + } + this.closeNetworkTimeout = closeNetworkTimeoutMs; + } + /** {@inheritDoc} */ @Override public long getIdleTimeout() diff --git a/src/main/java/com/zaxxer/hikari/HikariConfigMXBean.java b/src/main/java/com/zaxxer/hikari/HikariConfigMXBean.java index 10539114..7db806de 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfigMXBean.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfigMXBean.java @@ -19,9 +19,10 @@ package com.zaxxer.hikari; import com.zaxxer.hikari.util.Credentials; /** - * The javax.management MBean for a Hikari pool configuration. + * The {@code javax.management} MBean for a Hikari pool configuration. * * @author Brett Wooldridge + * @author Yanming Zhou */ public interface HikariConfigMXBean { @@ -43,6 +44,24 @@ public interface HikariConfigMXBean */ void setConnectionTimeout(long connectionTimeoutMs); + /** + * Get the network timeout in milliseconds for {@link java.sql.Connection#close()}. + * {@code 0} means no timeout will be set. + * {@link javax.sql.DataSource#getConnection()}. + * + * @return the close timeout in milliseconds + * @see java.sql.Connection#setNetworkTimeout(java.util.concurrent.Executor, int) + */ + long getCloseNetworkTimeout(); + + /** + * Set the network timeout in milliseconds for {@link java.sql.Connection#close()}. + * + * @param closeNetworkTimeoutMs the close timeout in milliseconds, {@code 0} means no timeout will be set + * @see java.sql.Connection#setNetworkTimeout(java.util.concurrent.Executor, int) + */ + void setCloseNetworkTimeout(long closeNetworkTimeoutMs); + /** * Get the maximum number of milliseconds that the pool will wait for a connection to be validated as * alive. diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 6ce3b616..1e835f38 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -135,9 +135,10 @@ abstract class PoolBase // continue with the close even if setNetworkTimeout() throws try (connection) { - if (!connection.isClosed()) - setNetworkTimeout(connection, SECONDS.toMillis(15)); - } catch (SQLException e) { + if (!connection.isClosed() && config.getCloseNetworkTimeout() > 0) { + setNetworkTimeout(connection, config.getCloseNetworkTimeout()); + } + } catch (SQLException e) { // ignore } } diff --git a/src/test/java/com/zaxxer/hikari/pool/TestMBean.java b/src/test/java/com/zaxxer/hikari/pool/TestMBean.java index 51eb6c08..8e7b9c8e 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestMBean.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestMBean.java @@ -136,12 +136,13 @@ public class TestMBean } @Test - public void testMBeanConnectionTimeoutChange() throws SQLException { + public void testMBeanConnectionTimeoutAndCloseNetworkTimeoutChange() throws SQLException { HikariConfig config = newHikariConfig(); config.setMinimumIdle(1); config.setMaximumPoolSize(2); config.setRegisterMbeans(true); config.setConnectionTimeout(2800); + config.setCloseNetworkTimeout(2300); config.setConnectionTestQuery("VALUES 1"); config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource"); @@ -150,12 +151,14 @@ public class TestMBean try (HikariDataSource ds = new HikariDataSource(config)) { HikariConfigMXBean hikariConfigMXBean = ds.getHikariConfigMXBean(); assertEquals(2800, hikariConfigMXBean.getConnectionTimeout()); + assertEquals(2300, hikariConfigMXBean.getCloseNetworkTimeout()); final StubDataSource stubDataSource = ds.unwrap(StubDataSource.class); // connection acquisition takes more than 0 ms in a real system stubDataSource.setConnectionAcquisitionTime(1200); hikariConfigMXBean.setConnectionTimeout(1000); + hikariConfigMXBean.setCloseNetworkTimeout(1000); quietlySleep(500); @@ -165,6 +168,7 @@ public class TestMBean } catch (SQLException e) { assertEquals(1000, ds.getConnectionTimeout()); + assertEquals(1000, ds.getCloseNetworkTimeout()); } } finally { diff --git a/src/test/java/com/zaxxer/hikari/pool/TestValidation.java b/src/test/java/com/zaxxer/hikari/pool/TestValidation.java index 820734be..b4445c02 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestValidation.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestValidation.java @@ -31,6 +31,7 @@ import com.zaxxer.hikari.HikariConfig; /** * @author Brett Wooldridge + * @author Yanming Zhou */ public class TestValidation { @@ -252,4 +253,31 @@ public class TestValidation // pass } } + + @Test + public void validateZeroCloseNetworkTimeout() + { + try { + HikariConfig config = newHikariConfig(); + config.setCloseNetworkTimeout(0); + config.validate(); + assertEquals(0, config.getCloseNetworkTimeout()); + } + catch (IllegalArgumentException ise) { + // pass + } + } + + @Test + public void validateInvalidCloseNetworkTimeout() + { + try { + HikariConfig config = newHikariConfig(); + config.setCloseNetworkTimeout(-1); + fail(); + } + catch (IllegalArgumentException ise) { + assertTrue(ise.getMessage().contains("closeNetworkTimeout cannot be negative")); + } + } }