Release connection after transaction if using Hibernate and open-in-view is on (turned off by default) #34138
Conversation
Even though those two actions trigger under the same condition, they don't have to be executed in one uninterrupted block of code. Connection only can be released if connection holder was successfully removed, but that's all
Releasing connection before transaction cleanup may break connection reset. It certainly does in case of Hibernate, because HibernateJpaDialect needs active connection to reset isolation level and read only flag. New unit test simply makes sure cleanup and connection release happen in expected order.
In case of pre-bound entity manager it makes sense to release connection after transaction (in some cases at least). But if entity manager is not pre-bound, then it is going to be closed immediately after transaction cleanup. And that releases the connection. So there's no point in releasing the connection manually, if it's going to be released next moment anyway.
…f open-in-view This commit introduces a new setting - releaseConnectionAfterTransaction, which controls if HibernateJpaDialect#releaseJdbcConnection will release connections when called. ReleaseJdbcConnection method is only called if session is pre-bound, so the new setting actually doesn't do anything unless open-in-view is on. And in case if open-in-view is on, turning releaseConnectionAfterTransaction on might help with connection pool starvation.
…Adapter Users don't create HibernateJpaDialect themselves, HibernateJpaVendorAdapter does that. So, this change is needed so that releaseConnectionAfterTransaction could be configured via application settings.
spring-projects-issues
added
the
status: waiting-for-triage
| * 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 |
There was a problem hiding this comment.
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 {
@linkEntityManager} is pre-bound
| * 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 |
There was a problem hiding this comment.
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 {
@linkorg.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 |
There was a problem hiding this comment.
Am I correct that here you're talking about this specific property that allows loading associations when the original transaction is already commited? Just to clarify.
| * <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 |
There was a problem hiding this comment.
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.
| * complete. Hibernate will not acquire and release connections for each | ||
| * lazy field loading. | ||
| */ | ||
| public void setReleaseConnectionAfterTransaction(boolean releaseConnectionAfterTransaction) { |
There was a problem hiding this comment.
I think we can just link the existing releaseConnectionAfterTransaction documentation via javadoc tag @link
One of the issues with open-in-view is that connections are not released for a long time, which can lead to connection pool starvation.
Sometimes, controller method even makes no queries for seconds and doesn't really need to keep the connection for itself all that time. For example that is often the case, when application makes REST call after making database query.
This pull request gives user setting, which tells transaction manager, that it should release JDBC connection after transaction is finished (successfully or not). Connection release only happens if the setting is turned on and if open-in-view is enabled. If open-in-view is disabled, then entity manager is closed after transaction is finished and connection is released anyway.
What this pull request changes in existing behaviour
because if connection is released, transaction manager can't reset isolation level and read-only flag. I think the only reason nobody noticed that before is that nobody tried to override JpaDialect#releaseJdbcConnection for HibernateJpaDialect
What this pull request adds
P. S. I think those changes are ok, because the behaviour is copied from HibernateTransactionManager. Hibernate 6 is officially only supported through JpaTransactionManager, so it would be nice to port this HibernateTransactionManager feature.