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

Release connection after transaction if using Hibernate and open-in-view is on (turned off by default) #34138

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -623,17 +623,6 @@ protected void doCleanupAfterCompletion(Object transaction) {
// Remove the JDBC connection holder from the thread, if exposed.
if (getDataSource() != null && txObject.hasConnectionHolder()) {
TransactionSynchronizationManager.unbindResource(getDataSource());
ConnectionHandle conHandle = txObject.getConnectionHolder().getConnectionHandle();
if (conHandle != null) {
try {
getJpaDialect().releaseJdbcConnection(conHandle,
txObject.getEntityManagerHolder().getEntityManager());
}
catch (Throwable ex) {
// Just log it, to keep a transaction-related exception.
logger.error("Failed to release JDBC connection after transaction", ex);
}
}
}

getJpaDialect().cleanupTransaction(txObject.getTransactionData());
Expand All @@ -648,6 +637,20 @@ protected void doCleanupAfterCompletion(Object transaction) {
}
else {
logger.debug("Not closing pre-bound JPA EntityManager after transaction");
// Give JpaDialect it's chance to release JDBC connection
if (getDataSource() != null && txObject.hasConnectionHolder()) {
ConnectionHandle conHandle = txObject.getConnectionHolder().getConnectionHandle();
if (conHandle != null) {
try {
getJpaDialect().releaseJdbcConnection(conHandle,
txObject.getEntityManagerHolder().getEntityManager());
}
catch (Throwable ex) {
// Just log it, to keep a transaction-related exception.
logger.error("Failed to release JDBC connection after transaction", ex);
}
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.hibernate.exception.JDBCConnectionException;
import org.hibernate.exception.LockAcquisitionException;
import org.hibernate.exception.SQLGrammarException;
import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor;
import org.jspecify.annotations.Nullable;

import org.springframework.dao.CannotAcquireLockException;
Expand Down Expand Up @@ -89,6 +90,8 @@ public class HibernateJpaDialect extends DefaultJpaDialect {

boolean prepareConnection = true;

boolean releaseConnectionAfterTransaction = false;

private @Nullable SQLExceptionTranslator jdbcExceptionTranslator;

private @Nullable SQLExceptionTranslator transactionExceptionTranslator = new SQLExceptionSubclassTranslator();
Expand Down Expand Up @@ -118,6 +121,34 @@ public void setPrepareConnection(boolean prepareConnection) {
this.prepareConnection = prepareConnection;
}

/**
* Set, whether to release connection after transaction is commited or rolled
* back. Only works for pre-bound sessions.
* <p> This setting is needed to work around the fact, that it is not possible
* to use {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode#DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION}
* handling mode without sacrificing setting non-default isolation levels
* or read-only flag. Releasing connection might prevent connection pool
* starvation if open-in-view is on.
* <p> Default is "false" for backward compatibility. If you turn this flag
Copy link

@mipo256 mipo256 Jan 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this

If you turn this flag on it still does nothing unless session is not pre-bound (most likely if open-in-view) is off

can be simplified to:

This flag only takes effect in case the underlying {@link EntityManager} is pre-bound

* on it still does nothing unless session is pre-bound (most likely if
* open-in-view) is off. If session is pre-bound and the flag is on, then
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it is perfectly sufficient to say the following:

If session is pre-bound and the flag is on, then after transaction is finished (successfully or not), underlying JDBC connection will be released and acquired later according to {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode}.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but PhysicalConnectionHandlingMode is not as simple as it seems.

One might suppose, that if you don't configure it manually, then Hibernate will use default value. Which is DELAYED_ACQUISITION_AND_RELEASE_AFTER_STATEMENT. But Spring overrides PhysicalConnectionHandlingMode to be DELAYED_ACQUISITION_AND_HOLD, because DELAYED_ACQUISITION_AND_RELEASE_AFTER_STATEMENT breaks read only transactions and custom isolation levels.

So I thought it would be better to explicitly specify Spring default for PhysicalConnectionHandlingMode . If you still think it is sufficient to say that JDBC connection is managed according to PhysicalConnectionHandlingMode, I don't mind, just wanted to give you the whole picture

* after transaction is finished (successfully or not), underlying JDBC
* connection will be released and acquired later according to {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode}
* (is set to {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode#DELAYED_ACQUISITION_AND_HOLD}
* with {@link HibernateJpaVendorAdapter#getJpaPropertyMap()} if you don't
* specify it yourself).
* <p> Please pay attention, that this setting doesn't affect how Hibernate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I am talking about loading LAZY fields. I wanted to say, that connection will not be released after loading LAZY field, even if releaseConnectionAfterTransaction is on

* handles connections, which were acquired on-demand, to lazily load
* collections outside of transaction context.
* <p> Specifically, connections, acquired to serialize entities, returned
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand this sentence and its purpose. What connections "acquired to serialize entities" are you talking about here?

Specifically, connections, acquired to serialize entities, returned by rest controller method will only be closed, after serialization is complete. Hibernate will not acquire and release connections for each lazy field loading.

Copy link
Author

@poxu poxu Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if controller method returns Entity , it is then serialized to JSON with Jackson. To generate JSON, sometimes we need to load LAZY fields. And to load them Hibernate needs to acquire a connection. And that is what I am talking about. That connection will only be released after EntityManager is closed

* by rest controller method will only be closed, after serialization is
* complete. Hibernate will not acquire and release connections for each
* lazy field loading.
*/
public void setReleaseConnectionAfterTransaction(boolean releaseConnectionAfterTransaction) {
this.releaseConnectionAfterTransaction = releaseConnectionAfterTransaction;
}

/**
* Set the JDBC exception translator for Hibernate exception translation purposes.
* <p>Applied to any detected {@link java.sql.SQLException} root cause of a Hibernate
Expand Down Expand Up @@ -231,6 +262,16 @@ public ConnectionHandle getJdbcConnection(EntityManager entityManager, boolean r
return new HibernateConnectionHandle(session);
}

@Override
public void releaseJdbcConnection(ConnectionHandle conHandle, EntityManager em) throws PersistenceException, SQLException {
if (releaseConnectionAfterTransaction) {
final LogicalConnectionImplementor logicalConnection = getSession(em).getJdbcCoordinator().getLogicalConnection();
if (logicalConnection.isPhysicallyConnected()) {
logicalConnection.manualDisconnect();
}
}
}

@Override
public @Nullable DataAccessException translateExceptionIfPossible(RuntimeException ex) {
if (ex instanceof HibernateException hibernateEx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,34 @@ public void setPrepareConnection(boolean prepareConnection) {
this.jpaDialect.setPrepareConnection(prepareConnection);
}

/**
* Set, whether to release connection after transaction is commited or rolled
* back. Only works for pre-bound sessions.
* <p> This setting is needed to work around the fact, that it is not possible
* to use {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode#DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION}
* handling mode without sacrificing setting non-default isolation levels
* or read-only flag. Releasing connection might prevent connection pool
* starvation if open-in-view is on.
* <p> Default is "false" for backward compatibility. If you turn this flag
* on it still does nothing unless session is pre-bound (most likely if
* open-in-view) is off. If session is pre-bound and the flag is on, then
* after transaction is finished (successfully or not), underlying JDBC
* connection will be released and acquired later according to {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode}
* (is set to {@link org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode#DELAYED_ACQUISITION_AND_HOLD}
* with {@link HibernateJpaVendorAdapter#getJpaPropertyMap()} if you don't
* specify it yourself).
* <p> Please pay attention, that this setting doesn't affect how Hibernate
* handles connections, which were acquired on-demand, to lazily load
* collections outside of transaction context.
* <p> Specifically, connections, acquired to serialize entities, returned
* by rest controller method will only be closed, after serialization is
* complete. Hibernate will not acquire and release connections for each
* lazy field loading.
*/
public void setReleaseConnectionAfterTransaction(boolean releaseConnectionAfterTransaction) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just link the existing releaseConnectionAfterTransaction documentation via javadoc tag @link

this.jpaDialect.setReleaseConnectionAfterTransaction(releaseConnectionAfterTransaction);
}


@Override
public PersistenceProvider getPersistenceProvider() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/*
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.orm.jpa;

import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.EntityTransaction;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.InOrder;
import org.springframework.jdbc.datasource.ConnectionHandle;
import org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.transaction.support.TransactionTemplate;

import javax.sql.DataSource;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;

/**
* @author Ilia Sazonov
*/
class JpaTransactionManagerConnectionReleaseTests {

private EntityManagerFactory factory = mock();

private EntityManager manager = mock();

private EntityTransaction tx = mock();

private JpaTransactionManager tm = new JpaTransactionManager(factory);

private TransactionTemplate tt = new TransactionTemplate(tm);

private DataSource ds = mock();

private ConnectionHandle connHandle = mock();

private JpaDialect jpaDialect = spy(new DefaultJpaDialect());


@BeforeEach
void setup() throws SQLException {
given(factory.createEntityManager()).willReturn(manager);
given(manager.getTransaction()).willReturn(tx);
given(manager.isOpen()).willReturn(true);
given(jpaDialect.getJdbcConnection(same(manager), anyBoolean())).willReturn(connHandle);
tm.setJpaDialect(jpaDialect);
tm.setDataSource(ds);
}

@AfterEach
void verifyTransactionSynchronizationManagerState() {
assertThat(TransactionSynchronizationManager.getResourceMap()).isEmpty();
assertThat(TransactionSynchronizationManager.isSynchronizationActive()).isFalse();
assertThat(TransactionSynchronizationManager.isCurrentTransactionReadOnly()).isFalse();
assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isFalse();
}

@Test
void testConnectionIsReleasedAfterTransactionCleanup() throws SQLException {
given(manager.getTransaction()).willReturn(tx);

final List<String> l = new ArrayList<>();
l.add("test");

assertThat(TransactionSynchronizationManager.hasResource(factory)).isFalse();
assertThat(TransactionSynchronizationManager.isSynchronizationActive()).isFalse();
TransactionSynchronizationManager.bindResource(factory, new EntityManagerHolder(manager));

try {
Object result = tt.execute(status -> {
assertThat(TransactionSynchronizationManager.hasResource(factory)).isTrue();
assertThat(TransactionSynchronizationManager.isSynchronizationActive()).isTrue();
EntityManagerFactoryUtils.getTransactionalEntityManager(factory);
return l;
});
assertThat(result).isSameAs(l);

assertThat(TransactionSynchronizationManager.hasResource(factory)).isTrue();
assertThat(TransactionSynchronizationManager.isSynchronizationActive()).isFalse();
}
finally {
TransactionSynchronizationManager.unbindResource(factory);
}

verify(tx).begin();
verify(tx).commit();

InOrder cleanupBeforeRelease = inOrder(jpaDialect);
cleanupBeforeRelease.verify(jpaDialect).cleanupTransaction(any());
cleanupBeforeRelease.verify(jpaDialect).releaseJdbcConnection(same(connHandle), same(manager));
}

@Test
void testConnectionIsNotReleasedIfSesionIsClosing() throws SQLException {
given(manager.getTransaction()).willReturn(tx);

final List<String> l = new ArrayList<>();
l.add("test");

assertThat(TransactionSynchronizationManager.hasResource(factory)).isFalse();
assertThat(TransactionSynchronizationManager.isSynchronizationActive()).isFalse();

Object result = tt.execute(status -> {
assertThat(TransactionSynchronizationManager.hasResource(factory)).isTrue();
EntityManagerFactoryUtils.getTransactionalEntityManager(factory).flush();
return l;
});
assertThat(result).isSameAs(l);

assertThat(TransactionSynchronizationManager.hasResource(factory)).isFalse();
assertThat(TransactionSynchronizationManager.isSynchronizationActive()).isFalse();

verify(tx).begin();
verify(tx).commit();

verify(jpaDialect).cleanupTransaction(any());
verify(jpaDialect, never()).releaseJdbcConnection(any(), any());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.orm.jpa.hibernate;

import jakarta.persistence.EntityManager;
import org.hibernate.engine.spi.SessionImplementor;
import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor;
import org.junit.jupiter.api.Test;
import org.springframework.orm.jpa.AbstractContainerEntityManagerFactoryIntegrationTests;
import org.springframework.orm.jpa.EntityManagerHolder;
import org.springframework.transaction.support.TransactionSynchronizationManager;

import static org.assertj.core.api.Assertions.assertThat;

/**
* Tests for {@link org.springframework.orm.jpa.vendor.HibernateJpaDialect#releaseConnectionAfterTransaction}
*
* @author Ilia Sazonov
*/
class HibernateEntityManagerFactoryReleaseConnectionAfterTransactionIntegrationTests extends AbstractContainerEntityManagerFactoryIntegrationTests {

@Override
protected String[] getConfigLocations() {
return new String[] {"/org/springframework/orm/jpa/hibernate/hibernate-manager-release-after-transaction.xml",
"/org/springframework/orm/jpa/memdb.xml", "/org/springframework/orm/jpa/inject.xml"};
}

@Test
public void testReleaseConnectionAfterTransaction() {
endTransaction();

try (EntityManager em = entityManagerFactory.createEntityManager()) {
EntityManagerHolder emHolder = new EntityManagerHolder(em);
TransactionSynchronizationManager.bindResource(entityManagerFactory, emHolder);

startNewTransaction();
endTransaction();

assertThat(em.isOpen()).isTrue();
final LogicalConnectionImplementor logicalConnection = em.unwrap(SessionImplementor.class)
.getJdbcCoordinator().getLogicalConnection();
assertThat(logicalConnection.isPhysicallyConnected()).isFalse();

} finally {
TransactionSynchronizationManager.unbindResource(entityManagerFactory);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans-2.5.xsd">

<bean id="entityManagerFactory" class="org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean" primary="true">
<property name="persistenceXmlLocation" value="org/springframework/orm/jpa/domain/persistence-context.xml"/>
<property name="dataSource" ref="dataSource"/>
<property name="jpaVendorAdapter">
<bean class="org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter">
<property name="database" value="HSQL"/>
<property name="showSql" value="true"/>
<property name="generateDdl" value="true"/>
<property name="releaseConnectionAfterTransaction" value="true"/>
</bean>
</property>
<property name="jpaPropertyMap">
<props>
<prop key="hibernate.current_session_context_class">org.springframework.orm.hibernate5.SpringSessionContext</prop>
<prop key="hibernate.cache.provider_class">org.hibernate.cache.HashtableCacheProvider</prop>
</props>
</property>
<property name="bootstrapExecutor">
<bean class="org.springframework.core.task.SimpleAsyncTaskExecutor"/>
</property>
</bean>

<bean id="transactionManager" class="org.springframework.orm.jpa.JpaTransactionManager">
<property name="entityManagerFactory" ref="entityManagerFactory"/>
</bean>

<bean id="hibernateStatistics" factory-bean="entityManagerFactory" factory-method="getStatistics" lazy-init="true"/>

<bean class="org.springframework.aop.framework.autoproxy.DefaultAdvisorAutoProxyCreator"/>

</beans>