Index: src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/PoolConfiguration.java (working copy) @@ -524,6 +524,19 @@ public void setValidationQuery(String validationQuery); /** + * The timeout in seconds before a connection validation queries fail. + * A value less than or equal to zero will disable this feature. Defaults to -1. + * @return the timeout value in seconds + */ + public int getValidationQueryTimeout(); + + /** + * The timeout in seconds before a connection validation queries fail. + * A value less than or equal to zero will disable this feature. Defaults to -1. + */ + public void setValidationQueryTimeout(int validationQueryTimeout); + + /** * Return the name of the optional validator class - may be null. * * @return the name of the optional validator class - may be null Index: src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java (working copy) @@ -451,6 +451,12 @@ Statement stmt = null; try { stmt = connection.createStatement(); + + int validationQueryTimeout = poolProperties.getValidationQueryTimeout(); + if (validationQueryTimeout > 0) { + stmt.setQueryTimeout(validationQueryTimeout); + } + stmt.execute(query); stmt.close(); this.lastValidated = now; Index: src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java (working copy) @@ -310,6 +310,11 @@ public String getValidationQuery() { return getPoolProperties().getValidationQuery(); } + + @Override + public int getValidationQueryTimeout() { + return getPoolProperties().getValidationQueryTimeout(); + } /** * {@inheritDoc} @@ -664,6 +669,11 @@ getPoolProperties().setValidationQuery(validationQuery); } + @Override + public void setValidationQueryTimeout(int validationQueryTimeout) { + getPoolProperties().setValidationQueryTimeout(validationQueryTimeout); + } + /** * {@inheritDoc} */ Index: src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/DataSourceFactory.java (working copy) @@ -80,6 +80,7 @@ protected static final String PROP_TESTWHILEIDLE = "testWhileIdle"; protected static final String PROP_TESTONCONNECT = "testOnConnect"; protected static final String PROP_VALIDATIONQUERY = "validationQuery"; + protected static final String PROP_VALIDATIONQUERY_TIMEOUT = "validationQueryTimeout"; protected static final String PROP_VALIDATOR_CLASS_NAME = "validatorClassName"; protected static final String PROP_NUMTESTSPEREVICTIONRUN = "numTestsPerEvictionRun"; @@ -149,6 +150,7 @@ PROP_URL, PROP_USERNAME, PROP_VALIDATIONQUERY, + PROP_VALIDATIONQUERY_TIMEOUT, PROP_VALIDATOR_CLASS_NAME, PROP_VALIDATIONINTERVAL, PROP_ACCESSTOUNDERLYINGCONNECTIONALLOWED, @@ -366,6 +368,11 @@ if (value != null) { poolProperties.setValidationQuery(value); } + + value = properties.getProperty(PROP_VALIDATIONQUERY_TIMEOUT); + if (value != null) { + poolProperties.setValidationQueryTimeout(Integer.parseInt(value)); + } value = properties.getProperty(PROP_VALIDATOR_CLASS_NAME); if (value != null) { Index: src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java (working copy) @@ -713,7 +713,7 @@ } else { //validation failed, make sure we disconnect //and clean up - error =true; + throw new SQLException("Validation Query Failed, enable logValidationErrors for more details."); } //end if } catch (Exception e) { error = true; @@ -733,7 +733,6 @@ } con.unlock(); }//catch - return null; } /** Index: src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java (working copy) @@ -57,6 +57,7 @@ private volatile int minIdle = initialSize; private volatile int maxWait = 30000; private volatile String validationQuery; + private volatile int validationQueryTimeout = -1; private volatile String validatorClassName; private volatile Validator validator; private volatile boolean testOnBorrow = false; @@ -382,6 +383,22 @@ /** * {@inheritDoc} */ + @Override + public int getValidationQueryTimeout() { + return validationQueryTimeout; + } + + /** + * {@inheritDoc} + */ + @Override + public void setValidationQueryTimeout(int validationQueryTimeout) { + this.validationQueryTimeout = validationQueryTimeout; + } + + /** + * {@inheritDoc} + */ @Override public String getValidatorClassName() { Index: src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java (working copy) @@ -452,6 +452,15 @@ public void setValidatorClassName(String className) { this.poolProperties.setValidatorClassName(className); } + + /** + * {@inheritDoc} + */ + + @Override + public void setValidationQueryTimeout(int validationQueryTimeout) { + this.poolProperties.setValidationQueryTimeout(validationQueryTimeout); + } /** * {@inheritDoc} @@ -928,6 +937,15 @@ /** * {@inheritDoc} */ + + @Override + public int getValidationQueryTimeout() { + return getPoolProperties().getValidationQueryTimeout(); + } + + /** + * {@inheritDoc} + */ @Override public String getValidatorClassName() { Index: src/main/java/org/apache/tomcat/jdbc/pool/mbeans-descriptors.xml =================================================================== --- src/main/java/org/apache/tomcat/jdbc/pool/mbeans-descriptors.xml (revision 1456054) +++ src/main/java/org/apache/tomcat/jdbc/pool/mbeans-descriptors.xml (working copy) @@ -125,6 +125,11 @@ description="The query to run during validation" type="java.lang.String" writeable="false"/> + + SELECT 1(mysql), select 1 from dual(oracle), SELECT 1(MS Sql Server)

+ + +

(int) The timeout in seconds before a connection validation queries fail. This works by calling + java.sql.Statement.setQueryTimeout(seconds) on the statement that executes the validationQuery. + The pool itself doesn't timeout the query, it is still up to the JDBC driver to enforce query timeouts. + A value less than or equal to zero will disable this feature. + The default value is -1. +

+

(String) The name of a class which implements the Index: src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java =================================================================== --- src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java (revision 0) +++ src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java (revision 0) @@ -0,0 +1,250 @@ +package org.apache.tomcat.jdbc.test; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.sql.Statement; +import java.util.Properties; +import java.util.logging.Logger; + +import junit.framework.Assert; + +import org.apache.tomcat.jdbc.pool.interceptor.QueryTimeoutInterceptor; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class TestValidationQueryTimeout extends DefaultTestCase { + + private static int TIMEOUT = 10; + private static boolean isTimeoutSet; + private static final String longQuery = "select * from test as A, test as B, test as C, test as D, test as E"; + + @Before + public void setUp() throws SQLException { + DriverManager.registerDriver(new MockDriver()); + + // use our mock driver + this.datasource.setDriverClassName(MockDriver.class.getName()); + this.datasource.setUrl(MockDriver.url); + + // Required to trigger validation query's execution + this.datasource.setInitialSize(1); + this.datasource.setTestOnBorrow(true); + this.datasource.setValidationInterval(-1); + this.datasource.setValidationQuery("SELECT 1"); + this.datasource.setValidationQueryTimeout(TIMEOUT); + + TIMEOUT = 10; + isTimeoutSet = false; + } + + @After + public void tearDown() throws SQLException { + DriverManager.deregisterDriver(new MockDriver()); + } + + @Test + public void testValidationQueryTimeoutEnabled() throws Exception { + // because testOnBorrow is true, this triggers the validation query + this.datasource.getConnection(); + Assert.assertTrue(isTimeoutSet); + } + + @Test + public void testValidationQueryTimeoutDisabled() throws Exception { + this.datasource.setValidationQueryTimeout(-1); + + // because testOnBorrow is true, this triggers the validation query + this.datasource.getConnection(); + Assert.assertFalse(isTimeoutSet); + } + + @Test + public void testValidationQueryTimeoutWithQueryTimeoutInterceptor() throws Exception { + int interceptorTimeout = 30; + this.datasource.setJdbcInterceptors( + QueryTimeoutInterceptor.class.getName()+ + "(queryTimeout="+ interceptorTimeout +")"); + + // because testOnBorrow is true, this triggers the validation query + Connection con = this.datasource.getConnection(); + Assert.assertTrue(isTimeoutSet); + + // increase the expected timeout to 30, which is what we set for the interceptor + TIMEOUT = 30; + + // now create a statement, make sure the query timeout is set by the interceptor + Statement st = con.createStatement(); + Assert.assertEquals(interceptorTimeout, st.getQueryTimeout()); + st.close(); + st = con.prepareStatement(""); + Assert.assertEquals(interceptorTimeout, st.getQueryTimeout()); + st.close(); + st = con.prepareCall(""); + Assert.assertEquals(interceptorTimeout, st.getQueryTimeout()); + st.close(); + con.close(); + + // pull another connection and check it + TIMEOUT = 10; + isTimeoutSet = false; + this.datasource.getConnection(); + Assert.assertTrue(isTimeoutSet); + } + + // this test depends on the execution time of the validation query + // specifically, it needs to run for longer than 1 second to pass + // if this fails + @Test(expected=SQLException.class) + public void testValidationQueryTimeoutOnConnection() throws Exception { + // use our mock driver + this.datasource.setDriverClassName("org.h2.Driver"); + this.datasource.setUrl("jdbc:h2:~/.h2/test;QUERY_TIMEOUT=0;DB_CLOSE_ON_EXIT=FALSE"); + + // Required to trigger validation query's execution + this.datasource.setTestOnConnect(true); + this.datasource.setValidationInterval(-1); + this.datasource.setValidationQuery(longQuery); + this.datasource.setValidationQueryTimeout(1); + + this.datasource.getConnection(); + } + + @Test(expected=SQLException.class) + public void testValidationInvalidOnConnection() throws Exception { + // use our mock driver + this.datasource.setDriverClassName("org.h2.Driver"); + this.datasource.setUrl("jdbc:h2:~/.h2/test;QUERY_TIMEOUT=0;DB_CLOSE_ON_EXIT=FALSE"); + + // Required to trigger validation query's execution + this.datasource.setTestOnBorrow(true); + this.datasource.setInitialSize(1); + this.datasource.setTestOnConnect(true); + this.datasource.setValidationInterval(-1); + this.datasource.setValidationQuery("SELECT"); + this.datasource.setValidationQueryTimeout(1); + + this.datasource.getConnection(); + } + + @Test + public void testLongValidationQueryTime() throws Exception { + // use our mock driver + this.datasource.setDriverClassName("org.h2.Driver"); + this.datasource.setUrl("jdbc:h2:~/.h2/test;QUERY_TIMEOUT=0;DB_CLOSE_ON_EXIT=FALSE"); + Connection con = this.datasource.getConnection(); + Statement stmt = null; + long start = 0, end = 0; + try { + stmt = con.createStatement(); + // set the query timeout to 2 sec + // this keeps this test from slowing things down too much + stmt.setQueryTimeout(2); + // assert that our long query takes longer than one second to run + // this is a requirement for other tests to run properly + start = System.currentTimeMillis(); + stmt.execute(longQuery); + } catch (SQLException ex) {} + finally { + end = System.currentTimeMillis(); + + if (stmt != null) { stmt.close(); } + if (con != null) { con.close(); } + + Assert.assertTrue(start != 0 && end != 0); + Assert.assertTrue((end - start) > 1000); + } + } + + @Test + public void testValidationQueryTimeoutOnBorrow() throws Exception { + // use our mock driver + this.datasource.setDriverClassName("org.h2.Driver"); + this.datasource.setUrl("jdbc:h2:~/.h2/test;QUERY_TIMEOUT=0;DB_CLOSE_ON_EXIT=FALSE"); + + // Required to trigger validation query's execution + this.datasource.setTestOnBorrow(true); + this.datasource.setValidationInterval(-1); + this.datasource.setValidationQuery(longQuery); + this.datasource.setValidationQueryTimeout(1); + + // assert that even though the validation query times out, we still get a connection + Connection con = this.datasource.getConnection(); + Assert.assertNotNull(con); + Statement st = con.createStatement(); + ResultSet rs = st.executeQuery("SELECT 1"); + rs.close(); + st.close(); + con.close(); + } + + /** + * Mock Driver, Connection and Statement implementations use to verify setQueryTimeout was called. + */ + public static class MockDriver implements java.sql.Driver { + public static final String url = "jdbc:tomcat:mock"; + + public MockDriver() { + } + + @Override + public boolean acceptsURL(String url) throws SQLException { + return url!=null && url.equals(MockDriver.url); + } + + @Override + public Connection connect(String url, Properties info) throws SQLException { + return new MockConnection(info); + } + + @Override + public int getMajorVersion() { + return 0; + } + + @Override + public int getMinorVersion() { + return 0; + } + + @Override + public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { + return null; + } + + @Override + public boolean jdbcCompliant() { + return false; + } + + @Override + public Logger getParentLogger() throws SQLFeatureNotSupportedException { + return null; + } + } + + public static class MockConnection extends org.apache.tomcat.jdbc.test.driver.Connection { + public MockConnection(Properties info) { + super(info); + } + + @Override + public Statement createStatement() throws SQLException { + return new MockStatement(); + } + } + + public static class MockStatement extends org.apache.tomcat.jdbc.test.driver.Statement { + @Override + public void setQueryTimeout(int seconds) throws SQLException { + super.setQueryTimeout(seconds); + Assert.assertEquals(TIMEOUT, seconds); + isTimeoutSet = true; + } + } + +}