Merge pull request #347 from nitincchauhan/dev

HikariPool: Log consitency

src/main/java/com/zaxxer/hikari/HikariConfig.java

@@ -452,7 +452,7 @@ public class HikariConfig implements HikariConfigMXBean
    public void setMetricsTrackerFactory(MetricsTrackerFactory metricsTrackerFactory)
    {
       if (metricRegistry != null) {
-         throw new IllegalStateException("setMetricsTrackerFactory() cannot used in combination with setMetricRegistry()");
+         throw new IllegalStateException("cannot use setMetricsTrackerFactory() and setMetricRegistry() together");
       }
 
       this.metricsTrackerFactory = metricsTrackerFactory;
@@ -476,7 +476,7 @@ public class HikariConfig implements HikariConfigMXBean
    public void setMetricRegistry(Object metricRegistry)
    {
       if (metricsTrackerFactory != null) {
-         throw new IllegalStateException("setMetricRegistry() cannot used in combination with setMetricsTrackerFactory()");
+         throw new IllegalStateException("cannot use setMetricRegistry() and setMetricsTrackerFactory() together");
       }
 
       if (metricRegistry != null) {
@@ -760,11 +760,11 @@ public class HikariConfig implements HikariConfigMXBean
 
       if (driverClassName != null && jdbcUrl == null) {
          logger.error("jdbcUrl is required with driverClassName");
-         throw new IllegalStateException("jdbcUrl is required with driverClassName");
+         throw new IllegalArgumentException("jdbcUrl is required with driverClassName");
       }
       else if (driverClassName != null && dataSourceClassName != null) {
          logger.error("cannot use driverClassName and dataSourceClassName together");
-         throw new IllegalStateException("cannot use driverClassName and dataSourceClassName together");
+         throw new IllegalArgumentException("cannot use driverClassName and dataSourceClassName together");
       }
       else if (jdbcUrl != null) {
          // OK
@@ -824,7 +824,7 @@ public class HikariConfig implements HikariConfigMXBean
 
    private void logConfiguration()
    {
-      LOGGER.debug("HikariCP pool {} configuration:", poolName);
+      LOGGER.debug("{} - configuration:", poolName);
       final Set<String> propertyNames = new TreeSet<>(PropertyElf.getPropertyNames(HikariConfig.class));
       for (String prop : propertyNames) {
          try {

src/main/java/com/zaxxer/hikari/HikariDataSource.java

@@ -69,7 +69,7 @@ public class HikariDataSource extends HikariConfig implements DataSource, Closea
       configuration.validate();
       configuration.copyState(this);
 
-      LOGGER.info("Hikari pool {} is starting.", configuration.getPoolName());
+      LOGGER.info("{} - is starting.", configuration.getPoolName());
       pool = fastPathPool = new HikariPool(this);
    }
 
@@ -78,7 +78,7 @@ public class HikariDataSource extends HikariConfig implements DataSource, Closea
    public Connection getConnection() throws SQLException
    {
       if (isClosed()) {
-         throw new SQLException("HikariDataSource " + this + " has been shutdown");
+         throw new SQLException("HikariDataSource " + this + " has been closed.");
       }
 
       if (fastPathPool != null) {
@@ -92,7 +92,7 @@ public class HikariDataSource extends HikariConfig implements DataSource, Closea
             result = pool;
             if (result == null) {
                validate();
-               LOGGER.info("Hikari pool {} is starting.", getPoolName());
+               LOGGER.info("{} - is starting.", getPoolName());
                pool = result = new HikariPool(this);
             }
          }
@@ -289,7 +289,7 @@ public class HikariDataSource extends HikariConfig implements DataSource, Closea
             pool.shutdown();
          }
          catch (InterruptedException e) {
-            LoggerFactory.getLogger(getClass()).warn("Interrupted during shutdown", e);
+        	 LOGGER.warn("Interrupted during closing", e);
          }
 
          if (pool.getDataSource() instanceof DriverDataSource) {

src/main/java/com/zaxxer/hikari/pool/HikariPool.java

@@ -24,7 +24,7 @@ import static com.zaxxer.hikari.util.UtilityElf.quietlySleep;
 
 import java.sql.Connection;
 import java.sql.SQLException;
-import java.sql.SQLTimeoutException;
+import java.sql.SQLTransientConnectionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
@@ -196,7 +196,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
          while (timeout > 0L);
       }
       catch (InterruptedException e) {
-         throw new SQLException("Interrupted during connection acquisition", e);
+         throw new SQLException(poolName + " - Interrupted during connection acquisition", e);
       }
       finally {
          suspendResumeLock.release();
@@ -208,7 +208,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
       if (originalException instanceof SQLException) {
          sqlState = ((SQLException) originalException).getSQLState();
       }
-      throw new SQLTimeoutException(String.format("Timeout after %dms of waiting for a connection.", clockSource.elapsedMillis(startTime)), sqlState, originalException);
+      throw new SQLTransientConnectionException(poolName + " - Connection is not available, request timed out after " + clockSource.elapsedMillis(startTime) + "ms.", sqlState, originalException);
    }
 
    /**
@@ -239,8 +239,8 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
       try {
          poolState = POOL_SHUTDOWN;
 
-         LOGGER.info("Hikari pool {} is shutting down.", poolName);
-         logPoolState("Before shutdown ");
+         LOGGER.info("{} - is closing down.", poolName);
+         logPoolState("Before closing ");
 
          connectionBag.close();
          softEvictConnections();
@@ -270,7 +270,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
          closeConnectionExecutor.awaitTermination(5L, TimeUnit.SECONDS);
       }
       finally {
-         logPoolState("After shutdown ");
+         logPoolState("After closing ");
 
          poolElf.unregisterMBeans();
          metricsTracker.close();
@@ -423,7 +423,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
    public final synchronized void suspendPool()
    {
       if (suspendResumeLock == SuspendResumeLock.FAUX_LOCK) {
-         throw new IllegalStateException("Pool " + poolName + " is not suspendable");
+         throw new IllegalStateException(poolName + " - is not suspendable");
       }
       else if (poolState != POOL_SUSPENDED) {
          suspendResumeLock.suspend();
@@ -483,7 +483,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
       // 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
-         lastConnectionFailure.set(new SQLException("Hikari pool " + poolName +" is at maximum capacity"));
+         lastConnectionFailure.set(new SQLException(poolName + " - is at maximum capacity"));
          return true;
       }
 
@@ -497,7 +497,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
 
          connectionBag.add(new PoolBagEntry(connection, this));
          lastConnectionFailure.set(null);
-         LOGGER.debug("{} - Connection {} added to pool", poolName, connection);
+         LOGGER.debug("{} - Added connection {}", poolName, connection);
 
          return true;
       }
@@ -505,7 +505,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener
          totalConnections.decrementAndGet(); // We failed, so undo speculative increment of totalConnections
          lastConnectionFailure.set(e);
          if (poolState == POOL_NORMAL) {
-            LOGGER.debug("{} - Connection attempt to database failed", poolName, e);
+            LOGGER.debug("{} - Cannot acquire connection from data source", poolName, e);
          }
          poolElf.quietlyCloseConnection(connection, "(exception during connection creation)");
          return false;

src/main/java/com/zaxxer/hikari/pool/PoolElf.java

@@ -109,8 +109,22 @@ public final class PoolElf
    {
       if (transactionIsolationName != null) {
          try {
-            Field field = Connection.class.getField(transactionIsolationName);
-            return field.getInt(null);
+            final String upperName = transactionIsolationName.toUpperCase();
+            if (upperName.startsWith("TRANSACTION_")) {
+               Field field = Connection.class.getField(upperName);
+               return field.getInt(null);
+            }
+            final int level = Integer.parseInt(transactionIsolationName);
+            switch (level) {
+               case Connection.TRANSACTION_READ_UNCOMMITTED:
+               case Connection.TRANSACTION_READ_COMMITTED:
+               case Connection.TRANSACTION_REPEATABLE_READ:
+               case Connection.TRANSACTION_SERIALIZABLE:
+               case Connection.TRANSACTION_NONE:
+                  return level;
+               default:
+                  throw new IllegalArgumentException();
+             }
          }
          catch (Exception e) {
             throw new IllegalArgumentException("Invalid transaction isolation value: " + transactionIsolationName);
@@ -165,14 +179,15 @@ public final class PoolElf
       }
 
       networkTimeout = getAndSetNetworkTimeout(connection, connectionTimeout);
-      transactionIsolation = (transactionIsolation < 0 ? connection.getTransactionIsolation() : transactionIsolation);
 
       connection.setAutoCommit(isAutoCommit);
       if (isReadOnly != null) {
          connection.setReadOnly(isReadOnly);
       }
 
-      if (transactionIsolation != connection.getTransactionIsolation()) {
+      final int defaultLevel = connection.getTransactionIsolation();
+      transactionIsolation = (transactionIsolation < 0 ? defaultLevel : transactionIsolation);
+      if (transactionIsolation != defaultLevel) {
          connection.setTransactionIsolation(transactionIsolation);
       }
 
@@ -205,9 +220,7 @@ public final class PoolElf
    
          try (Statement statement = connection.createStatement()) {
             setQueryTimeout(statement, timeoutSec);
-            if (statement.execute(config.getConnectionTestQuery())) {
-               statement.getResultSet().close();
-            }
+            statement.execute(config.getConnectionTestQuery());
          }
    
          if (isIsolateInternalQueries && !isAutoCommit) {
@@ -438,9 +451,7 @@ public final class PoolElf
    {
       if (sql != null) {
          try (Statement statement = connection.createStatement()) {
-            if (statement.execute(sql)) {
-               statement.getResultSet().close();
-            }
+            statement.execute(sql);
 
             if (!isAutoCommit) {
                connection.commit();

src/test/java/com/zaxxer/hikari/ShutdownTest.java

@@ -225,7 +225,7 @@ public class ShutdownTest
            ds.getConnection();
        }
        catch (SQLException e) {
-          Assert.assertTrue(e.getMessage().contains("has been shutdown"));
+          Assert.assertTrue(e.getMessage().contains("has been closed."));
        }
    }
 

src/test/java/com/zaxxer/hikari/TestValidation.java

@@ -59,7 +59,7 @@ public class TestValidation
          config.validate();
          Assert.fail();
       }
-      catch (IllegalStateException ise) {
+      catch (IllegalArgumentException ise) {
          // pass
       }
    }
@@ -183,8 +183,8 @@ public class TestValidation
          config.validate();
          Assert.fail();
       }
-      catch (IllegalStateException ise) {
-         Assert.assertTrue(ise.getMessage().contains("cannot use"));
+      catch (IllegalArgumentException ise) {
+         Assert.assertTrue(ise.getMessage().contains("together"));
       }
    }