Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLTimeoutException closes connection #1388

Open
nytro77 opened this issue Jun 8, 2019 · 17 comments
Open

SQLTimeoutException closes connection #1388

nytro77 opened this issue Jun 8, 2019 · 17 comments

Comments

@nytro77
Copy link

nytro77 commented Jun 8, 2019

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 #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
}

@johnou
Copy link
Contributor

johnou commented Sep 18, 2019

cc @brettwooldridge

@eicki
Copy link

eicki commented Feb 3, 2020

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

@eicki
Copy link

eicki commented Feb 3, 2020

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

@johnou
Copy link
Contributor

johnou commented Feb 3, 2020

@eicki see #1489 (comment)

@aritzbastida
Copy link

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".

@talk2debendra
Copy link

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)

@idkw
Copy link

idkw commented Oct 13, 2020

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
Copy link

idkw commented Dec 23, 2020

Hi @johnou
Do you consider a fix for this bug ?
I think this commit should be rollbacked because it introduced the regression : cda2605

@tonyldo
Copy link

tonyldo commented Feb 28, 2021

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

@kzander91
Copy link

kzander91 commented Dec 13, 2021

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.

@idkw
Copy link

idkw commented Dec 13, 2021

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 cda2605

Thank you

@petarov
Copy link

petarov commented Feb 22, 2022

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.

@boonelschenbroich
Copy link

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."

@brettwooldridge
Copy link
Owner

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

@MartinGantenbein
Copy link

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.

@lfbayer
Copy link
Collaborator

lfbayer commented Sep 20, 2023

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 lfbayer added the bug label Sep 20, 2023
@chuckbenger
Copy link

chuckbenger commented May 7, 2024

Hi Folks! We have faced this exact same issue (partial commit) involving a Spring Boot application that interacts with Oracle. We are going to try the workaround above, but it would be great if we can have this behaviour configurable so that a rollback is always done if a connection is closed by Hikari due to timeouts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests