diff --git a/pom.xml b/pom.xml index e8dc31d4..5229b4dc 100644 --- a/pom.xml +++ b/pom.xml @@ -187,6 +187,12 @@ <version>${pax.url.version}</version> <scope>test</scope> </dependency> + <dependency> + <groupId>com.h2database</groupId> + <artifactId>h2</artifactId> + <version>1.4.191</version> + <scope>test</scope> + </dependency> </dependencies> <build> diff --git a/src/main/java/com/zaxxer/hikari/HikariConfig.java b/src/main/java/com/zaxxer/hikari/HikariConfig.java index 07742618..31dff494 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfig.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfig.java @@ -53,6 +53,7 @@ public class HikariConfig implements HikariConfigMXBean private static final long VALIDATION_TIMEOUT = SECONDS.toMillis(5); private static final long IDLE_TIMEOUT = MINUTES.toMillis(10); private static final long MAX_LIFETIME = MINUTES.toMillis(30); + private static final int DEFAULT_POOL_SIZE = 10; private static boolean unitTest; @@ -825,7 +826,7 @@ public class HikariConfig implements HikariConfigMXBean } if (maxPoolSize < 1) { - maxPoolSize = (minIdle <= 0) ? 10 : minIdle; + maxPoolSize = (minIdle <= 0) ? DEFAULT_POOL_SIZE : minIdle; } if (minIdle < 0 || minIdle > maxPoolSize) { diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 1a1369e5..cb4136be 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -131,9 +131,9 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL this.houseKeepingExecutorService = config.getScheduledExecutorService(); } - this.houseKeepingExecutorService.scheduleWithFixedDelay(new HouseKeeper(), 0L, HOUSEKEEPING_PERIOD_MS, MILLISECONDS); - this.leakTask = new ProxyLeakTask(config.getLeakDetectionThreshold(), houseKeepingExecutorService); + + this.houseKeepingExecutorService.scheduleWithFixedDelay(new HouseKeeper(), 100L, HOUSEKEEPING_PERIOD_MS, MILLISECONDS); } /** @@ -587,54 +587,59 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL @Override public void run() { - // refresh timeouts in case they changed via MBean - connectionTimeout = config.getConnectionTimeout(); - validationTimeout = config.getValidationTimeout(); - leakTask.updateLeakDetectionThreshold(config.getLeakDetectionThreshold()); - - final long idleTimeout = config.getIdleTimeout(); - final long now = clockSource.currentTime(); - - // Detect retrograde time, allowing +128ms as per NTP spec. - if (clockSource.plusMillis(now, 128) < clockSource.plusMillis(previous, HOUSEKEEPING_PERIOD_MS)) { - LOGGER.warn("{} - Retrograde clock change detected (housekeeper delta={}), soft-evicting connections from pool.", - clockSource.elapsedDisplayString(previous, now), poolName); + try { + // refresh timeouts in case they changed via MBean + connectionTimeout = config.getConnectionTimeout(); + validationTimeout = config.getValidationTimeout(); + leakTask.updateLeakDetectionThreshold(config.getLeakDetectionThreshold()); + + final long idleTimeout = config.getIdleTimeout(); + final long now = clockSource.currentTime(); + + // Detect retrograde time, allowing +128ms as per NTP spec. + if (clockSource.plusMillis(now, 128) < clockSource.plusMillis(previous, HOUSEKEEPING_PERIOD_MS)) { + LOGGER.warn("{} - Retrograde clock change detected (housekeeper delta={}), soft-evicting connections from pool.", + clockSource.elapsedDisplayString(previous, now), poolName); + previous = now; + softEvictConnections(); + fillPool(); + return; + } + else if (now > clockSource.plusMillis(previous, (3 * HOUSEKEEPING_PERIOD_MS) / 2)) { + // No point evicting for forward clock motion, this merely accelerates connection retirement anyway + LOGGER.warn("{} - Thread starvation or clock leap detected (housekeeper delta={}).", clockSource.elapsedDisplayString(previous, now), poolName); + } + previous = now; - softEvictConnections(); - fillPool(); - return; - } - else if (now > clockSource.plusMillis(previous, (3 * HOUSEKEEPING_PERIOD_MS) / 2)) { - // No point evicting for forward clock motion, this merely accelerates connection retirement anyway - LOGGER.warn("{} - Thread starvation or clock leap detected (housekeeper delta={}).", clockSource.elapsedDisplayString(previous, now), poolName); - } - - previous = now; - - String afterPrefix = "Pool "; - if (idleTimeout > 0L) { - final List<PoolEntry> idleList = connectionBag.values(STATE_NOT_IN_USE); - int removable = idleList.size() - config.getMinimumIdle(); - if (removable > 0) { - logPoolState("Before cleanup "); - afterPrefix = "After cleanup "; - - // Sort pool entries on lastAccessed - Collections.sort(idleList, LASTACCESS_COMPARABLE); - for (PoolEntry poolEntry : idleList) { - if (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > idleTimeout && connectionBag.reserve(poolEntry)) { - closeConnection(poolEntry, "(connection has passed idleTimeout)"); - if (--removable == 0) { - break; // keep min idle cons + + String afterPrefix = "Pool "; + if (idleTimeout > 0L) { + final List<PoolEntry> idleList = connectionBag.values(STATE_NOT_IN_USE); + int removable = idleList.size() - config.getMinimumIdle(); + if (removable > 0) { + logPoolState("Before cleanup "); + afterPrefix = "After cleanup "; + + // Sort pool entries on lastAccessed + Collections.sort(idleList, LASTACCESS_COMPARABLE); + for (PoolEntry poolEntry : idleList) { + if (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > idleTimeout && connectionBag.reserve(poolEntry)) { + closeConnection(poolEntry, "(connection has passed idleTimeout)"); + if (--removable == 0) { + break; // keep min idle cons + } } } } } + + logPoolState(afterPrefix); + + fillPool(); // Try to maintain minimum connections + } + catch (Exception e) { + LOGGER.error("Unexpected exception in housekeeping task", e); } - - logPoolState(afterPrefix); - - fillPool(); // Try to maintain minimum connections } } diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 50388409..9929ccd6 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -12,6 +12,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import java.lang.management.ManagementFactory; import java.sql.Connection; import java.sql.SQLException; +import java.sql.SQLTransientConnectionException; import java.sql.Statement; import java.util.Properties; import java.util.concurrent.Executor; @@ -313,6 +314,10 @@ abstract class PoolBase String password = config.getPassword(); connection = (username == null) ? dataSource.getConnection() : dataSource.getConnection(username, password); + if (connection == null) { + throw new SQLTransientConnectionException("DataSource returned null unexpectedly"); + } + setupConnection(connection); lastConnectionFailure.set(null); return connection; diff --git a/src/test/java/com/zaxxer/hikari/db/BasicPoolTest.java b/src/test/java/com/zaxxer/hikari/db/BasicPoolTest.java new file mode 100644 index 00000000..4ac4f21f --- /dev/null +++ b/src/test/java/com/zaxxer/hikari/db/BasicPoolTest.java @@ -0,0 +1,144 @@ +/* + * Copyright (C) 2016 Brett Wooldridge + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.zaxxer.hikari.db; + +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.concurrent.TimeUnit; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; +import com.zaxxer.hikari.pool.HikariPool; +import com.zaxxer.hikari.pool.TestElf; + +/** + * @author brettw + * + */ +public class BasicPoolTest +{ + @Before + public void setup() throws SQLException + { + HikariConfig config = new HikariConfig(); + config.setMinimumIdle(1); + config.setMaximumPoolSize(2); + config.setConnectionTestQuery("SELECT 1"); + config.setDataSourceClassName("org.h2.jdbcx.JdbcDataSource"); + config.addDataSourceProperty("url", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1"); + + try (HikariDataSource ds = new HikariDataSource(config); + Connection conn = ds.getConnection(); + Statement stmt = conn.createStatement()) { + stmt.executeUpdate("DROP TABLE IF EXISTS basic_pool_test"); + stmt.executeUpdate("CREATE TABLE basic_pool_test (" + + "id INTEGER NOT NULL IDENTITY PRIMARY KEY, " + + "timestamp TIMESTAMP, " + + "string VARCHAR(128), " + + "string_from_number NUMERIC " + + ")"); + } + } + + @Test + public void testIdleTimeout() throws InterruptedException, SQLException + { + HikariConfig config = new HikariConfig(); + config.setMinimumIdle(5); + config.setMaximumPoolSize(10); + config.setConnectionTestQuery("SELECT 1"); + config.setDataSourceClassName("org.h2.jdbcx.JdbcDataSource"); + config.addDataSourceProperty("url", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1"); + + System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "1000"); + + try (HikariDataSource ds = new HikariDataSource(config)) { + System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs"); + + TimeUnit.SECONDS.sleep(1); + + HikariPool pool = TestElf.getPool(ds); + + ds.setIdleTimeout(3000); + + Assert.assertSame("Total connections not as expected", 5, pool.getTotalConnections()); + Assert.assertSame("Idle connections not as expected", 5, pool.getIdleConnections()); + + Connection connection = ds.getConnection(); + Assert.assertNotNull(connection); + + TimeUnit.MILLISECONDS.sleep(1500); + + Assert.assertSame("Second total connections not as expected", 6, pool.getTotalConnections()); + Assert.assertSame("Second idle connections not as expected", 5, pool.getIdleConnections()); + connection.close(); + + Assert.assertSame("Idle connections not as expected", 6, pool.getIdleConnections()); + + TimeUnit.SECONDS.sleep(2); + + Assert.assertSame("Third total connections not as expected", 5, pool.getTotalConnections()); + Assert.assertSame("Third idle connections not as expected", 5, pool.getIdleConnections()); + } + } + + @Test + public void testIdleTimeout2() throws InterruptedException, SQLException + { + HikariConfig config = new HikariConfig(); + config.setMaximumPoolSize(50); + config.setConnectionTestQuery("SELECT 1"); + config.setDataSourceClassName("org.h2.jdbcx.JdbcDataSource"); + config.addDataSourceProperty("url", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1"); + + System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "1000"); + + try (HikariDataSource ds = new HikariDataSource(config)) { + System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs"); + + TimeUnit.SECONDS.sleep(1); + + HikariPool pool = TestElf.getPool(ds); + + ds.setIdleTimeout(3000); + + Assert.assertSame("Total connections not as expected", 50, pool.getTotalConnections()); + Assert.assertSame("Idle connections not as expected", 50, pool.getIdleConnections()); + + Connection connection = ds.getConnection(); + Assert.assertNotNull(connection); + + TimeUnit.MILLISECONDS.sleep(1500); + + Assert.assertSame("Second total connections not as expected", 50, pool.getTotalConnections()); + Assert.assertSame("Second idle connections not as expected", 49, pool.getIdleConnections()); + connection.close(); + + Assert.assertSame("Idle connections not as expected", 50, pool.getIdleConnections()); + + TimeUnit.SECONDS.sleep(3); + + Assert.assertSame("Third total connections not as expected", 50, pool.getTotalConnections()); + Assert.assertSame("Third idle connections not as expected", 50, pool.getIdleConnections()); + } + } +} diff --git a/src/test/java/com/zaxxer/hikari/pool/TestConnections.java b/src/test/java/com/zaxxer/hikari/pool/TestConnections.java index 9f1112dc..070fcfd3 100644 --- a/src/test/java/com/zaxxer/hikari/pool/TestConnections.java +++ b/src/test/java/com/zaxxer/hikari/pool/TestConnections.java @@ -508,4 +508,48 @@ public class TestConnections Assert.assertSame("Bad query or something.", e.getNextException().getMessage()); } } + + @Test + public void testPopulationSlowAcquisition() throws InterruptedException, SQLException + { + HikariConfig config = new HikariConfig(); + config.setPoolName("SlowAcquisition"); + config.setMaximumPoolSize(30); + config.setConnectionTestQuery("VALUES 1"); + config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource"); + + System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "1000"); + + StubConnection.slowCreate = true; + try (HikariDataSource ds = new HikariDataSource(config)) { + System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs"); + + ds.setIdleTimeout(3000); + + TimeUnit.SECONDS.sleep(2); + + HikariPool pool = TestElf.getPool(ds); + Assert.assertSame("Total connections not as expected", 1, pool.getTotalConnections()); + Assert.assertSame("Idle connections not as expected", 1, pool.getIdleConnections()); + + Connection connection = ds.getConnection(); + Assert.assertNotNull(connection); + + TimeUnit.SECONDS.sleep(30); + + Assert.assertSame("Second total connections not as expected", 30, pool.getTotalConnections()); + Assert.assertSame("Second idle connections not as expected", 29, pool.getIdleConnections()); + connection.close(); + + Assert.assertSame("Idle connections not as expected", 30, pool.getIdleConnections()); + + TimeUnit.SECONDS.sleep(5); + + Assert.assertSame("Third total connections not as expected", 30, pool.getTotalConnections()); + Assert.assertSame("Third idle connections not as expected", 30, pool.getIdleConnections()); + } + finally { + StubConnection.slowCreate = false; + } + } }