Skip to content

Commit

Permalink
Avoid registration of an extra transaction synchronization for Hibern…
Browse files Browse the repository at this point in the history
…ate ORM

Hibernate ORM can already handle everything we were doing through that
extra synchronization.

Note that without this patch:

1. Hibernate ORM flushes before transaction completion.
2. Then Hibernate ORM closes the connection wrapper.
3. Then Quarkus flushes again in its own synchronization, which
   usually does nothing, but in some cases (when the dirtiness of some
   entities cannot be known, for example when there are mutable
   @converted properties) it will execute the updates again...
   This is problematic in itself, but it will also lead to acquiring a
   new connection wrapper from Agroal, which will not be closed.
4. Then when the commit happens, Agroal notices a connection wrapper was
   not closed and logs a warning.

Signed-off-by: Yoann Rodière <yoann@hibernate.org>
  • Loading branch information
yrodiere committed Feb 5, 2021
1 parent 5f5dc72 commit efc2b8e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ private MergedSettings mergeSettings(PersistenceUnitDescriptor persistenceUnit)
cfg.putIfAbsent(AvailableSettings.CONNECTION_HANDLING,
PhysicalConnectionHandlingMode.DELAYED_ACQUISITION_AND_RELEASE_BEFORE_TRANSACTION_COMPLETION);

// Auto-close sessions before transaction completion, as they should be when using JTA.
cfg.putIfAbsent(AvailableSettings.AUTO_CLOSE_SESSION, "true");

if (readBooleanConfigurationValue(cfg, WRAP_RESULT_SETS)) {
LOG.warn("Wrapping result sets is not supported. Setting " + WRAP_RESULT_SETS + " to false.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import javax.persistence.criteria.CriteriaUpdate;
import javax.persistence.metamodel.Metamodel;
import javax.transaction.Status;
import javax.transaction.Synchronization;
import javax.transaction.TransactionManager;
import javax.transaction.TransactionSynchronizationRegistry;

Expand Down Expand Up @@ -86,17 +85,13 @@ SessionResult acquireSession() {
Session newSession = sessionFactory.openSession();
newSession.joinTransaction();
transactionSynchronizationRegistry.putResource(sessionKey, newSession);
transactionSynchronizationRegistry.registerInterposedSynchronization(new Synchronization() {
@Override
public void beforeCompletion() {
newSession.flush();
}

@Override
public void afterCompletion(int i) {
newSession.close();
}
});
// No need to flush or close the session upon transaction completion:
// Hibernate ORM itself registers a transaction that does just that.
// See:
// - io.quarkus.hibernate.orm.runtime.boot.FastBootMetadataBuilder.mergeSettings
// - org.hibernate.resource.transaction.backend.jta.internal.JtaTransactionCoordinatorImpl.joinJtaTransaction
// - org.hibernate.internal.SessionImpl.beforeTransactionCompletion
// - org.hibernate.internal.SessionImpl.afterTransactionCompletion
return new SessionResult(newSession, false, true);
} else {
//this will throw an exception if the request scope is not active
Expand Down

0 comments on commit efc2b8e

Please sign in to comment.