From 616a8321ab9729ee4d5a57c74afe85fb28e68fd5 Mon Sep 17 00:00:00 2001 From: Nitin Date: Fri, 14 Aug 2015 18:49:52 +0530 Subject: [PATCH 1/9] break loop --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 44ec8b28..3ddb423f 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -636,7 +636,10 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener int removable = bag.size() - config.getMinimumIdle(); for (PoolBagEntry bagEntry : bag) { if (connectionBag.reserve(bagEntry)) { - if (removable > 0 && idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout) { + if (removable <= 0) { + break; + } + if (idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout) { closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; } From cefc2af679a157b13c2aafd9df6e681eddc7cb47 Mon Sep 17 00:00:00 2001 From: Nitin Date: Fri, 14 Aug 2015 18:56:21 +0530 Subject: [PATCH 2/9] level up --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 3ddb423f..39e14ed9 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -635,10 +635,10 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener final List bag = connectionBag.values(STATE_NOT_IN_USE); int removable = bag.size() - config.getMinimumIdle(); for (PoolBagEntry bagEntry : bag) { + if (removable <= 0) { + break; + } if (connectionBag.reserve(bagEntry)) { - if (removable <= 0) { - break; - } if (idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout) { closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; From d04f4dd3f1c3cdb356bbbad131e827d95383a9a6 Mon Sep 17 00:00:00 2001 From: Nitin Date: Fri, 14 Aug 2015 19:32:36 +0530 Subject: [PATCH 3/9] log actual error message instead of 'Exception during pool' name 'PoolInitializationException' is descriptive enough --- .../com/zaxxer/hikari/pool/PoolInitializationException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java b/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java index 4265e8a8..4e0c8770 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java @@ -31,6 +31,6 @@ public class PoolInitializationException extends RuntimeException */ public PoolInitializationException(Throwable t) { - super("Exception during pool initialization", t); + super(t); } } From ebe5868fd119951814c1ae0dbf37c200b5ebbcc7 Mon Sep 17 00:00:00 2001 From: Nitin Date: Fri, 14 Aug 2015 20:02:55 +0530 Subject: [PATCH 4/9] changes as suggested align log stats --- .../java/com/zaxxer/hikari/pool/HikariPool.java | 17 +++++++---------- .../pool/PoolInitializationException.java | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 39e14ed9..e67517b2 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -203,7 +203,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener suspendResumeLock.release(); } - logPoolState("Timeout failure "); + logPoolState("Timeout failure\t"); String sqlState = null; final Throwable originalException = lastConnectionFailure.getAndSet(null); if (originalException instanceof SQLException) { @@ -241,7 +241,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener poolState = POOL_SHUTDOWN; LOGGER.info("{} - is closing down.", poolName); - logPoolState("Before closing "); + logPoolState("Before closing\t"); connectionBag.close(); softEvictConnections(); @@ -271,7 +271,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener closeConnectionExecutor.awaitTermination(5L, TimeUnit.SECONDS); } finally { - logPoolState("After closing "); + logPoolState("After closing\t"); poolElf.unregisterMBeans(); metricsTracker.close(); @@ -526,7 +526,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener addConnectionExecutor.execute(new Runnable() { @Override public void run() { - logPoolState("After fill "); + logPoolState("After fill\t"); } }); } @@ -631,14 +631,11 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener previous = now; } - logPoolState("Before cleanup "); + logPoolState("Before cleanup\t"); final List bag = connectionBag.values(STATE_NOT_IN_USE); int removable = bag.size() - config.getMinimumIdle(); for (PoolBagEntry bagEntry : bag) { - if (removable <= 0) { - break; - } - if (connectionBag.reserve(bagEntry)) { + if (removable > 0 && connectionBag.reserve(bagEntry)) { if (idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout) { closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; @@ -649,7 +646,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener } } - logPoolState("After cleanup "); + logPoolState("After cleanup\t"); fillPool(); // Try to maintain minimum connections } diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java b/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java index 4e0c8770..a6d1d25d 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolInitializationException.java @@ -31,6 +31,6 @@ public class PoolInitializationException extends RuntimeException */ public PoolInitializationException(Throwable t) { - super(t); + super("Exception during pool initialization: " + t.getMessage(), t); } } From 6a2bfb758c3c9a955216417653db2937a8dab3cb Mon Sep 17 00:00:00 2001 From: Nitin Date: Sat, 15 Aug 2015 10:50:17 +0530 Subject: [PATCH 5/9] for closing connection, reserve entry only when it is past idle time or evicted, not otherwise --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index e67517b2..06b57cd6 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -627,21 +627,20 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener fillPool(); return; } - else { - previous = now; - } + + previous = now; logPoolState("Before cleanup\t"); final List bag = connectionBag.values(STATE_NOT_IN_USE); int removable = bag.size() - config.getMinimumIdle(); for (PoolBagEntry bagEntry : bag) { - if (removable > 0 && connectionBag.reserve(bagEntry)) { - if (idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout) { + if (removable > 0 && (bagEntry.evicted || (idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout))) { + if (connectionBag.reserve(bagEntry)) { closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; } else { - connectionBag.unreserve(bagEntry); + bagEntry.evicted = true; } } } From 5004e1f4631d345bbad22d243438a66017fdc06b Mon Sep 17 00:00:00 2001 From: Nitin Date: Sat, 15 Aug 2015 10:59:20 +0530 Subject: [PATCH 6/9] setting evicted was 'little' more aggressive --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 06b57cd6..83bf29e6 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -639,9 +639,6 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; } - else { - bagEntry.evicted = true; - } } } From 7685f10303115a2b7ea69bfbd3afa06fdcef51a2 Mon Sep 17 00:00:00 2001 From: Nitin Date: Sat, 15 Aug 2015 12:04:11 +0530 Subject: [PATCH 7/9] check for evicted should not depend on removable --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 83bf29e6..06756142 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -634,7 +634,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener final List bag = connectionBag.values(STATE_NOT_IN_USE); int removable = bag.size() - config.getMinimumIdle(); for (PoolBagEntry bagEntry : bag) { - if (removable > 0 && (bagEntry.evicted || (idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout))) { + if (bagEntry.evicted || ((removable > 0 && idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout))) { if (connectionBag.reserve(bagEntry)) { closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; From c4b0ac352c8ec72be98cb0936dc227d049a9cf7c Mon Sep 17 00:00:00 2001 From: Nitin Date: Sat, 15 Aug 2015 12:20:53 +0530 Subject: [PATCH 8/9] housekeeper should close evicted connections... but forget it :) --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 2 +- src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 06756142..924ea215 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -634,7 +634,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener final List bag = connectionBag.values(STATE_NOT_IN_USE); int removable = bag.size() - config.getMinimumIdle(); for (PoolBagEntry bagEntry : bag) { - if (bagEntry.evicted || ((removable > 0 && idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout))) { + if (removable > 0 && idleTimeout > 0L && clockSource.elapsedMillis(bagEntry.lastAccess, now) > idleTimeout) { if (connectionBag.reserve(bagEntry)) { closeConnection(bagEntry, "(connection passed idleTimeout)"); removable--; diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java index 166438ff..178acc91 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java @@ -94,7 +94,7 @@ public final class PoolBagEntry implements IConcurrentBagEntry pool.closeConnection(PoolBagEntry.this, "(connection reached maxLifetime)"); } else { - // else the connection is "in-use" and we mark it for eviction by pool.releaseConnection() or the housekeeper + // else the connection is "in-use" and we mark it for eviction by pool.releaseConnection() PoolBagEntry.this.evicted = true; } } From 2a90f26145a193df3f14a0cf7c76cee081be6c15 Mon Sep 17 00:00:00 2001 From: Nitin Date: Sat, 15 Aug 2015 15:40:38 +0530 Subject: [PATCH 9/9] log /creation cleanup --- .../java/com/zaxxer/hikari/HikariConfig.java | 26 ++++++++----------- .../com/zaxxer/hikari/pool/HikariPool.java | 2 +- .../java/com/zaxxer/hikari/pool/PoolElf.java | 2 +- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/HikariConfig.java b/src/main/java/com/zaxxer/hikari/HikariConfig.java index 945bba11..1d4f3f46 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfig.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfig.java @@ -745,8 +745,6 @@ public class HikariConfig implements HikariConfigMXBean public void validate() { - Logger logger = LoggerFactory.getLogger(getClass()); - validateNumerics(); if (poolName == null) { @@ -758,22 +756,22 @@ public class HikariConfig implements HikariConfigMXBean } if (driverClassName != null && jdbcUrl == null) { - logger.error("jdbcUrl is required with driverClassName"); + LOGGER.error("jdbcUrl is required with driverClassName"); throw new IllegalArgumentException("jdbcUrl is required with driverClassName"); } else if (driverClassName != null && dataSourceClassName != null) { - logger.error("cannot use driverClassName and dataSourceClassName together"); + LOGGER.error("cannot use driverClassName and dataSourceClassName together"); throw new IllegalArgumentException("cannot use driverClassName and dataSourceClassName together"); } else if (jdbcUrl != null) { // OK } else if (dataSource == null && dataSourceClassName == null) { - logger.error("either dataSource or dataSourceClassName is required"); + LOGGER.error("either dataSource or dataSourceClassName is required"); throw new IllegalArgumentException("either dataSource or dataSourceClassName is required"); } else if (dataSource != null && dataSourceClassName != null) { - logger.warn("using dataSource and ignoring dataSourceClassName"); + LOGGER.warn("using dataSource and ignoring dataSourceClassName"); } if (transactionIsolationName != null) { @@ -787,8 +785,6 @@ public class HikariConfig implements HikariConfigMXBean private void validateNumerics() { - Logger logger = LoggerFactory.getLogger(getClass()); - if (validationTimeout > connectionTimeout && connectionTimeout != Integer.MAX_VALUE) { validationTimeout = connectionTimeout; } @@ -798,29 +794,29 @@ public class HikariConfig implements HikariConfigMXBean } if (maxLifetime < 0) { - logger.error("maxLifetime cannot be negative."); + LOGGER.error("maxLifetime cannot be negative."); throw new IllegalArgumentException("maxLifetime cannot be negative."); } else if (maxLifetime > 0 && maxLifetime < TimeUnit.SECONDS.toMillis(30)) { - logger.warn("maxLifetime is less than 30000ms, setting to default {}ms.", MAX_LIFETIME); + LOGGER.warn("maxLifetime is less than 30000ms, setting to default {}ms.", MAX_LIFETIME); maxLifetime = MAX_LIFETIME; } if (idleTimeout != 0 && idleTimeout < TimeUnit.SECONDS.toMillis(10)) { - logger.warn("idleTimeout is less than 10000ms, setting to default {}ms.", IDLE_TIMEOUT); + LOGGER.warn("idleTimeout is less than 10000ms, setting to default {}ms.", IDLE_TIMEOUT); idleTimeout = IDLE_TIMEOUT; } if (idleTimeout + TimeUnit.SECONDS.toMillis(1) > maxLifetime && maxLifetime > 0) { - logger.warn("idleTimeout is close to or greater than maxLifetime, disabling it."); + LOGGER.warn("idleTimeout is close to or greater than maxLifetime, disabling it."); idleTimeout = 0; } if (maxLifetime == 0 && idleTimeout == 0) { - logger.warn("setting idleTimeout to {}ms.", IDLE_TIMEOUT); - idleTimeout = IDLE_TIMEOUT; + LOGGER.warn("setting idleTimeout to {}ms.", IDLE_TIMEOUT); + idleTimeout = IDLE_TIMEOUT; } if (leakDetectionThreshold != 0 && leakDetectionThreshold < TimeUnit.SECONDS.toMillis(2) && !unitTest) { - logger.warn("leakDetectionThreshold is less than 2000ms, setting to minimum 2000ms."); + LOGGER.warn("leakDetectionThreshold is less than 2000ms, setting to minimum 2000ms."); leakDetectionThreshold = 2000L; } } diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 924ea215..7320c567 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -68,7 +68,7 @@ import com.zaxxer.hikari.util.PropertyElf; */ public class HikariPool implements HikariPoolMXBean, IBagStateListener { - final Logger LOGGER = LoggerFactory.getLogger(getClass()); + private static final Logger LOGGER = LoggerFactory.getLogger(HikariPool.class); private static final ClockSource clockSource = ClockSource.INSTANCE; diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolElf.java b/src/main/java/com/zaxxer/hikari/pool/PoolElf.java index 528cd427..8a9cda84 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolElf.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolElf.java @@ -207,7 +207,7 @@ public final class PoolElf * @param lastConnectionFailure last connection failure * @return true if the connection is alive, false if it is not alive or we timed out */ - boolean isConnectionAlive(final Connection connection, AtomicReference lastConnectionFailure) + boolean isConnectionAlive(final Connection connection, final AtomicReference lastConnectionFailure) { try { int timeoutSec = (int) TimeUnit.MILLISECONDS.toSeconds(validationTimeout);