improve exception messages and logging related to tx management

along with some minor aesthetic code cleanups
This commit is contained in:
Gavin King 2024-11-20 10:50:01 +01:00
parent 47f1a1207a
commit 453f0ff074
11 changed files with 161 additions and 196 deletions

View File

@ -69,9 +69,13 @@ public interface Transaction extends EntityTransaction {
/**
* Attempt to mark the underlying transaction for rollback only.
* <p>
* Unlike {@link #setRollbackOnly()}, which is specified by JPA
* to throw when the transaction is inactive, this operation may
* be called on an inactive transaction, in which case it has no
* effect.
*
* @see #setRollbackOnly()
*/
default void markRollbackOnly() {
setRollbackOnly();
}
void markRollbackOnly();
}

View File

@ -12,7 +12,6 @@ import org.hibernate.TransactionException;
import org.hibernate.engine.transaction.spi.TransactionImplementor;
import org.hibernate.internal.AbstractSharedSessionContract;
import org.hibernate.internal.CoreLogging;
import org.hibernate.jpa.spi.JpaCompliance;
import org.hibernate.resource.transaction.spi.TransactionCoordinator;
import org.hibernate.resource.transaction.spi.TransactionStatus;
@ -28,7 +27,7 @@ public class TransactionImpl implements TransactionImplementor {
private static final Logger LOG = CoreLogging.logger( TransactionImpl.class );
private final TransactionCoordinator transactionCoordinator;
private final JpaCompliance jpaCompliance;
private final boolean jpaCompliance;
private final AbstractSharedSessionContract session;
private TransactionDriver transactionDriverControl;
@ -37,7 +36,9 @@ public class TransactionImpl implements TransactionImplementor {
TransactionCoordinator transactionCoordinator,
AbstractSharedSessionContract session) {
this.transactionCoordinator = transactionCoordinator;
this.jpaCompliance = session.getFactory().getSessionFactoryOptions().getJpaCompliance();
this.jpaCompliance =
session.getFactory().getSessionFactoryOptions().getJpaCompliance()
.isJpaTransactionComplianceEnabled();
this.session = session;
if ( session.isOpen() && transactionCoordinator.isActive() ) {
@ -47,11 +48,8 @@ public class TransactionImpl implements TransactionImplementor {
LOG.debug( "TransactionImpl created on closed Session/EntityManager" );
}
if ( LOG.isDebugEnabled() ) {
LOG.debugf(
"On TransactionImpl creation, JpaCompliance#isJpaTransactionComplianceEnabled == %s",
jpaCompliance.isJpaTransactionComplianceEnabled()
);
if ( LOG.isDebugEnabled() && jpaCompliance ) {
LOG.debugf( "TransactionImpl created in JPA compliant mode" );
}
}
@ -65,36 +63,31 @@ public class TransactionImpl implements TransactionImplementor {
transactionDriverControl = transactionCoordinator.getTransactionDriverControl();
}
// per-JPA
if ( isActive() ) {
if ( jpaCompliance.isJpaTransactionComplianceEnabled()
|| !transactionCoordinator.getTransactionCoordinatorBuilder().isJta() ) {
throw new IllegalStateException( "Transaction already active" );
if ( jpaCompliance ) {
throw new IllegalStateException( "Transaction already active (in JPA compliant mode)" );
}
else if ( !transactionCoordinator.getTransactionCoordinatorBuilder().isJta() ) {
throw new IllegalStateException( "Resource-local transaction already active" );
}
}
else {
return;
LOG.debug( "begin transaction" );
transactionDriverControl.begin();
}
}
LOG.debug( "begin" );
this.transactionDriverControl.begin();
}
@Override
public void commit() {
if ( !isActive( true ) ) {
// allow MARKED_ROLLBACK to propagate through to transactionDriverControl
// the boolean passed to isActive indicates whether MARKED_ROLLBACK should be
// considered active
//
// essentially here we have a transaction that is not active and
// has not been marked for rollback only
// the boolean passed to isActive indicates whether MARKED_ROLLBACK should
// be considered active
if ( !isActive( true ) ) {
// we have a transaction that is inactive and has not been marked for rollback only
throw new IllegalStateException( "Transaction not successfully started" );
}
LOG.debug( "committing" );
else {
LOG.debug( "committing transaction" );
try {
internalGetTransactionDriverControl().commit();
}
@ -102,41 +95,34 @@ public class TransactionImpl implements TransactionImplementor {
throw session.getExceptionConverter().convertCommitException( e );
}
}
}
public TransactionDriver internalGetTransactionDriverControl() {
// NOTE here to help be a more descriptive NullPointerException
if ( this.transactionDriverControl == null ) {
if ( transactionDriverControl == null ) {
throw new IllegalStateException( "Transaction was not properly begun/started" );
}
return this.transactionDriverControl;
else {
return transactionDriverControl;
}
}
@Override
public void rollback() {
if ( !isActive() ) {
if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) {
throw new IllegalStateException(
"JPA compliance dictates throwing IllegalStateException when #rollback " +
"is called on non-active transaction"
);
}
if ( !isActive() && jpaCompliance ) {
throw new IllegalStateException( "rollback() called on inactive transaction (in JPA compliant mode)" );
}
TransactionStatus status = getStatus();
final TransactionStatus status = getStatus();
if ( status == TransactionStatus.ROLLED_BACK || status == TransactionStatus.NOT_ACTIVE ) {
// Allow rollback() calls on completed transactions, just no-op.
// allow rollback() on completed transaction as noop
LOG.debug( "rollback() called on an inactive transaction" );
return;
}
if ( !status.canRollback() ) {
throw new TransactionException( "Cannot rollback transaction in current status [" + status.name() + "]" );
else if ( !status.canRollback() ) {
throw new TransactionException( "Cannot roll back transaction in current status [" + status.name() + "]" );
}
LOG.debug( "rolling back" );
if ( status != TransactionStatus.FAILED_COMMIT || allowFailedCommitToPhysicallyRollback() ) {
else if ( status != TransactionStatus.FAILED_COMMIT || allowFailedCommitToPhysicallyRollback() ) {
LOG.debug( "rolling back transaction" );
internalGetTransactionDriverControl().rollback();
}
}
@ -176,55 +162,50 @@ public class TransactionImpl implements TransactionImplementor {
@Override
public void registerSynchronization(Synchronization synchronization) throws HibernateException {
this.transactionCoordinator.getLocalSynchronizations().registerSynchronization( synchronization );
transactionCoordinator.getLocalSynchronizations().registerSynchronization( synchronization );
}
@Override
public void setTimeout(int seconds) {
this.transactionCoordinator.setTimeOut( seconds );
transactionCoordinator.setTimeOut( seconds );
}
@Override
public void setTimeout(@Nullable Integer seconds) {
this.transactionCoordinator.setTimeOut( seconds == null ? -1 : seconds );
transactionCoordinator.setTimeOut( seconds == null ? -1 : seconds );
}
@Override
public @Nullable Integer getTimeout() {
final int timeOut = this.transactionCoordinator.getTimeOut();
if ( timeOut == -1 ) {
return null;
}
return timeOut;
final int timeout = transactionCoordinator.getTimeOut();
return timeout == -1 ? null : timeout;
}
@Override
public void markRollbackOnly() {
// this is the Hibernate-specific API, whereas #setRollbackOnly is the
// this is the Hibernate-specific API, whereas setRollbackOnly is the
// JPA-defined API. In our opinion it is much more user-friendly to
// always allow user/integration to indicate that the transaction
// should not be allowed to commit.
//
// However.. should only "do something" on an active transaction
if ( isActive() ) {
internalGetTransactionDriverControl().markRollbackOnly();
}
// else noop for an inactive transaction
}
@Override
public void setRollbackOnly() {
if ( !isActive() ) {
// Since this is the JPA-defined one, we make sure the txn is active first
// so long as compliance (JpaCompliance) has not been defined to disable
// that check - making this active more like Hibernate's #markRollbackOnly
if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) {
throw new IllegalStateException(
"JPA compliance dictates throwing IllegalStateException when #setRollbackOnly " +
"is called on non-active transaction"
);
if ( jpaCompliance ) {
// This is the JPA-defined version of this operation,
// so we must check that the transaction is active
throw new IllegalStateException( "setRollbackOnly() called on inactive transaction (in JPA compliant mode)" );
}
else {
LOG.debug( "#setRollbackOnly called on a not-active transaction" );
// JpaCompliance disables the check, so this method
// is equivalent our native markRollbackOnly()
LOG.debug( "setRollbackOnly() called on a inactive transaction" );
}
}
else {
@ -234,17 +215,13 @@ public class TransactionImpl implements TransactionImplementor {
@Override
public boolean getRollbackOnly() {
if ( !isActive() ) {
if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) {
throw new IllegalStateException(
"JPA compliance dictates throwing IllegalStateException when #getRollbackOnly " +
"is called on non-active transaction"
);
if ( jpaCompliance && !isActive() ) {
throw new IllegalStateException( "getRollbackOnly() called on inactive transaction (in JPA compliant mode)" );
}
}
else {
return getStatus() == TransactionStatus.MARKED_ROLLBACK;
}
}
protected boolean allowFailedCommitToPhysicallyRollback() {
return false;

View File

@ -63,7 +63,7 @@ public class ExceptionConverterImpl implements ExceptionConverter {
catch (Exception re) {
//swallow
}
return new RollbackException( "Error while committing the transaction",
return new RollbackException( "Error while committing the transaction [" + exception.getMessage() + "]",
exception instanceof HibernateException hibernateException
? convert( hibernateException )
: exception );

View File

@ -358,18 +358,16 @@ public final class FastSessionServices {
private static CacheRetrieveMode determineCacheRetrieveMode(Map<String, Object> settings) {
final CacheRetrieveMode cacheRetrieveMode = (CacheRetrieveMode) settings.get( JPA_SHARED_CACHE_RETRIEVE_MODE );
if ( cacheRetrieveMode == null ) {
return (CacheRetrieveMode) settings.get( JAKARTA_SHARED_CACHE_RETRIEVE_MODE );
}
return cacheRetrieveMode;
return cacheRetrieveMode == null
? (CacheRetrieveMode) settings.get( JAKARTA_SHARED_CACHE_RETRIEVE_MODE )
: cacheRetrieveMode;
}
private static CacheStoreMode determineCacheStoreMode(Map<String, Object> settings) {
final CacheStoreMode cacheStoreMode = (CacheStoreMode) settings.get( JPA_SHARED_CACHE_STORE_MODE );
if ( cacheStoreMode == null ) {
return ( CacheStoreMode ) settings.get( JAKARTA_SHARED_CACHE_STORE_MODE );
}
return cacheStoreMode;
return cacheStoreMode == null
? (CacheStoreMode) settings.get( JAKARTA_SHARED_CACHE_STORE_MODE )
: cacheStoreMode;
}
public JdbcValuesMappingProducerProvider getJdbcValuesMappingProducerProvider() {

View File

@ -169,15 +169,15 @@ public class MutableJpaComplianceImpl implements MutableJpaCompliance {
@Override
public JpaCompliance immutableCopy() {
JpaComplianceImpl.JpaComplianceBuilder builder = new JpaComplianceImpl.JpaComplianceBuilder();
builder = builder.setProxyCompliance( proxyCompliance )
return new JpaComplianceImpl.JpaComplianceBuilder()
.setProxyCompliance( proxyCompliance )
.setOrderByMappingCompliance( orderByMappingCompliance )
.setGlobalGeneratorNameCompliance( generatorNameScopeCompliance )
.setQueryCompliance( queryCompliance )
.setTransactionCompliance( transactionCompliance )
.setClosedCompliance( closedCompliance )
.setCachingCompliance( cachingCompliance )
.setLoadByIdCompliance( loadByIdCompliance );
return builder.createJpaCompliance();
.setLoadByIdCompliance( loadByIdCompliance )
.createJpaCompliance();
}
}

View File

@ -148,7 +148,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
transactionCoordinatorOwner.setTransactionTimeOut( timeOut );
}
// report entering into a "transactional context"
transactionCoordinatorOwner.startTransactionBoundary();
@ -184,7 +183,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
log.tracef( "ResourceLocalTransactionCoordinatorImpl#afterCompletionCallback(%s)", successful );
final int statusToSend = successful ? Status.STATUS_COMMITTED : Status.STATUS_UNKNOWN;
synchronizationRegistry.notifySynchronizationsAfterTransactionCompletion( statusToSend );
transactionCoordinatorOwner.afterTransactionCompletion( successful, false );
for ( TransactionObserver observer : observers() ) {
observer.afterCompletion( successful, false );
@ -216,7 +214,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
private boolean rollbackOnly = false;
public TransactionDriverControlImpl(JdbcResourceTransaction jdbcResourceTransaction) {
super();
this.jdbcResourceTransaction = jdbcResourceTransaction;
}
@ -227,7 +224,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
@Override
public void begin() {
errorIfInvalid();
jdbcResourceTransaction.begin();
JdbcResourceLocalTransactionCoordinatorImpl.this.afterBeginCallback();
}
@ -242,31 +238,14 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
public void commit() {
try {
if ( rollbackOnly ) {
log.debug( "On commit, transaction was marked for roll-back only, rolling back" );
try {
rollback();
if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) {
log.debug( "Throwing RollbackException on roll-back of transaction marked rollback-only on commit" );
throw new RollbackException( "Transaction was marked for rollback-only" );
commitRollbackOnly();
}
return;
}
catch (RollbackException e) {
throw e;
}
catch (RuntimeException e) {
log.debug( "Encountered failure rolling back failed commit", e );
throw e;
}
}
else {
JdbcResourceLocalTransactionCoordinatorImpl.this.beforeCompletionCallback();
jdbcResourceTransaction.commit();
JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( true );
}
}
catch (RollbackException e) {
throw e;
}
@ -281,6 +260,23 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
}
}
private void commitRollbackOnly() {
log.debug( "On commit, transaction was marked for roll-back only, rolling back" );
try {
rollback();
if ( jpaCompliance.isJpaTransactionComplianceEnabled() ) {
throw new RollbackException( "Transaction was marked for rollback-only" );
}
}
catch (RollbackException e) {
throw e;
}
catch (RuntimeException e) {
log.debug( "Encountered failure rolling back failed commit", e );
throw e;
}
}
@Override
public void rollback() {
try {
@ -292,8 +288,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
finally {
rollbackOnly = false;
}
// no-op otherwise.
}
@Override
@ -305,12 +299,9 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
public void markRollbackOnly() {
if ( getStatus() != TransactionStatus.ROLLED_BACK ) {
if ( log.isDebugEnabled() ) {
log.debug(
"JDBC transaction marked for rollback-only (exception provided for stack trace)",
new Exception( "exception just for purpose of providing stack trace" )
);
log.debug( "JDBC transaction marked for rollback-only (exception provided for stack trace)",
new Exception( "exception just for purpose of providing stack trace" ) );
}
rollbackOnly = true;
}
}

View File

@ -136,56 +136,47 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
@Override
public void pulse() {
if ( !autoJoinTransactions ) {
return;
}
if ( synchronizationRegistered ) {
return;
}
if ( autoJoinTransactions && !synchronizationRegistered ) {
// Can we register a synchronization according to the JtaPlatform?
if ( !jtaPlatform.canRegisterSynchronization() ) {
log.trace( "JTA platform says we cannot currently register synchronization; skipping" );
return;
}
else {
joinJtaTransaction();
}
}
}
/**
* Join to the JTA transaction. Note that the underlying meaning of joining in JTA environments is to register the
* RegisteredSynchronization with the JTA system
*/
private void joinJtaTransaction() {
if ( synchronizationRegistered ) {
return;
}
jtaPlatform.registerSynchronization( new RegisteredSynchronization( getSynchronizationCallbackCoordinator() ) );
if ( !synchronizationRegistered ) {
jtaPlatform.registerSynchronization(
new RegisteredSynchronization( getSynchronizationCallbackCoordinator() ) );
getSynchronizationCallbackCoordinator().synchronizationRegistered();
synchronizationRegistered = true;
log.debug( "Hibernate RegisteredSynchronization successfully registered with JTA platform" );
// report entering into a "transactional context"
getTransactionCoordinatorOwner().startTransactionBoundary();
}
}
@Override
public void explicitJoin() {
if ( synchronizationRegistered ) {
log.debug( "JTA transaction was already joined (RegisteredSynchronization already registered)" );
return;
}
else {
if ( getTransactionDriverControl().getStatus() != ACTIVE ) {
throw new TransactionRequiredForJoinException(
"Explicitly joining a JTA transaction requires a JTA transaction be currently active"
);
}
joinJtaTransaction();
}
}
@Override
public boolean isJoined() {
@ -204,7 +195,7 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
}
public TransactionCoordinatorOwner getTransactionCoordinatorOwner(){
return this.transactionCoordinatorOwner;
return transactionCoordinatorOwner;
}
@Override
@ -221,39 +212,44 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
}
private TransactionDriverControlImpl makePhysicalTransactionDelegate() {
JtaTransactionAdapter adapter;
if ( preferUserTransactions ) {
adapter = makeUserTransactionAdapter();
if ( adapter == null ) {
log.debug( "Unable to access UserTransaction, attempting to use TransactionManager instead" );
adapter = makeTransactionManagerAdapter();
}
}
else {
adapter = makeTransactionManagerAdapter();
if ( adapter == null ) {
log.debug( "Unable to access TransactionManager, attempting to use UserTransaction instead" );
adapter = makeUserTransactionAdapter();
}
}
final JtaTransactionAdapter adapter =
preferUserTransactions
? getTransactionAdapterPreferringUserTransaction()
: getTransactionAdapterPreferringTransactionManager();
if ( adapter == null ) {
throw new JtaPlatformInaccessibleException(
"Unable to access TransactionManager or UserTransaction to make physical transaction delegate"
);
}
else {
return new TransactionDriverControlImpl( adapter );
}
}
private JtaTransactionAdapter getTransactionAdapterPreferringTransactionManager() {
final JtaTransactionAdapter adapter = makeTransactionManagerAdapter();
if ( adapter == null ) {
log.debug( "Unable to access TransactionManager, attempting to use UserTransaction instead" );
return makeUserTransactionAdapter();
}
return adapter;
}
private JtaTransactionAdapter getTransactionAdapterPreferringUserTransaction() {
final JtaTransactionAdapter adapter = makeUserTransactionAdapter();
if ( adapter == null ) {
log.debug( "Unable to access UserTransaction, attempting to use TransactionManager instead" );
return makeTransactionManagerAdapter();
}
return adapter;
}
private JtaTransactionAdapter makeUserTransactionAdapter() {
try {
final UserTransaction userTransaction = jtaPlatform.retrieveUserTransaction();
if ( userTransaction == null ) {
log.debug( "JtaPlatform#retrieveUserTransaction returned null" );
return null;
}
else {
return new JtaTransactionAdapterUserTransactionImpl( userTransaction );
@ -261,16 +257,16 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
}
catch ( Exception exception ) {
log.debugf( "JtaPlatform#retrieveUserTransaction threw an exception [%s]", exception.getMessage() );
}
return null;
}
}
private JtaTransactionAdapter makeTransactionManagerAdapter() {
try {
final TransactionManager transactionManager = jtaPlatform.retrieveTransactionManager();
if ( transactionManager == null ) {
log.debug( "JtaPlatform#retrieveTransactionManager returned null" );
return null;
}
else {
return new JtaTransactionAdapterTransactionManagerImpl( transactionManager );
@ -278,10 +274,9 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
}
catch ( Exception exception ) {
log.debugf( "JtaPlatform#retrieveTransactionManager threw an exception [%s]", exception.getMessage() );
}
return null;
}
}
@Override
public SynchronizationRegistry getLocalSynchronizations() {
@ -399,7 +394,6 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
@Override
public void begin() {
errorIfInvalid();
jtaTransactionAdapter.begin();
JtaTransactionCoordinatorImpl.this.joinJtaTransaction();
}
@ -414,7 +408,6 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
public void commit() {
errorIfInvalid();
getTransactionCoordinatorOwner().flushBeforeTransactionCompletion();
// we don't have to perform any before/after completion processing here. We leave that for
// the Synchronization callbacks
jtaTransactionAdapter.commit();
@ -423,7 +416,6 @@ public class JtaTransactionCoordinatorImpl implements TransactionCoordinator, Sy
@Override
public void rollback() {
errorIfInvalid();
// we don't have to perform any after completion processing here. We leave that for
// the Synchronization callbacks
jtaTransactionAdapter.rollback();

View File

@ -40,6 +40,8 @@ public interface TransactionCoordinatorBuilder extends Service {
PhysicalConnectionHandlingMode getDefaultConnectionHandlingMode();
default DdlTransactionIsolator buildDdlTransactionIsolator(JdbcContext jdbcContext) {
return isJta() ? new DdlTransactionIsolatorJtaImpl( jdbcContext ) : new DdlTransactionIsolatorNonJtaImpl( jdbcContext );
return isJta()
? new DdlTransactionIsolatorJtaImpl( jdbcContext )
: new DdlTransactionIsolatorNonJtaImpl( jdbcContext );
}
}

View File

@ -31,6 +31,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
/**
@ -73,7 +74,7 @@ public class TransactionCommitFailureTest {
em.getTransaction().commit();
}
catch (RollbackException e) {
assertEquals( COMMIT_FAILURE, e.getLocalizedMessage() );
assertTrue( e.getLocalizedMessage().startsWith( COMMIT_FAILURE ) );
}
finally {
if ( em.getTransaction() != null && em.getTransaction().isActive() ) {

View File

@ -193,7 +193,7 @@ public class SchemaUpdateTask extends MatchingTask {
throw new BuildException( "File not found: " + e.getMessage(), e );
}
catch (IOException e) {
throw new BuildException( "IOException : " + e.getMessage(), e );
throw new BuildException( "IOException: " + e.getMessage(), e );
}
catch (BuildException e) {
throw e;

View File

@ -135,7 +135,7 @@ public class SchemaValidatorTask extends MatchingTask {
throw new BuildException("File not found: " + e.getMessage(), e);
}
catch (IOException e) {
throw new BuildException("IOException : " + e.getMessage(), e);
throw new BuildException("IOException: " + e.getMessage(), e);
}
catch (BuildException e) {
throw e;