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

JdbcTemplate.queryForStream() does not return connection to the pool, causing the connection leak. #27988

Closed
arciszewski opened this issue Jan 30, 2022 · 8 comments
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: invalid An issue that we don't feel is valid

Comments

@arciszewski
Copy link

Affects: <Spring JDBC version 5.3.15>

Please refer to the demo project that replicates the issue.
When using JdbcTemplate.queryForObject() and subsequently calling the .findFirst() method, the connection is not getting closed, hense the connection pool runs dry very quickly.

jdbcTemplate.queryForStream("SELECT VALUE FROM TEST WHERE KEY = ?",
                (rs, i) -> rs.getString("VALUE"), key)
                .findFirst();

The result of the execution of the following code is:

2022-01-30 13:00:38.783 DEBUG 19628 --- [l-1 housekeeper] com.zaxxer.hikari.pool.HikariPool        : HikariPool-1 - Pool stats (total=10, active=10, idle=0, waiting=1)
2022-01-30 13:00:38.783 DEBUG 19628 --- [l-1 housekeeper] com.zaxxer.hikari.pool.HikariPool        : HikariPool-1 - Fill pool skipped, pool is at sufficient level.
2022-01-30 13:00:39.585 DEBUG 19628 --- [           main] com.zaxxer.hikari.pool.HikariPool        : HikariPool-1 - Timeout failure stats (total=10, active=10, idle=0, waiting=0)
2022-01-30 13:00:39.585  INFO 19628 --- [           main] ConditionEvaluationReportLoggingListener : 

Error starting ApplicationContext. To display the conditions report re-run your application with 'debug' enabled.
2022-01-30 13:00:39.616 ERROR 19628 --- [           main] o.s.boot.SpringApplication               : Application run failed

java.lang.IllegalStateException: Failed to execute CommandLineRunner
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:772) ~[spring-boot-2.6.3.jar:2.6.3]
	at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:753) ~[spring-boot-2.6.3.jar:2.6.3]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:309) ~[spring-boot-2.6.3.jar:2.6.3]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1303) ~[spring-boot-2.6.3.jar:2.6.3]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1292) ~[spring-boot-2.6.3.jar:2.6.3]
	at demo.Demo.main(Demo.java:21) ~[main/:na]
Caused by: org.springframework.jdbc.CannotGetJdbcConnectionException: Failed to obtain JDBC Connection; nested exception is java.sql.SQLTransientConnectionException: HikariPool-1 - Connection is not available, request timed out after 30018ms.
	at org.springframework.jdbc.datasource.DataSourceUtils.getConnection(DataSourceUtils.java:83) ~[spring-jdbc-5.3.15.jar:5.3.15]
	at org.springframework.jdbc.core.JdbcTemplate.execute(JdbcTemplate.java:646) ~[spring-jdbc-5.3.15.jar:5.3.15]
	at org.springframework.jdbc.core.JdbcTemplate.queryForStream(JdbcTemplate.java:834) ~[spring-jdbc-5.3.15.jar:5.3.15]
	at org.springframework.jdbc.core.JdbcTemplate.queryForStream(JdbcTemplate.java:863) ~[spring-jdbc-5.3.15.jar:5.3.15]
	at demo.JdbcRepo.getFromStream(JdbcRepo.java:32) ~[main/:na]
	at demo.JdbcRepo$$FastClassBySpringCGLIB$$87a3d4d8.invoke(<generated>) ~[main/:na]
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218) ~[spring-core-5.3.15.jar:5.3.15]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:783) ~[spring-aop-5.3.15.jar:5.3.15]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163) ~[spring-aop-5.3.15.jar:5.3.15]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:753) ~[spring-aop-5.3.15.jar:5.3.15]
	at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:137) ~[spring-tx-5.3.15.jar:5.3.15]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186) ~[spring-aop-5.3.15.jar:5.3.15]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:753) ~[spring-aop-5.3.15.jar:5.3.15]
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:698) ~[spring-aop-5.3.15.jar:5.3.15]
	at demo.JdbcRepo$$EnhancerBySpringCGLIB$$c0949ac4.getFromStream(<generated>) ~[main/:na]
	at demo.Demo.run(Demo.java:49) ~[main/:na]
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:769) ~[spring-boot-2.6.3.jar:2.6.3]
	... 5 common frames omitted
Caused by: java.sql.SQLTransientConnectionException: HikariPool-1 - Connection is not available, request timed out after 30018ms.
	at com.zaxxer.hikari.pool.HikariPool.createTimeoutException(HikariPool.java:696) ~[HikariCP-4.0.3.jar:na]
	at com.zaxxer.hikari.pool.HikariPool.getConnection(HikariPool.java:197) ~[HikariCP-4.0.3.jar:na]
	at com.zaxxer.hikari.pool.HikariPool.getConnection(HikariPool.java:162) ~[HikariCP-4.0.3.jar:na]
	at com.zaxxer.hikari.HikariDataSource.getConnection(HikariDataSource.java:128) ~[HikariCP-4.0.3.jar:na]
	at org.springframework.jdbc.datasource.DataSourceUtils.fetchConnection(DataSourceUtils.java:159) ~[spring-jdbc-5.3.15.jar:5.3.15]
	at org.springframework.jdbc.datasource.DataSourceUtils.doGetConnection(DataSourceUtils.java:117) ~[spring-jdbc-5.3.15.jar:5.3.15]
	at org.springframework.jdbc.datasource.DataSourceUtils.getConnection(DataSourceUtils.java:80) ~[spring-jdbc-5.3.15.jar:5.3.15]
	... 21 common frames omitted
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 30, 2022
@sbrannen sbrannen added in: data Issues in data modules (jdbc, orm, oxm, tx) status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 30, 2022
@sbrannen
Copy link
Member

As stated in the Javadoc for org.springframework.jdbc.core.JdbcOperations.queryForStream(String, RowMapper<T>, Object...), the caller is responsible for closing the stream.

Returns:
the result Stream, containing mapped objects, needing to be closed once fully processed (e.g. through a try-with-resources clause)

As you can see in the following code listing, the Stream returned has an onClose() callback associated with it that will close the connection.

return new ResultSetSpliterator<>(rs, rowMapper).stream().onClose(() -> {
JdbcUtils.closeResultSet(rs);
if (pss instanceof ParameterDisposer) {
((ParameterDisposer) pss).cleanupParameters();
}
JdbcUtils.closeStatement(ps);
DataSourceUtils.releaseConnection(con, getDataSource());
});

In light of that, I am closing this issue as "works as designed".


As a side note, if you are retrieving a single value from the database, have you considered using queryForObject(...) with a limit 1 clause in your SQL statement (or whatever syntax is supported by your database to return a single result)?

In other words, why are you using a stream for a single result?

@arciszewski
Copy link
Author

Thanks for explanation. I thought that the stream terminal methods also close the internal connection. When I modified my example code to use try-with-resources it worked as expected.
I tried to use the queryForStream(...) and findFirst() because in my particular case I know there could be either one or none results, hense I wanted to use Optional as a return type. And Stream > Optional seemed a reasonable choice.

@jeanxu1
Copy link

jeanxu1 commented Feb 26, 2022

I am seeing the same issue. it is not that intuitive and I thought it is a bug for connection leak as well.

What is the use case for queryForStream() then? can you provide example of the right way of using it?

@sbrannen

@ryber
Copy link

ryber commented Feb 28, 2023

I agree with other posters that this violates the principle of least surprise, I would never expect that I would have to close the connection because I don't do that with any other of the methods in that class.

@lcy2317
Copy link

lcy2317 commented Dec 22, 2023

Adding Transaction annotation also helps, but I still think queryForStream() should be avoided using, using jdbctemplate.query() instead.

@sbrannen
Copy link
Member

I tried to use the queryForStream(...) and findFirst() because in my particular case I know there could be either one or none results, hense I wanted to use Optional as a return type. And Stream > Optional seemed a reasonable choice.

If you insist on having Optional semantics when using JdbcTemplate, see all comments in #30927 for details.

Note, however, that as of Spring Framework 6.1 the recommend way to have Optional semantics is via the fluent API in the new JdbcClient. For a concrete example, see #30931 (comment).

@sbrannen
Copy link
Member

What is the use case for queryForStream() then? can you provide example of the right way of using it?

I don't believe we have an actual example in the documentation; however, you can see an example in our test suite:

String sql = "SELECT AGE FROM CUSTMR WHERE ID = ?";
given(this.resultSet.next()).willReturn(true, false);
given(this.resultSet.getInt(1)).willReturn(22);
AtomicInteger count = new AtomicInteger();
try (Stream<Integer> s = this.template.queryForStream(sql, (rs, rowNum) -> rs.getInt(1), 3)) {
s.forEach(val -> {
count.incrementAndGet();
assertThat(val).isEqualTo(22);
});
}

Feel free to open a new ticket to suggest that an example be included in the reference manual.

@sbrannen
Copy link
Member

I agree with other posters that this violates the principle of least surprise, I would never expect that I would have to close the connection because I don't do that with any other of the methods in that class.

Although it may be surprising, it is sometimes the case that one must close a Stream in Java.

For example, the Stream returned from java.nio.file.Files.walk(Path, int, FileVisitOption...) must also be manually closed (or used in try-with-resources block).

queryForStream(...) is also one of those cases, and it's documented as such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

6 participants