From b2cfd099988a9491b1fa0052ab32a0c05596698e Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Tue, 29 Sep 2015 13:31:24 +0900 Subject: [PATCH 01/33] Fix timing sensitive test that often fails on travis-ci --- .../pool/TestConnectionTimeoutRetry.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java index 0d5f0099..c8c9d277 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java @@ -31,7 +31,7 @@ public class TestConnectionTimeoutRetry try (HikariDataSource ds = new HikariDataSource(config)) { StubDataSource stubDataSource = ds.unwrap(StubDataSource.class); stubDataSource.setThrowException(new SQLException("Connection refused")); - + long start = ClockSource.INSTANCE.currentTime(); try (Connection connection = ds.getConnection()) { connection.close(); @@ -60,7 +60,7 @@ public class TestConnectionTimeoutRetry try (HikariDataSource ds = new HikariDataSource(config)) { final StubDataSource stubDataSource = ds.unwrap(StubDataSource.class); stubDataSource.setThrowException(new SQLException("Connection refused")); - + ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); scheduler.schedule(new Runnable() { public void run() @@ -68,12 +68,12 @@ public class TestConnectionTimeoutRetry stubDataSource.setThrowException(null); } }, 300, TimeUnit.MILLISECONDS); - + long start = ClockSource.INSTANCE.currentTime(); try { Connection connection = ds.getConnection(); connection.close(); - + long elapsed = ClockSource.INSTANCE.elapsedMillis(start); Assert.assertTrue("Connection returned too quickly, something is wrong.", elapsed > 250); Assert.assertTrue("Waited too long to get a connection.", elapsed < config.getConnectionTimeout()); @@ -103,7 +103,7 @@ public class TestConnectionTimeoutRetry final Connection connection2 = ds.getConnection(); Assert.assertNotNull(connection1); Assert.assertNotNull(connection2); - + ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2); scheduler.schedule(new Runnable() { public void run() @@ -116,12 +116,12 @@ public class TestConnectionTimeoutRetry } } }, 800, TimeUnit.MILLISECONDS); - + long start = ClockSource.INSTANCE.currentTime(); try { Connection connection3 = ds.getConnection(); connection3.close(); - + long elapsed = ClockSource.INSTANCE.elapsedMillis(start); Assert.assertTrue("Waited too long to get a connection.", (elapsed >= 700) && (elapsed < 950)); } @@ -148,7 +148,7 @@ public class TestConnectionTimeoutRetry try (HikariDataSource ds = new HikariDataSource(config)) { StubDataSource stubDataSource = ds.unwrap(StubDataSource.class); stubDataSource.setThrowException(new SQLException("Connection refused")); - + long start = ClockSource.INSTANCE.currentTime(); try { Connection connection = ds.getConnection(); @@ -175,9 +175,9 @@ public class TestConnectionTimeoutRetry try (HikariDataSource ds = new HikariDataSource(config)) { final Connection connection1 = ds.getConnection(); - + long start = ClockSource.INSTANCE.currentTime(); - + ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2); scheduler.schedule(new Runnable() { public void run() @@ -190,14 +190,14 @@ public class TestConnectionTimeoutRetry } } }, 250, TimeUnit.MILLISECONDS); - + StubDataSource stubDataSource = ds.unwrap(StubDataSource.class); stubDataSource.setThrowException(new SQLException("Connection refused")); - + try { Connection connection2 = ds.getConnection(); connection2.close(); - + long elapsed = ClockSource.INSTANCE.elapsedMillis(start); Assert.assertTrue("Waited too long to get a connection.", (elapsed >= 250) && (elapsed < config.getConnectionTimeout())); } @@ -234,7 +234,7 @@ public class TestConnectionTimeoutRetry Connection connection6 = ds.getConnection(); Connection connection7 = ds.getConnection(); - Thread.sleep(1200); + Thread.sleep(2000); Assert.assertSame("Totals connections not as expected", 10, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 3, TestElf.getPool(ds).getIdleConnections()); From 79386c6487a53eba8b6690e3d03b90eda0e2d842 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Tue, 29 Sep 2015 23:08:02 +0900 Subject: [PATCH 02/33] Fixes #432 improve accuracy of elapsed time calculation to account for time spent in isConnectionAlive() call. --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 4 ++-- 1 file 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 19b49e7c..3a87b7fe 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -167,7 +167,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL final long now = clockSource.currentTime(); if (poolEntry.evict || (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > ALIVE_BYPASS_WINDOW_MS && !isConnectionAlive(poolEntry.connection))) { closeConnection(poolEntry, "(connection evicted or dead)"); // Throw away the dead connection and try again - timeout = hardTimeout - clockSource.elapsedMillis(startTime, now); + timeout = hardTimeout - clockSource.elapsedMillis(startTime); } else { metricsTracker.recordBorrowStats(poolEntry, startTime); @@ -626,7 +626,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL } } } - + logPoolState("After cleanup\t"); fillPool(); // Try to maintain minimum connections From 2362af9e14dcc3c0852bf973eceefa9de37d1ebd Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Tue, 29 Sep 2015 23:24:35 +0900 Subject: [PATCH 03/33] Update CHANGES --- CHANGES | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGES b/CHANGES index e77f64b2..02e8347e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,25 @@ HikariCP Changes +Changes in 2.4.2 + + * Improve accuracy of timeouts for getConnection() calls by accounting for possibly + long delay aliveness tests. + + * Improve adherence to minimumIdle goal by closing idle connections starting from + longest idle time to shortest. Additionally, stop when minimumIdle is reached even + if connections exceeding idleTimeout remain (but are still within maxLifetime). + + * Ongoing com.zaxxer.hikari.metrics refactors. This is not considered public API until + such time as we announce it. Caveat lector. + + * Performance improvements in the getConnection()/close() hot path. + + * issue 415: remove use of java.beans classes to allow use of HikariCP with the + Zulu JRE compact3 profile. + + * issue 406: execute validation query during connection setup to make sure it is + valid SQL. + Changes in 2.4.1 * issue 380: housekeeper was not being scheduled in the case of a user specified From 31447dd1e23ba72c89be91f08be258c2cacd6c63 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 30 Sep 2015 00:49:49 +0900 Subject: [PATCH 04/33] Evict connection immediately upon broken state detection. --- .../com/zaxxer/hikari/pool/HikariPool.java | 13 ++++-------- .../com/zaxxer/hikari/pool/PoolEntry.java | 21 ++++++++++++------- .../zaxxer/hikari/pool/ProxyConnection.java | 12 +++++++---- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 3a87b7fe..d5c9e1ea 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -165,7 +165,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL } final long now = clockSource.currentTime(); - if (poolEntry.evict || (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > ALIVE_BYPASS_WINDOW_MS && !isConnectionAlive(poolEntry.connection))) { + if (poolEntry.isMarkedEvicted() || (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > ALIVE_BYPASS_WINDOW_MS && !isConnectionAlive(poolEntry.connection))) { closeConnection(poolEntry, "(connection evicted or dead)"); // Throw away the dead connection and try again timeout = hardTimeout - clockSource.elapsedMillis(startTime); } @@ -206,12 +206,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL { metricsTracker.recordConnectionUsage(poolEntry); - if (poolEntry.evict) { - closeConnection(poolEntry, "(connection broken or evicted)"); - } - else { - connectionBag.requite(poolEntry); - } + connectionBag.requite(poolEntry); } /** @@ -506,7 +501,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL { for (PoolEntry poolEntry : connectionBag.values(STATE_IN_USE)) { try { - poolEntry.evict = true; + poolEntry.markEvicted(); poolEntry.connection.abort(assassinExecutor); } catch (Throwable e) { @@ -557,7 +552,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL private void softEvictConnection(final PoolEntry poolEntry, final String reason, final boolean owner) { - poolEntry.evict(); + poolEntry.markEvicted(); if (connectionBag.reserve(poolEntry) || owner) { closeConnection(poolEntry, reason); } diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java index 320586b1..29e16473 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java @@ -42,10 +42,10 @@ final class PoolEntry implements IConcurrentBagEntry Connection connection; long lastAccessed; long lastBorrowed; - volatile boolean evict; + private volatile boolean evict; private final FastList openStatements; - private final PoolBase poolBase; + private final HikariPool hikariPool; private final AtomicInteger state; private volatile ScheduledFuture endOfLife; @@ -63,7 +63,7 @@ final class PoolEntry implements IConcurrentBagEntry PoolEntry(final Connection connection, final PoolBase pool) { this.connection = connection; - this.poolBase = pool; + this.hikariPool = (HikariPool) pool; this.state = new AtomicInteger(STATE_NOT_IN_USE); this.lastAccessed = ClockSource.INSTANCE.currentTime(); this.openStatements = new FastList<>(Statement.class, 16); @@ -77,7 +77,7 @@ final class PoolEntry implements IConcurrentBagEntry void recycle(final long lastAccessed) { this.lastAccessed = lastAccessed; - poolBase.releaseConnection(this); + hikariPool.releaseConnection(this); } /** @@ -95,12 +95,12 @@ final class PoolEntry implements IConcurrentBagEntry void resetConnectionState(final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException { - poolBase.resetConnectionState(connection, proxyConnection, dirtyBits); + hikariPool.resetConnectionState(connection, proxyConnection, dirtyBits); } String getPoolName() { - return poolBase.toString(); + return hikariPool.toString(); } Connection getConnection() @@ -108,16 +108,21 @@ final class PoolEntry implements IConcurrentBagEntry return connection; } - boolean isEvicted() + boolean isMarkedEvicted() { return evict; } - void evict() + void markEvicted() { this.evict = true; } + void evict(final String closureReason) + { + hikariPool.closeConnection(this, closureReason); + } + FastList getStatementsList() { return openStatements; diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index 3bb6ebfd..4a44e5b6 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -140,10 +140,12 @@ public abstract class ProxyConnection implements Connection String sqlState = sqle.getSQLState(); if (sqlState != null) { boolean isForceClose = sqlState.startsWith("08") || SQL_ERRORS.contains(sqlState); - if (isForceClose) { - poolEntry.evict(); + if (isForceClose && delegate != ClosedConnection.CLOSED_CONNECTION) { LOGGER.warn("{} - Connection {} marked as broken because of SQLSTATE({}), ErrorCode({})", poolEntry.getPoolName(), delegate, sqlState, sqle.getErrorCode(), sqle); + leakTask.cancel(); + delegate = ClosedConnection.CLOSED_CONNECTION; + poolEntry.evict("(connection broken)"); } else { SQLException nse = sqle.getNextException(); @@ -211,11 +213,13 @@ public abstract class ProxyConnection implements Connection @Override public final void close() throws SQLException { + // Closing statements can cause connection eviction, so this must run before the conditional below + closeStatements(); + if (delegate != ClosedConnection.CLOSED_CONNECTION) { leakTask.cancel(); try { - closeStatements(); if (isCommitStateDirty && !isAutoCommit) { delegate.rollback(); lastAccess = clockSource.currentTime(); @@ -231,7 +235,7 @@ public abstract class ProxyConnection implements Connection } catch (SQLException e) { // when connections are aborted, exceptions are often thrown that should not reach the application - if (!poolEntry.isEvicted()) { + if (!poolEntry.isMarkedEvicted()) { throw checkException(e); } } From 4ae5c69eae230ee814a4fd926f65957a4b0d3635 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 30 Sep 2015 08:59:34 +0900 Subject: [PATCH 05/33] Shorten some tests timeouts to speed-up build. --- .../java/com/zaxxer/hikari/pool/TestConnections.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/test/java/com/zaxxer/hikari/pool/TestConnections.java b/src/test/java/com/zaxxer/hikari/pool/TestConnections.java index d1768378..44953de2 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestConnections.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestConnections.java @@ -240,8 +240,6 @@ public class TestConnections try { Connection connection = ds.getConnection(); - quietlySleep(1000); - Assert.assertEquals(1, TestElf.getPool(ds).getTotalConnections()); ds.evictConnection(connection); Assert.assertEquals(0, TestElf.getPool(ds).getTotalConnections()); @@ -264,7 +262,7 @@ public class TestConnections HikariDataSource ds = new HikariDataSource(config); try { - UtilityElf.quietlySleep(1200L); + UtilityElf.quietlySleep(500); Assert.assertSame("Totals connections not as expected", 1, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 1, TestElf.getPool(ds).getIdleConnections()); @@ -273,8 +271,6 @@ public class TestConnections Connection connection = ds.getConnection(); Assert.assertNotNull(connection); - quietlySleep(500); - Assert.assertSame("Totals connections not as expected", 1, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 0, TestElf.getPool(ds).getIdleConnections()); @@ -295,8 +291,6 @@ public class TestConnections // The connection will be ejected from the pool here connection.close(); - quietlySleep(500); - Assert.assertSame("Totals connections not as expected", 0, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 0, TestElf.getPool(ds).getIdleConnections()); @@ -372,12 +366,12 @@ public class TestConnections StubStatement.oldDriver = true; HikariDataSource ds = new HikariDataSource(config); try { - quietlySleep(1001); + quietlySleep(500); Connection connection = ds.getConnection(); connection.close(); - quietlySleep(1001); + quietlySleep(500); connection = ds.getConnection(); } finally { From 6137805ab3d00f7de9028d94dd29843d2e818b33 Mon Sep 17 00:00:00 2001 From: Nitin Date: Wed, 30 Sep 2015 18:21:54 +0530 Subject: [PATCH 06/33] minor cleanup --- .../com/zaxxer/hikari/pool/HikariPool.java | 59 ++++++++++--------- .../com/zaxxer/hikari/pool/PoolEntry.java | 10 ---- 2 files changed, 30 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index d5c9e1ea..4362df91 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -197,18 +197,6 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL throw connectionException; } - /** - * Release a connection back to the pool, or permanently close it if it is broken. - * - * @param poolEntry the PoolBagEntry to release back to the pool - */ - public final void releaseConnection(final PoolEntry poolEntry) - { - metricsTracker.recordConnectionUsage(poolEntry); - - connectionBag.requite(poolEntry); - } - /** * Shutdown the pool, closing all idle connections and aborting or closing * active connections. @@ -297,20 +285,6 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL } } - /** - * Log the current pool state at debug level. - * - * @param prefix an optional prefix to prepend the log message - */ - public final void logPoolState(String... prefix) - { - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("{}pool {} stats (total={}, active={}, idle={}, waiting={})", - (prefix.length > 0 ? prefix[0] : ""), poolName, - getTotalConnections(), getActiveConnections(), getIdleConnections(), getThreadsAwaitingConnection()); - } - } - // *********************************************************************** // IBagStateListener callback // *********************************************************************** @@ -407,12 +381,38 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL // Package methods // *********************************************************************** + /** + * Log the current pool state at debug level. + * + * @param prefix an optional prefix to prepend the log message + */ + final void logPoolState(String... prefix) + { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("{}pool {} stats (total={}, active={}, idle={}, waiting={})", + (prefix.length > 0 ? prefix[0] : ""), poolName, + getTotalConnections(), getActiveConnections(), getIdleConnections(), getThreadsAwaitingConnection()); + } + } + + /** + * Release a connection back to the pool, or permanently close it if it is broken. + * + * @param poolEntry the PoolBagEntry to release back to the pool + */ + final void releaseConnection(final PoolEntry poolEntry) + { + metricsTracker.recordConnectionUsage(poolEntry); + + connectionBag.requite(poolEntry); + } + /** * Permanently close the real (underlying) connection (eat any exception). * * @param poolEntry the connection to actually close */ - void closeConnection(final PoolEntry poolEntry, final String closureReason) + final void closeConnection(final PoolEntry poolEntry, final String closureReason) { final Connection connection = poolEntry.connection; poolEntry.close(); @@ -612,13 +612,14 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL // Sort pool entries on lastAccessed Collections.sort(notInUseList, PoolEntry.LASTACCESS_COMPARABLE); // Iterate the first N removable elements - for (final Iterator iter = notInUseList.iterator(); removable > 0 && iter.hasNext(); ) { + final Iterator iter = notInUseList.iterator(); + do { final PoolEntry poolEntry = iter.next(); if (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > idleTimeout && connectionBag.reserve(poolEntry)) { closeConnection(poolEntry, "(connection passed idleTimeout)"); removable--; } - } + } while (removable > 0 && iter.hasNext()); } } diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java index 29e16473..cd3a905e 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java @@ -103,11 +103,6 @@ final class PoolEntry implements IConcurrentBagEntry return hikariPool.toString(); } - Connection getConnection() - { - return connection; - } - boolean isMarkedEvicted() { return evict; @@ -123,11 +118,6 @@ final class PoolEntry implements IConcurrentBagEntry hikariPool.closeConnection(this, closureReason); } - FastList getStatementsList() - { - return openStatements; - } - /** Returns millis since lastBorrowed */ long getMillisSinceBorrowed() { From 03aad4ea35027e705e9f9afbc172fd3967f5c99d Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 30 Sep 2015 23:56:37 +0900 Subject: [PATCH 07/33] minor cleanup --- .../java/com/zaxxer/hikari/pool/PoolBase.java | 25 +++++++++++-------- .../zaxxer/hikari/pool/ProxyConnection.java | 16 ++++++++---- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 8ddf569b..44bc02ec 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -1,6 +1,11 @@ package com.zaxxer.hikari.pool; import static com.zaxxer.hikari.util.UtilityElf.createInstance; +import static com.zaxxer.hikari.pool.ProxyConnection.DIRTY_BIT_CATALOG; +import static com.zaxxer.hikari.pool.ProxyConnection.DIRTY_BIT_READONLY; +import static com.zaxxer.hikari.pool.ProxyConnection.DIRTY_BIT_ISOLATION; +import static com.zaxxer.hikari.pool.ProxyConnection.DIRTY_BIT_AUTOCOMMIT; +import static com.zaxxer.hikari.pool.ProxyConnection.DIRTY_BIT_NETTIMEOUT; import java.lang.management.ManagementFactory; import java.sql.Connection; @@ -170,32 +175,32 @@ abstract class PoolBase { int resetBits = 0; - if ((dirtyBits & 0b00001) != 0 && proxyConnection.getReadOnlyState() != isReadOnly) { + if ((dirtyBits & DIRTY_BIT_READONLY) != 0 && proxyConnection.getReadOnlyState() != isReadOnly) { connection.setReadOnly(isReadOnly); - resetBits |= 0b00001; + resetBits |= DIRTY_BIT_READONLY; } - if ((dirtyBits & 0b00010) != 0 && proxyConnection.getAutoCommitState() != isAutoCommit) { + if ((dirtyBits & DIRTY_BIT_AUTOCOMMIT) != 0 && proxyConnection.getAutoCommitState() != isAutoCommit) { connection.setAutoCommit(isAutoCommit); - resetBits |= 0b00010; + resetBits |= DIRTY_BIT_AUTOCOMMIT; } - if ((dirtyBits & 0b00100) != 0 && proxyConnection.getTransactionIsolationState() != transactionIsolation) { + if ((dirtyBits & DIRTY_BIT_ISOLATION) != 0 && proxyConnection.getTransactionIsolationState() != transactionIsolation) { connection.setTransactionIsolation(transactionIsolation); - resetBits |= 0b00100; + resetBits |= DIRTY_BIT_ISOLATION; } - if ((dirtyBits & 0b01000) != 0) { + if ((dirtyBits & DIRTY_BIT_CATALOG) != 0) { final String currentCatalog = proxyConnection.getCatalogState(); if ((currentCatalog != null && !currentCatalog.equals(catalog)) || (currentCatalog == null && catalog != null)) { connection.setCatalog(catalog); - resetBits |= 0b01000; + resetBits |= DIRTY_BIT_CATALOG; } } - if ((dirtyBits & 0b10000) != 0 && proxyConnection.getNetworkTimeoutState() != networkTimeout) { + if ((dirtyBits & DIRTY_BIT_NETTIMEOUT) != 0 && proxyConnection.getNetworkTimeoutState() != networkTimeout) { setNetworkTimeout(connection, networkTimeout); - resetBits |= 0b10000; + resetBits |= DIRTY_BIT_NETTIMEOUT; } if (LOGGER.isDebugEnabled()) { diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index ef24a5a5..6d6961cc 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -43,6 +43,12 @@ import com.zaxxer.hikari.util.FastList; */ public abstract class ProxyConnection implements Connection { + static final int DIRTY_BIT_READONLY = 0b00001; + static final int DIRTY_BIT_AUTOCOMMIT = 0b00010; + static final int DIRTY_BIT_ISOLATION = 0b00100; + static final int DIRTY_BIT_CATALOG = 0b01000; + static final int DIRTY_BIT_NETTIMEOUT = 0b10000; + private static final Logger LOGGER; private static final Set SQL_ERRORS; private static final ClockSource clockSource; @@ -371,7 +377,7 @@ public abstract class ProxyConnection implements Connection { delegate.setAutoCommit(autoCommit); isAutoCommit = autoCommit; - dirtyBits |= 0b00010; + dirtyBits |= DIRTY_BIT_AUTOCOMMIT; } /** {@inheritDoc} */ @@ -380,7 +386,7 @@ public abstract class ProxyConnection implements Connection { delegate.setReadOnly(readOnly); isReadOnly = readOnly; - dirtyBits |= 0b00001; + dirtyBits |= DIRTY_BIT_READONLY; } /** {@inheritDoc} */ @@ -389,7 +395,7 @@ public abstract class ProxyConnection implements Connection { delegate.setTransactionIsolation(level); transactionIsolation = level; - dirtyBits |= 0b00100; + dirtyBits |= DIRTY_BIT_ISOLATION; } /** {@inheritDoc} */ @@ -398,7 +404,7 @@ public abstract class ProxyConnection implements Connection { delegate.setCatalog(catalog); dbcatalog = catalog; - dirtyBits |= 0b01000; + dirtyBits |= DIRTY_BIT_CATALOG; } /** {@inheritDoc} */ @@ -407,7 +413,7 @@ public abstract class ProxyConnection implements Connection { delegate.setNetworkTimeout(executor, milliseconds); networkTimeout = milliseconds; - dirtyBits |= 0b10000; + dirtyBits |= DIRTY_BIT_NETTIMEOUT; } /** {@inheritDoc} */ From c37a6cb20acaa2b2b3c7afa064fc2102417b9b7e Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 1 Oct 2015 13:19:36 +0530 Subject: [PATCH 08/33] removed isClosed(), why trip to check when close() is no-op if already closed, moreover isClosed() was not guarded by network timeout. mostly call to close would be once anyway. --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 6 ++---- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 7 ++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 4362df91..f5327475 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -173,8 +173,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL metricsTracker.recordBorrowStats(poolEntry, startTime); return poolEntry.createProxyConnection(leakTask.start(poolEntry), now); } - } - while (timeout > 0L); + } while (timeout > 0L); } catch (InterruptedException e) { throw new SQLException(poolName + " - Interrupted during connection acquisition", e); @@ -227,8 +226,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL do { softEvictConnections(); abortActiveConnections(assassinExecutor); - } - while (getTotalConnections() > 0 && clockSource.elapsedMillis(start) < TimeUnit.SECONDS.toMillis(5)); + } while (getTotalConnections() > 0 && clockSource.elapsedMillis(start) < TimeUnit.SECONDS.toMillis(5)); } finally { assassinExecutor.shutdown(); assassinExecutor.awaitTermination(5L, TimeUnit.SECONDS); diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 8ddf569b..cb058365 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -95,7 +95,7 @@ abstract class PoolBase void quietlyCloseConnection(final Connection connection, final String closureReason) { try { - if (connection == null || connection.isClosed()) { + if (connection == null) { return; } @@ -128,10 +128,10 @@ abstract class PoolBase if (isNetworkTimeoutSupported != TRUE) { setQueryTimeout(statement, (int) TimeUnit.MILLISECONDS.toSeconds(validationTimeout)); } - + statement.execute(config.getConnectionTestQuery()); } - + if (isIsolateInternalQueries && !isReadOnly && !isAutoCommit) { connection.rollback(); } @@ -461,6 +461,7 @@ abstract class PoolBase if (sql != null) { try (Statement statement = connection.createStatement()) { + //con created few ms before, set query timeout is omitted statement.execute(sql); if (!isReadOnly) { From 285157c33538f6b15ab327e8e03493e0a3e19e20 Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 1 Oct 2015 16:59:22 +0530 Subject: [PATCH 09/33] removed old call --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 3 ++- src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java | 4 +++- 2 files changed, 5 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 f5327475..af4a7d8d 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -227,7 +227,8 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL softEvictConnections(); abortActiveConnections(assassinExecutor); } while (getTotalConnections() > 0 && clockSource.elapsedMillis(start) < TimeUnit.SECONDS.toMillis(5)); - } finally { + } + finally { assassinExecutor.shutdown(); assassinExecutor.awaitTermination(5L, TimeUnit.SECONDS); } diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index ef24a5a5..c6971c35 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -198,6 +198,9 @@ public abstract class ProxyConnection implements Connection } catch (SQLException e) { checkException(e); + if (delegate == ClosedConnection.CLOSED_CONNECTION) { + break; //connection closed in checkException + } } } @@ -220,7 +223,6 @@ public abstract class ProxyConnection implements Connection leakTask.cancel(); try { - closeStatements(); if (isCommitStateDirty && !isAutoCommit) { delegate.rollback(); lastAccess = clockSource.currentTime(); From f9d493a7d8e8868f87f59bd990ff634a84341bb9 Mon Sep 17 00:00:00 2001 From: Nitin Date: Mon, 5 Oct 2015 13:22:14 +0530 Subject: [PATCH 10/33] checking evict only when required --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 7 ++++--- src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java | 5 +---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index af4a7d8d..558d3a06 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -500,7 +500,6 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL { for (PoolEntry poolEntry : connectionBag.values(STATE_IN_USE)) { try { - poolEntry.markEvicted(); poolEntry.connection.abort(assassinExecutor); } catch (Throwable e) { @@ -551,10 +550,12 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL private void softEvictConnection(final PoolEntry poolEntry, final String reason, final boolean owner) { - poolEntry.markEvicted(); - if (connectionBag.reserve(poolEntry) || owner) { + if (owner || connectionBag.reserve(poolEntry)) { closeConnection(poolEntry, reason); } + else { + poolEntry.markEvicted(); + } } private PoolStats getPoolStats() diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index c6971c35..da243ae7 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -189,7 +189,7 @@ public abstract class ProxyConnection implements Connection { final int size = openStatements.size(); if (size > 0) { - for (int i = 0; i < size; i++) { + for (int i = 0; i < size && delegate != ClosedConnection.CLOSED_CONNECTION; i++) { try { final Statement statement = openStatements.get(i); if (statement != null) { @@ -198,9 +198,6 @@ public abstract class ProxyConnection implements Connection } catch (SQLException e) { checkException(e); - if (delegate == ClosedConnection.CLOSED_CONNECTION) { - break; //connection closed in checkException - } } } From c3482cde10ec0fe4c8c911e6ee4fc368bc8613ce Mon Sep 17 00:00:00 2001 From: Nitin Date: Mon, 5 Oct 2015 18:30:23 +0530 Subject: [PATCH 11/33] removed redundant loop, addItem() is actually adding itemS --- .../com/zaxxer/hikari/pool/HikariPool.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 558d3a06..64474ece 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -95,7 +95,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL * @param config a HikariConfig instance */ public HikariPool(final HikariConfig config) - { + { super(config); this.connectionBag = new ConcurrentBag<>(this); @@ -479,17 +479,18 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL private void fillPool() { final int connectionsToAdd = Math.min(config.getMaximumPoolSize() - totalConnections.get(), config.getMinimumIdle() - getIdleConnections()); - for (int i = 0; i < connectionsToAdd; i++) { + + if (connectionsToAdd > 0) { addBagItem(); - } - if (connectionsToAdd > 0 && LOGGER.isDebugEnabled()) { - addConnectionExecutor.execute(new Runnable() { - @Override - public void run() { - logPoolState("After fill\t"); - } - }); + if (LOGGER.isDebugEnabled()) { + addConnectionExecutor.execute(new Runnable() { + @Override + public void run() { + logPoolState("After fill\t"); + } + }); + } } } From 1c2f1c13a5d827e7ab0439e243a0255af6263941 Mon Sep 17 00:00:00 2001 From: Nitin Date: Mon, 5 Oct 2015 19:48:45 +0530 Subject: [PATCH 12/33] loop is required --- .../com/zaxxer/hikari/pool/HikariPool.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 64474ece..857c4865 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -479,18 +479,17 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL private void fillPool() { final int connectionsToAdd = Math.min(config.getMaximumPoolSize() - totalConnections.get(), config.getMinimumIdle() - getIdleConnections()); - - if (connectionsToAdd > 0) { + for (int i = 0; i < connectionsToAdd; i++) { addBagItem(); + } - if (LOGGER.isDebugEnabled()) { - addConnectionExecutor.execute(new Runnable() { - @Override - public void run() { - logPoolState("After fill\t"); - } - }); - } + if (connectionsToAdd > 0 && LOGGER.isDebugEnabled()) { + addConnectionExecutor.execute(new Runnable() { + @Override + public void run() { + logPoolState("After fill\t"); + } + }); } } From 0da14c8ce95aa1f8ac4a6055741a6bf27b7b7ac7 Mon Sep 17 00:00:00 2001 From: Nitin Date: Tue, 6 Oct 2015 19:52:15 +0530 Subject: [PATCH 13/33] align log with 'Before cleanup' --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 3 ++- 1 file changed, 2 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 857c4865..1940025c 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -442,6 +442,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL // Speculative increment of totalConnections with expectation of success if (totalConnections.incrementAndGet() > config.getMaximumPoolSize()) { totalConnections.decrementAndGet(); // Pool is maxed out, so undo speculative increment of totalConnections + LOGGER.debug("{} - Cannot exceed maximum connections capacity: {}", poolName, config.getMaximumPoolSize()); return true; } @@ -487,7 +488,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL addConnectionExecutor.execute(new Runnable() { @Override public void run() { - logPoolState("After fill\t"); + logPoolState("After adding\t"); } }); } From 716dd345cd5f87ac7cfb0f0b77863227a2b7ca9f Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 7 Oct 2015 01:02:47 +0900 Subject: [PATCH 14/33] Fixed issue with proxy generation --- CHANGES | 6 ++++++ .../java/com/zaxxer/hikari/util/JavassistProxyFactory.java | 2 ++ 2 files changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index e77f64b2..9a29e3e9 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,11 @@ HikariCP Changes +Changes in 2.4.2 + + * Fixed issue with proxy generation whereby the generated classes contain the + major version number for Java 8, which makes them incompatible with the Java 7 + runtime. + Changes in 2.4.1 * issue 380: housekeeper was not being scheduled in the case of a user specified diff --git a/src/main/java/com/zaxxer/hikari/util/JavassistProxyFactory.java b/src/main/java/com/zaxxer/hikari/util/JavassistProxyFactory.java index ba192e7f..bb4329b2 100644 --- a/src/main/java/com/zaxxer/hikari/util/JavassistProxyFactory.java +++ b/src/main/java/com/zaxxer/hikari/util/JavassistProxyFactory.java @@ -42,6 +42,7 @@ import javassist.CtNewMethod; import javassist.LoaderClassPath; import javassist.Modifier; import javassist.NotFoundException; +import javassist.bytecode.ClassFile; /** * This class generates the proxy objects for {@link Connection}, {@link Statement}, @@ -190,6 +191,7 @@ public final class JavassistProxyFactory } } + targetCt.getClassFile().setMajorVersion(ClassFile.JAVA_7); targetCt.writeFile("target/classes"); } From 49be257130ba7bc3080f5bc8a0f191db86f23394 Mon Sep 17 00:00:00 2001 From: Nitin Date: Tue, 6 Oct 2015 22:15:07 +0530 Subject: [PATCH 15/33] commented log & added changes removed from old commit --- CHANGES | 24 +++++++++++++++++++ .../com/zaxxer/hikari/pool/HikariPool.java | 2 +- .../pool/TestConnectionTimeoutRetry.java | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index e77f64b2..9e3c3693 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,29 @@ HikariCP Changes +Changes in 2.4.2 + + * Fixed issue with proxy generation whereby the generated classes contain the + major version number for Java 8, which makes them incompatible with the Java 7 + runtime. + + * Improve accuracy of timeouts for getConnection() calls by accounting for possibly + long delay aliveness tests. + + * Improve adherence to minimumIdle goal by closing idle connections starting from + longest idle time to shortest. Additionally, stop when minimumIdle is reached even + if connections exceeding idleTimeout remain (but are still within maxLifetime). + + * Ongoing com.zaxxer.hikari.metrics refactors. This is not considered public API until + such time as we announce it. Caveat lector. + + * Performance improvements in the getConnection()/close() hot path. + + * issue 415: remove use of java.beans classes to allow use of HikariCP with the + Zulu JRE compact3 profile. + + * issue 406: execute validation query during connection setup to make sure it is + valid SQL. + Changes in 2.4.1 * issue 380: housekeeper was not being scheduled in the case of a user specified diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 1940025c..a40717e9 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -442,7 +442,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL // Speculative increment of totalConnections with expectation of success if (totalConnections.incrementAndGet() > config.getMaximumPoolSize()) { totalConnections.decrementAndGet(); // Pool is maxed out, so undo speculative increment of totalConnections - LOGGER.debug("{} - Cannot exceed maximum connections capacity: {}", poolName, config.getMaximumPoolSize()); + //LOGGER.debug("{} - Cannot exceed maximum connections capacity: {}", poolName, config.getMaximumPoolSize()); return true; } diff --git a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java index c8c9d277..176788d4 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java @@ -236,7 +236,7 @@ public class TestConnectionTimeoutRetry Thread.sleep(2000); - Assert.assertSame("Totals connections not as expected", 10, TestElf.getPool(ds).getTotalConnections()); + Assert.assertSame("Total connections not as expected", 10, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 3, TestElf.getPool(ds).getIdleConnections()); connection1.close(); From efa972683524566b9bfcc4cf87c64c8c7b73d16f Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 7 Oct 2015 12:30:07 +0900 Subject: [PATCH 16/33] Adjust test timeout to try to address travis-ci transient failures. --- .../com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java index 176788d4..6ebe251c 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java @@ -223,7 +223,7 @@ public class TestConnectionTimeoutRetry config.setConnectionTestQuery("VALUES 2"); config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource"); - System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "500"); + System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "400"); try (HikariDataSource ds = new HikariDataSource(config)) { Connection connection1 = ds.getConnection(); @@ -234,7 +234,7 @@ public class TestConnectionTimeoutRetry Connection connection6 = ds.getConnection(); Connection connection7 = ds.getConnection(); - Thread.sleep(2000); + Thread.sleep(2500); Assert.assertSame("Total connections not as expected", 10, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 3, TestElf.getPool(ds).getIdleConnections()); From 23c1b80bfd7ffa4df21365b6f5d6a80bcdc8f32d Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 7 Oct 2015 12:38:02 +0900 Subject: [PATCH 17/33] Tighten bypass window. May tighten further to 250ms later. --- 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 a40717e9..3f42fc33 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -67,7 +67,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL private static final ClockSource clockSource = ClockSource.INSTANCE; - private static final long ALIVE_BYPASS_WINDOW_MS = Long.getLong("com.zaxxer.hikari.aliveBypassWindow", TimeUnit.SECONDS.toMillis(1)); + private static final long ALIVE_BYPASS_WINDOW_MS = Long.getLong("com.zaxxer.hikari.aliveBypassWindowMs", TimeUnit.MILLISECONDS.toMillis(500)); private static final long HOUSEKEEPING_PERIOD_MS = Long.getLong("com.zaxxer.hikari.housekeeping.periodMs", TimeUnit.SECONDS.toMillis(30)); private static final int POOL_NORMAL = 0; From c0c6daf93de068f53cafed4ed0161efdd0e77172 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 7 Oct 2015 16:33:10 +0900 Subject: [PATCH 18/33] Fixed #401 log warning when both dataSourceClassName and jdbcUrl are specified. --- src/main/java/com/zaxxer/hikari/HikariConfig.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/zaxxer/hikari/HikariConfig.java b/src/main/java/com/zaxxer/hikari/HikariConfig.java index b88454f6..823cf81f 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfig.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfig.java @@ -763,6 +763,9 @@ public class HikariConfig implements HikariConfigMXBean LOGGER.error("cannot use driverClassName and dataSourceClassName together"); throw new IllegalArgumentException("cannot use driverClassName and dataSourceClassName together"); } + else if (jdbcUrl != null && dataSourceClassName != null) { + LOGGER.warn("using dataSourceClassName and ignoring jdbcUrl"); + } else if (jdbcUrl != null) { // OK } From 8a7901f292a278249bc0e7596f2e9d506f95588a Mon Sep 17 00:00:00 2001 From: Florian Rampp Date: Wed, 7 Oct 2015 15:18:47 +0200 Subject: [PATCH 19/33] do not fail if setLoginTimeout on delegate data source is not supported Some `DataSource` implementations (like [`BasicDataSource`](https://commons.apache.org/proper/commons-dbcp/api-1.4/org/apache/commons/dbcp/BasicDataSource.html) or [`DriverManagerDataSource`](https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/jdbc/datasource/DriverManagerDataSource.html) do not support setting a login timeout. `UnsupportedOperationException` is thrown in these cases. When a delegate `dataSource` is set for the HikariCP, it tries to invoke this method on the delegate. Only log a warning in this case instead of failing to initialize the HikariCP in this case. This is amongst others relevant for using HikariCP in Grails, which creates instances of `org.springframework.jdbc.datasource.DriverManagerDataSource` (at least in version 2.2.4). --- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index d00f747c..5b6d6175 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -528,6 +528,9 @@ abstract class PoolBase catch (SQLException e) { LOGGER.warn("{} - Unable to set DataSource login timeout", poolName, e); } + catch (UnsupportedOperationException e) { + LOGGER.warn("{} - Unable to set DataSource login timeout", poolName, e); + } } } From 4e1aaa1802dfa158b94474abe87861f345f7f388 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 7 Oct 2015 23:05:56 +0900 Subject: [PATCH 20/33] Prepare for 2.4.2 release candidate. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7bd01ff0..b0c614fd 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ com.zaxxer HikariCP - 2.4.2-SNAPSHOT + 2.4.2-RC1 bundle HikariCP From 678b2ce139305b3b55960ebf733e0eaa75803978 Mon Sep 17 00:00:00 2001 From: Florian Rampp Date: Wed, 7 Oct 2015 16:09:34 +0200 Subject: [PATCH 21/33] use multi-catch statement I was not aware that this feature is available in JDK7 already and the README specifies that JDK7 is required only. --- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 5b6d6175..09ce2dbd 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -525,10 +525,7 @@ abstract class PoolBase try { dataSource.setLoginTimeout((int) TimeUnit.MILLISECONDS.toSeconds(Math.max(1000L, connectionTimeout))); } - catch (SQLException e) { - LOGGER.warn("{} - Unable to set DataSource login timeout", poolName, e); - } - catch (UnsupportedOperationException e) { + catch (SQLException | UnsupportedOperationException e) { LOGGER.warn("{} - Unable to set DataSource login timeout", poolName, e); } } From 1daf44b75480abb98522bf657eacdca92f0fd8e2 Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 11:20:46 +0530 Subject: [PATCH 22/33] using Throwable, better for backport too --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 2 +- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 9 ++++----- 2 files 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 3f42fc33..e4592c00 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -528,7 +528,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL final PoolEntry poolEntry = connectionBag.borrow(connectionTimeout, TimeUnit.MILLISECONDS); if (config.getMinimumIdle() == 0) { - closeConnection(poolEntry, "Initialization validation complete, closing test connection."); + closeConnection(poolEntry, "Closing connection borrowed for validation."); } else { connectionBag.requite(poolEntry); diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 09ce2dbd..1a57f56f 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -99,11 +99,10 @@ abstract class PoolBase void quietlyCloseConnection(final Connection connection, final String closureReason) { + if (connection == null) { + return; + } try { - if (connection == null) { - return; - } - LOGGER.debug("{} - Closing connection {}: {}", poolName, connection, closureReason); try { setNetworkTimeout(connection, TimeUnit.SECONDS.toMillis(15)); @@ -525,7 +524,7 @@ abstract class PoolBase try { dataSource.setLoginTimeout((int) TimeUnit.MILLISECONDS.toSeconds(Math.max(1000L, connectionTimeout))); } - catch (SQLException | UnsupportedOperationException e) { + catch (Throwable e) { LOGGER.warn("{} - Unable to set DataSource login timeout", poolName, e); } } From 0218ed14b5d9a5d717ea77c8d39b1b7c524104ae Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 14:31:08 +0530 Subject: [PATCH 23/33] format --- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 1a57f56f..291da338 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -102,6 +102,7 @@ abstract class PoolBase if (connection == null) { return; } + try { LOGGER.debug("{} - Closing connection {}: {}", poolName, connection, closureReason); try { From dcd8fdb3a9ba7438de56f900d90994a1b1819b5b Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 15:14:19 +0530 Subject: [PATCH 24/33] final --- src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index 74de3e24..c7460a13 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -143,9 +143,9 @@ public abstract class ProxyConnection implements Connection /** {@inheritDoc} */ final SQLException checkException(final SQLException sqle) { - String sqlState = sqle.getSQLState(); + final String sqlState = sqle.getSQLState(); if (sqlState != null) { - boolean isForceClose = sqlState.startsWith("08") || SQL_ERRORS.contains(sqlState); + final boolean isForceClose = sqlState.startsWith("08") || SQL_ERRORS.contains(sqlState); if (isForceClose && delegate != ClosedConnection.CLOSED_CONNECTION) { LOGGER.warn("{} - Connection {} marked as broken because of SQLSTATE({}), ErrorCode({})", poolEntry.getPoolName(), delegate, sqlState, sqle.getErrorCode(), sqle); @@ -154,7 +154,7 @@ public abstract class ProxyConnection implements Connection poolEntry.evict("(connection broken)"); } else { - SQLException nse = sqle.getNextException(); + final SQLException nse = sqle.getNextException(); if (nse != null && nse != sqle) { checkException(nse); } From e74639c41847da8124b2fbdabf72162943164f9b Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Thu, 8 Oct 2015 18:45:07 +0900 Subject: [PATCH 25/33] Fix bug uncovered during live DB testing. If the database is autoCommit=true we were trying to rollback because ProxyConnection.isAutoCommit was never initialized to true. --- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 10 ++++++++++ src/main/java/com/zaxxer/hikari/pool/PoolEntry.java | 2 +- .../java/com/zaxxer/hikari/pool/ProxyConnection.java | 5 +++-- src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java | 2 +- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 291da338..9f987c6f 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -157,6 +157,11 @@ abstract class PoolBase return lastConnectionFailure.getAndSet(null); } + boolean isAutoCommit() + { + return isAutoCommit; + } + public DataSource getUnwrappedDataSource() { return dataSource; @@ -171,6 +176,11 @@ abstract class PoolBase return new PoolEntry(newConnection(), this); } + public void initConnectionState(ProxyConnection proxyConnection) + { + proxyConnection.isAutoCommit = isAutoCommit; + } + void resetConnectionState(final Connection connection, final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException { int resetBits = 0; diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java index cd3a905e..3dcaa419 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java @@ -90,7 +90,7 @@ final class PoolEntry implements IConcurrentBagEntry Connection createProxyConnection(final ProxyLeakTask leakTask, final long now) { - return ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now); + return ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now, hikariPool.isAutoCommit()); } void resetConnectionState(final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index 74de3e24..24a435a1 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -63,7 +63,7 @@ public abstract class ProxyConnection implements Connection private long lastAccess; private boolean isCommitStateDirty; - private boolean isAutoCommit; + protected boolean isAutoCommit; private int networkTimeout; private int transactionIsolation; private String dbcatalog; @@ -83,12 +83,13 @@ public abstract class ProxyConnection implements Connection SQL_ERRORS.add("JZ0C1"); // Sybase disconnect error } - protected ProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now) { + protected ProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now, final boolean isAutoCommit) { this.poolEntry = poolEntry; this.delegate = connection; this.openStatements = openStatements; this.leakTask = leakTask; this.lastAccess = now; + this.isAutoCommit = isAutoCommit; } /** {@inheritDoc} */ diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java b/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java index 181e07db..713ba7c1 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java @@ -47,7 +47,7 @@ public final class ProxyFactory * @param now current timestamp in milliseconds * @return a proxy that wraps the specified {@link Connection} */ - static ProxyConnection getProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now) + static ProxyConnection getProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now, final boolean isAutoCommit) { // Body is replaced (injected) by JavassistProxyFactory throw new IllegalStateException("You need to run the CLI build and you need target/classes in your classpath to run."); From 87fa34af15ae9e3ed3542866159c9be02f5e4ad5 Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 17:35:53 +0530 Subject: [PATCH 26/33] init 'other' vars too for proxy connection --- .../java/com/zaxxer/hikari/pool/PoolBase.java | 10 +++++----- .../com/zaxxer/hikari/pool/PoolEntry.java | 4 +++- .../zaxxer/hikari/pool/ProxyConnection.java | 20 +++++++++++++------ .../com/zaxxer/hikari/pool/ProxyFactory.java | 13 ++++++------ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 9f987c6f..3911d7f9 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -167,6 +167,11 @@ abstract class PoolBase return dataSource; } + public void initProxyConnection(ProxyConnection pc) + { + pc.init(isReadOnly, isAutoCommit, networkTimeout, transactionIsolation, catalog); + } + // *********************************************************************** // PoolEntry methods // *********************************************************************** @@ -176,11 +181,6 @@ abstract class PoolBase return new PoolEntry(newConnection(), this); } - public void initConnectionState(ProxyConnection proxyConnection) - { - proxyConnection.isAutoCommit = isAutoCommit; - } - void resetConnectionState(final Connection connection, final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException { int resetBits = 0; diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java index 3dcaa419..16f78ac6 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java @@ -90,7 +90,9 @@ final class PoolEntry implements IConcurrentBagEntry Connection createProxyConnection(final ProxyLeakTask leakTask, final long now) { - return ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now, hikariPool.isAutoCommit()); + ProxyConnection pc = ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now); + hikariPool.initProxyConnection(pc); + return pc; } void resetConnectionState(final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index c66fe50c..d4bb19c7 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -55,19 +55,19 @@ public abstract class ProxyConnection implements Connection protected Connection delegate; - private final ProxyLeakTask leakTask; private final PoolEntry poolEntry; + private final ProxyLeakTask leakTask; private final FastList openStatements; private int dirtyBits; private long lastAccess; private boolean isCommitStateDirty; - protected boolean isAutoCommit; + private boolean isReadOnly; + private boolean isAutoCommit; private int networkTimeout; private int transactionIsolation; private String dbcatalog; - private boolean isReadOnly; // static initializer static { @@ -83,13 +83,12 @@ public abstract class ProxyConnection implements Connection SQL_ERRORS.add("JZ0C1"); // Sybase disconnect error } - protected ProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now, final boolean isAutoCommit) { + protected ProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now) { this.poolEntry = poolEntry; this.delegate = connection; this.openStatements = openStatements; this.leakTask = leakTask; this.lastAccess = now; - this.isAutoCommit = isAutoCommit; } /** {@inheritDoc} */ @@ -103,9 +102,18 @@ public abstract class ProxyConnection implements Connection } // *********************************************************************** - // Connection State Accessors + // Connection State set/getters // *********************************************************************** + final void init(boolean isReadOnly, boolean isAutoCommit, int networkTimeout, int transactionIsolation, String dbcatalog) + { + this.isReadOnly = isReadOnly; + this.isAutoCommit = isAutoCommit; + this.networkTimeout = networkTimeout; + this.transactionIsolation = transactionIsolation; + this.dbcatalog = dbcatalog; + } + final boolean getAutoCommitState() { return isAutoCommit; diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java b/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java index 713ba7c1..b41d78ce 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java @@ -39,15 +39,14 @@ public final class ProxyFactory /** * Create a proxy for the specified {@link Connection} instance. - * @param openStatements - * @param connection - * - * @param connectionState the PoolBagEntry entry for this proxy - * @param openStatements a leak detetection task - * @param now current timestamp in milliseconds + * @param poolEntry + * @param connection + * @param openStatements + * @param leakTask + * @param now * @return a proxy that wraps the specified {@link Connection} */ - static ProxyConnection getProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now, final boolean isAutoCommit) + static ProxyConnection getProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now) { // Body is replaced (injected) by JavassistProxyFactory throw new IllegalStateException("You need to run the CLI build and you need target/classes in your classpath to run."); From 63c0fcb893210f5b05bafac23d0f9ad9dd3d83ad Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 17:46:10 +0530 Subject: [PATCH 27/33] removed unused method --- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 3911d7f9..c4a5c7ce 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -157,11 +157,6 @@ abstract class PoolBase return lastConnectionFailure.getAndSet(null); } - boolean isAutoCommit() - { - return isAutoCommit; - } - public DataSource getUnwrappedDataSource() { return dataSource; From 10b0547ab74babd49d44ddb409d3f54f9b80237d Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 17:49:35 +0530 Subject: [PATCH 28/33] comment corrected --- src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index d4bb19c7..c944a4a8 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -102,7 +102,7 @@ public abstract class ProxyConnection implements Connection } // *********************************************************************** - // Connection State set/getters + // Connection State init & getters // *********************************************************************** final void init(boolean isReadOnly, boolean isAutoCommit, int networkTimeout, int transactionIsolation, String dbcatalog) From bcde774078e553c363d9e31c0022ace87ca62f71 Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 18:40:18 +0530 Subject: [PATCH 29/33] check readonly on rollback --- src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index c944a4a8..57bd4b27 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -235,7 +235,7 @@ public abstract class ProxyConnection implements Connection leakTask.cancel(); try { - if (isCommitStateDirty && !isAutoCommit) { + if (isCommitStateDirty && !isAutoCommit && !isReadOnly) { delegate.rollback(); lastAccess = clockSource.currentTime(); LOGGER.debug("{} - Executed rollback on connection {} due to dirty commit state on close().", poolEntry.getPoolName(), delegate); From 216b3c0ac468a421c8fcff431383ace60f196940 Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 19:16:12 +0530 Subject: [PATCH 30/33] setting only readonly and autocommit --- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 2 +- src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index c4a5c7ce..67482586 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -164,7 +164,7 @@ abstract class PoolBase public void initProxyConnection(ProxyConnection pc) { - pc.init(isReadOnly, isAutoCommit, networkTimeout, transactionIsolation, catalog); + pc.init(isReadOnly, isAutoCommit); } // *********************************************************************** diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index 57bd4b27..6bb26168 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -105,13 +105,11 @@ public abstract class ProxyConnection implements Connection // Connection State init & getters // *********************************************************************** - final void init(boolean isReadOnly, boolean isAutoCommit, int networkTimeout, int transactionIsolation, String dbcatalog) + final void init(boolean isReadOnly, boolean isAutoCommit) { + // only these two are used in close so set them correctly. this.isReadOnly = isReadOnly; this.isAutoCommit = isAutoCommit; - this.networkTimeout = networkTimeout; - this.transactionIsolation = transactionIsolation; - this.dbcatalog = dbcatalog; } final boolean getAutoCommitState() From 8c509f383700eb863bd09880d4f519852463dc66 Mon Sep 17 00:00:00 2001 From: Nitin Date: Thu, 8 Oct 2015 20:02:36 +0530 Subject: [PATCH 31/33] calling methods back and forth is past for good :) --- .../java/com/zaxxer/hikari/pool/PoolBase.java | 15 ++++++++++----- .../java/com/zaxxer/hikari/pool/PoolEntry.java | 4 +--- .../com/zaxxer/hikari/pool/ProxyConnection.java | 13 ++++--------- .../java/com/zaxxer/hikari/pool/ProxyFactory.java | 4 +++- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 67482586..8074c0d6 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -162,11 +162,6 @@ abstract class PoolBase return dataSource; } - public void initProxyConnection(ProxyConnection pc) - { - pc.init(isReadOnly, isAutoCommit); - } - // *********************************************************************** // PoolEntry methods // *********************************************************************** @@ -176,6 +171,16 @@ abstract class PoolBase return new PoolEntry(newConnection(), this); } + boolean getReadOnly() + { + return isReadOnly; + } + + boolean getAutoCommit() + { + return isAutoCommit; + } + void resetConnectionState(final Connection connection, final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException { int resetBits = 0; diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java index 16f78ac6..e02f4396 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java @@ -90,9 +90,7 @@ final class PoolEntry implements IConcurrentBagEntry Connection createProxyConnection(final ProxyLeakTask leakTask, final long now) { - ProxyConnection pc = ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now); - hikariPool.initProxyConnection(pc); - return pc; + return ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now, hikariPool.getReadOnly(), hikariPool.getAutoCommit()); } void resetConnectionState(final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java index 6bb26168..35a7be79 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java @@ -83,12 +83,14 @@ public abstract class ProxyConnection implements Connection SQL_ERRORS.add("JZ0C1"); // Sybase disconnect error } - protected ProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now) { + protected ProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now, final boolean isReadOnly, final boolean isAutoCommit) { this.poolEntry = poolEntry; this.delegate = connection; this.openStatements = openStatements; this.leakTask = leakTask; this.lastAccess = now; + this.isReadOnly = isReadOnly; + this.isAutoCommit = isAutoCommit; } /** {@inheritDoc} */ @@ -102,16 +104,9 @@ public abstract class ProxyConnection implements Connection } // *********************************************************************** - // Connection State init & getters + // Connection State Accessors // *********************************************************************** - final void init(boolean isReadOnly, boolean isAutoCommit) - { - // only these two are used in close so set them correctly. - this.isReadOnly = isReadOnly; - this.isAutoCommit = isAutoCommit; - } - final boolean getAutoCommitState() { return isAutoCommit; diff --git a/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java b/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java index b41d78ce..026debbe 100644 --- a/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java +++ b/src/main/java/com/zaxxer/hikari/pool/ProxyFactory.java @@ -44,9 +44,11 @@ public final class ProxyFactory * @param openStatements * @param leakTask * @param now + * @param isReadOnly + * @param isAutoCommit * @return a proxy that wraps the specified {@link Connection} */ - static ProxyConnection getProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now) + static ProxyConnection getProxyConnection(final PoolEntry poolEntry, final Connection connection, final FastList openStatements, final ProxyLeakTask leakTask, final long now, final boolean isReadOnly, final boolean isAutoCommit) { // Body is replaced (injected) by JavassistProxyFactory throw new IllegalStateException("You need to run the CLI build and you need target/classes in your classpath to run."); From 3218166f7d2416fdecd8fc2e231ed6ed0613c366 Mon Sep 17 00:00:00 2001 From: Nitin Date: Fri, 9 Oct 2015 12:50:24 +0530 Subject: [PATCH 32/33] remove method calls from hot path and sleep in smaller steps --- src/main/java/com/zaxxer/hikari/pool/HikariPool.java | 2 +- src/main/java/com/zaxxer/hikari/pool/PoolBase.java | 12 +----------- src/main/java/com/zaxxer/hikari/pool/PoolEntry.java | 11 ++++++++--- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index e4592c00..d6e8ed28 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -302,7 +302,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL while (poolState == POOL_NORMAL && totalConnections.get() < maxPoolSize && getIdleConnections() <= minimumIdle && !addConnection()) { // If we got into the loop, addConnection() failed, so we sleep and retry quietlySleep(sleepBackoff); - sleepBackoff = Math.min(connectionTimeout / 2, (long) (sleepBackoff * 1.5)); + sleepBackoff = Math.min(connectionTimeout / 2, (long) (sleepBackoff * 1.3)); } } }, true); diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 8074c0d6..5e3e2ccb 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -168,17 +168,7 @@ abstract class PoolBase PoolEntry newPoolEntry() throws Exception { - return new PoolEntry(newConnection(), this); - } - - boolean getReadOnly() - { - return isReadOnly; - } - - boolean getAutoCommit() - { - return isAutoCommit; + return new PoolEntry(newConnection(), this, isReadOnly, isAutoCommit); } void resetConnectionState(final Connection connection, final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java index e02f4396..b41828af 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolEntry.java @@ -44,11 +44,14 @@ final class PoolEntry implements IConcurrentBagEntry long lastBorrowed; private volatile boolean evict; + private volatile ScheduledFuture endOfLife; + private final FastList openStatements; private final HikariPool hikariPool; private final AtomicInteger state; - private volatile ScheduledFuture endOfLife; + private final boolean isReadOnly; + private final boolean isAutoCommit; static { @@ -60,13 +63,15 @@ final class PoolEntry implements IConcurrentBagEntry }; } - PoolEntry(final Connection connection, final PoolBase pool) + PoolEntry(final Connection connection, final PoolBase pool, final boolean isReadOnly, final boolean isAutoCommit) { this.connection = connection; this.hikariPool = (HikariPool) pool; this.state = new AtomicInteger(STATE_NOT_IN_USE); this.lastAccessed = ClockSource.INSTANCE.currentTime(); this.openStatements = new FastList<>(Statement.class, 16); + this.isReadOnly = isReadOnly; + this.isAutoCommit = isAutoCommit; } /** @@ -90,7 +95,7 @@ final class PoolEntry implements IConcurrentBagEntry Connection createProxyConnection(final ProxyLeakTask leakTask, final long now) { - return ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now, hikariPool.getReadOnly(), hikariPool.getAutoCommit()); + return ProxyFactory.getProxyConnection(this, connection, openStatements, leakTask, now, isReadOnly, isAutoCommit); } void resetConnectionState(final ProxyConnection proxyConnection, final int dirtyBits) throws SQLException From ce71aa8dc7adcb57b4f1072cd5bf0b24f7829c5b Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Fri, 9 Oct 2015 17:25:09 +0900 Subject: [PATCH 33/33] Unit test for issue #451 --- .../pool/AddConnectionRaceConditionTest.java | 83 +++++++++++++++++++ .../pool/TestConnectionTimeoutRetry.java | 16 +++- 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/test/java/com/zaxxer/hikari/pool/AddConnectionRaceConditionTest.java diff --git a/src/test/java/com/zaxxer/hikari/pool/AddConnectionRaceConditionTest.java b/src/test/java/com/zaxxer/hikari/pool/AddConnectionRaceConditionTest.java new file mode 100644 index 00000000..b5728234 --- /dev/null +++ b/src/test/java/com/zaxxer/hikari/pool/AddConnectionRaceConditionTest.java @@ -0,0 +1,83 @@ +package com.zaxxer.hikari.pool; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import org.junit.Test; + +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; + +/** + * @author Matthew Tambara (matthew.tambara@liferay.com) + */ +public class AddConnectionRaceConditionTest +{ + private HikariPool _hikariPool; + + // @Test + public void testRaceCondition() throws Exception + { + HikariConfig config = new HikariConfig(); + config.setMinimumIdle(0); + config.setMaximumPoolSize(10); + config.setInitializationFailFast(false); + config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource"); + + // Initialize HikariPool with no initial connections and room to grow + try (final HikariDataSource ds = new HikariDataSource(config)) { + _hikariPool = TestElf.getPool(ds); + + ExecutorService threadPool = Executors.newFixedThreadPool(2); + for (int i = 0; i < 100000; i++) { + Future submit1 = threadPool.submit(new Callable() { + /** {@inheritDoc} */ + @Override + public Exception call() throws Exception + { + Connection c2; + try { + c2 = _hikariPool.getConnection(5000); + ds.evictConnection(c2); + } + catch (SQLException e) { + return e; + } + return null; + } + }); + + Future submit2 = threadPool.submit(new Callable() { + /** {@inheritDoc} */ + @Override + public Exception call() throws Exception + { + Connection c2; + try { + c2 = _hikariPool.getConnection(5000); + ds.evictConnection(c2); + } + catch (SQLException e) { + return e; + } + return null; + } + }); + + if (submit1.get() != null) { + throw submit1.get(); + } + if (submit2.get() != null) { + throw submit2.get(); + } + } + } + catch (Exception e) { + throw e; + } + } +} diff --git a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java index 6ebe251c..e2172d91 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestConnectionTimeoutRetry.java @@ -1,5 +1,7 @@ package com.zaxxer.hikari.pool; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; import java.sql.Connection; import java.sql.SQLException; import java.util.concurrent.Executors; @@ -9,6 +11,7 @@ import java.util.concurrent.TimeUnit; import org.junit.Assert; import org.junit.Test; +import com.sun.media.jfxmedia.logging.Logger; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; import com.zaxxer.hikari.mocks.StubConnection; @@ -225,6 +228,12 @@ public class TestConnectionTimeoutRetry System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "400"); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos, true); + TestElf.setSlf4jTargetStream(HikariPool.class, ps); + TestElf.setSlf4jLogLevel(HikariPool.class, Logger.DEBUG); + + boolean success = false; try (HikariDataSource ds = new HikariDataSource(config)) { Connection connection1 = ds.getConnection(); Connection connection2 = ds.getConnection(); @@ -234,7 +243,7 @@ public class TestConnectionTimeoutRetry Connection connection6 = ds.getConnection(); Connection connection7 = ds.getConnection(); - Thread.sleep(2500); + Thread.sleep(900); Assert.assertSame("Total connections not as expected", 10, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 3, TestElf.getPool(ds).getIdleConnections()); @@ -249,9 +258,14 @@ public class TestConnectionTimeoutRetry Assert.assertSame("Totals connections not as expected", 10, TestElf.getPool(ds).getTotalConnections()); Assert.assertSame("Idle connections not as expected", 10, TestElf.getPool(ds).getIdleConnections()); + success = true; } finally { + TestElf.setSlf4jLogLevel(HikariPool.class, Logger.INFO); System.getProperties().remove("com.zaxxer.hikari.housekeeping.periodMs"); + if (!success) { + System.err.println(new String(baos.toByteArray())); + } } } }