HikariCP icon indicating copy to clipboard operation
HikariCP copied to clipboard

SQLTimeoutException closes connection

Open nytro77 opened this issue 5 years ago • 17 comments

Environment

HikariCP version: 3.3.1
JDK version     : 1.8.0_201
Database        : SQL Server
Driver version  : 7.2.2.jre8

If executing a java.sql.Statement results in a SQLTimeoutException then Hikari loggs a warning and force closes the connection.

WARN [pool-4-thread-30] - HikariPool-28 - Connection ConnectionID:10 ClientConnectionId: f3925681-0baf-4cc2-9a16-39ae212b92e8 marked as broken because of SQLSTATE(HY008), ErrorCode(0) [com.zaxxer.hikari.pool.ProxyConnection]
java.sql.SQLTimeoutException: The query has timed out.
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeStatement(SQLServerStatement.java:226) ~[mssql-jdbc-7.2.2.jre8.jar:?]
	at com.microsoft.sqlserver.jdbc.SQLServerStatement.execute(SQLServerStatement.java:744) ~[mssql-jdbc-7.2.2.jre8.jar:?]
	at com.zaxxer.hikari.pool.ProxyStatement.execute(ProxyStatement.java:95) ~[HikariCP-3.3.1.jar:?]
	at com.zaxxer.hikari.pool.HikariProxyStatement.execute(HikariProxyStatement.java) ~[HikariCP-3.3.1.jar:?]
	...

The java.sql.SQLTimeoutException extends java.sql.SQLTransientException and that clearly states in the documentation that

The subclass of SQLException is thrown in situations where a previously failed operation might be able to succeed when the operation is retried without any intervention by application-level functionality

So the JDBC specification says that the connection is still alive and well after a timeout so Hikari should not log a warning or close the connection! It is up to the application to catch any SQLTimeoutException and either retry or throw exception and close the connection.

This bug was introduced in fix of https://github.com/brettwooldridge/HikariCP/issues/1308.

Right now it is impossible to use Hikari with code like this that works just fine when using a raw SQL Server JDBC connection:

try (Connection con = getConnectionFromPool(); Statement stmt = con.createStatement()) {
	stmt.setQueryTimeout(1);

	try (ResultSet rs = stmt.executeQuery(sql)) {
		// Handle result
	} catch (SQLTimeoutException e) {
		// Not done within a second? Use alternate query
		try (ResultSet rs = stmt.executeQuery(alternateSQL)) {
			// Handle result
		}
	}
} catch (SQLException e) {
	// Handle in some way
}

nytro77 avatar Jun 08 '19 09:06 nytro77

cc @brettwooldridge

johnou avatar Sep 18 '19 12:09 johnou

Any progress on this issue? We have been facing this issue with Oracle that lead to partial commits.

eicki avatar Feb 03 '20 10:02 eicki

To explain our issue. If an SQLTimeoutException occurs, Hikari will close the connection. Now Oracle reacts on a close with a commit instead of a rollback, so I end up with a partially committed state in my database. I see the following options:

  • Revert the change that handles SQLTimeoutException
  • Add a rollback call prior to closing the connection in that case

eicki avatar Feb 03 '20 15:02 eicki

@eicki see https://github.com/brettwooldridge/HikariCP/issues/1489#issuecomment-581437268

johnou avatar Feb 03 '20 15:02 johnou

Hi, any news on this? I'm also quite surprised with the connection being closed in the event of a timeout. This kind of error is transient, as the OP explained.

In our scenario, we are using Spring Data JPA, Eclipselink and Oracle DB. Spring sets a query timeout depending on the time left for the transaction (e.g. @Transactional(timeout=20)). So, we can have a timeout either because of a long database call or because of the service chaining multiple calls and running out of time...

Either way, I guess the connection is still alive and well; so it should not be closed. This behavior can impact performance because connections are not reused and need be created again.

It also confuses logs. In our scenario, Spring tries to rollback the transaction because the last query failed, but the rollback itself fails due to the connection being closed. This does not seem to be proper rollback behavior. In order to know that this is motivated by a query timeout, one has to go backwards in the logs to find the cause error: "ORA-01013: user requested cancel of current operation".

aritzbastida avatar Apr 15 '20 13:04 aritzbastida

Hi , I am also facing the same issue.

marked as broken because of SQLSTATE(HY008), ErrorCode(0)java.sql.SQLTimeoutException: The query has timed out. at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeStatement(SQLServerStatement.java:226) at com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement.executeQuery(SQLServerPreparedStatement.java:446) at com.zaxxer.hikari.pool.ProxyPreparedStatement.executeQuery(ProxyPreparedStatement.java:52) at com.zaxxer.hikari.pool.HikariProxyCallableStatement.executeQuery(HikariProxyCallableStatement.java)

talk2debendra avatar Jul 28 '20 11:07 talk2debendra

Same issue for me, this behavior should be considered as a regression. I understand that it was originally introduced as a fix for another user's use case, but it should be a opt-in behavior. The old behavior should be reinstated as the default.

@talk2debendra @aritzbastida As mentionned in @johnou comment, you can implement the suggested workaround. it works fine for me for now. Just make sure you have a test for it to catch a further sneaky breaking change regarding that workaround.

import com.zaxxer.hikari.SQLExceptionOverride;
import java.sql.SQLException;
import java.sql.SQLTimeoutException;

/**
 * Hikari devs decided to close the connection upon encountering a SQLTimeoutException
 * which is wrong and against the JDBC specification
 * This Hikari's  SQLExceptionOverride will restore the old behavior and let the application decide what it should do
 * when a query timeout occurs (e.g.: when purposely configuring a short lock timeout to be caught by the application)
 *
 * @see "https://github.com/brettwooldridge/HikariCP/issues/1308"
 * @see "https://github.com/brettwooldridge/HikariCP/issues/1388"
 * @see "https://github.com/brettwooldridge/HikariCP/issues/1489#issuecomment-581437268"
 * @see "https://github.com/brettwooldridge/HikariCP/blob/dev/src/test/java/com/zaxxer/hikari/pool/TestConnections.java"
 */
public class HikariPreventQueryTimeoutEviction implements SQLExceptionOverride {

	@java.lang.Override
	public Override adjudicate(SQLException ex) {
	    if(ex instanceof SQLTimeoutException) {
	        return Override.DO_NOT_EVICT;
             }
		return null;
	}
}

And then you configure it on your Hikari datasource :

HikariDataSource dataSource = new HikariDataSource();
// (...)
dataSource.setExceptionOverrideClassName(HikariPreventQueryTimeoutEviction.class.getName());
return dataSource;

idkw avatar Oct 13 '20 11:10 idkw

Hi @johnou Do you consider a fix for this bug ? I think this commit should be rollbacked because it introduced the regression : https://github.com/brettwooldridge/HikariCP/commit/cda2605accd258b569bfd9dbf16265faaf1fa18c

idkw avatar Dec 23 '20 18:12 idkw

This is serious behavior. since the atomicity of the database transaction is not respected! The connection shouldnt been closed until the rollback.

tonyldo avatar Feb 28 '21 23:02 tonyldo

Are there any updates on this? I bumped into this issue when using select ... for update queries on H2. These throw SQLTimeoutException when the rows are already locked by another session. Since Hikari closes the connection, proper error recovery (such as performing a rollback) is impossible.

@idkw's workaround works, but that seem rather hacky and introduces a compile-time dependency on Hikari.

kzander91 avatar Dec 13 '21 16:12 kzander91

Are there any updates on this? I bumped into this issue when using select ... for update queries on H2. These throw SQLTimeoutException when the rows are already locked by another session. Since Hikari closes the connection, proper error recovery (such as performing a rollback) is impossible.

@idkw's workaround works, but that seem rather hacky and introduces a compile-time dependency on Hikari.

I'm still using my hacky workaround too. @brettwooldridge Could you have a look at this and rollback the offending commit at https://github.com/brettwooldridge/HikariCP/commit/cda2605accd258b569bfd9dbf16265faaf1fa18c

Thank you

idkw avatar Dec 13 '21 17:12 idkw

Caused by: java.sql.SQLTimeoutException: The query has timed out.

After digging for several hours around this, I completely removed Hikari and used plain SQLServer JDBC data source instead. The problem was gone and then I ended up here.

@idkw Is your workaround still valid for version 5.0.1? I set it using HikariConfig#setExceptionOverrideClassName, but it doesn't seem to get triggered.

petarov avatar Feb 22 '22 14:02 petarov

Any news on this? We also have the same issue. I think it is even wrong to just close the connection on evict. The JDBC close method documentation has this warning: "It is strongly recommended that an application explicitly commits or rolls back an active transaction prior to calling the close method. If the close method is called and there is an active transaction, the results are implementation-defined."

boonelschenbroich avatar Sep 06 '22 16:09 boonelschenbroich

Ok, I will investigate a setting for evict-on-timeout, but a setting that applies only to statement-level timeouts, not connection-level timeouts.

brettwooldridge avatar Sep 12 '22 19:09 brettwooldridge

A setting for evict-on-timeout is only a workaround that does not solve the underlying problem. The real problem is that on eviction, a connection is only closed but not rolled back (which it should according to the JDBC close method documentation, see comment from @boonelschenbroich on Sep 6). Evict in case of other exceptions would still commit with Oracle (if the connection still communicates with the DB).

From my point of view, the missing rollback() call is a bug in Hikari, a setting for evict-on-timeout would be a new (nice to have) feature - more or less independent of the bugfix.

MartinGantenbein avatar Dec 12 '22 09:12 MartinGantenbein

If someone is able to create a pull request with a fix for this as well as sufficient junit test coverage, I will be happy to review and merge it.

lfbayer avatar Sep 20 '23 13:09 lfbayer