Rollback without transaction with propagation SUPPORTS causes a warn-level log entry [SPR-1749]

[spring-projects-issues]

Erwin Bolwidt opened SPR-1749 and commented

When a 'transaction' is rolled back (method processRollback in AbstractPlatformTransactionManager), in the testcase as a result of an exception in an intercepted class, a log entry is logged at level 'warn' with the message "[12:54:46,750 438 ,DataSourceTransactionManager] Should roll back transaction but cannot - no transaction available".

The transaction properties of the interceptor are defined as "PROPAGATION_SUPPORTS,-Exception". The problem also occurs with "PROPAGATION_NEVER,-Exception", which is in itself an illogical definition but allowed by spring.

The problem is that a transaction doesn't have to be present in the intercepted code - that's why PROPAGATION_SUPPORTS was used - and warn level log entries cause alarm bells to go off in the production environment. I think that a 'rollback' on a spring transaction that doesn't have a real transaction behind it - the implementation used for SUPPORTS - should not cause this warning because this is what is expected.

I can think of the following solutions:

1 - don't log the warning at all in AbstractPlatformTransactionManager
2 - pass a TransactionDefinition into the rollback method of AbstractPlatformTransactionManager so processRollback can decide to log dependent on the propagation behaviour
3 - let the TransactionAspectSupport class decide whether to call rollback or not dependent on the propagation behaviour and whether there is an actual transaction.

I think that 3 is easier to implement, but 2 is cleaner. However 1 is easiest and I don't see a big issue with it either.

One way to implement 3 is to patch the doCloseTransactionAfterThrowing method of TransactionAspectSupport, and replace the line

this.transactionManager.rollback(txInfo.getTransactionStatus());

with the code

int propagation = txInfo.getTransactionAttribute().getPropagationBehavior();
DefaultTransactionStatus txStatus = (DefaultTransactionStatus) txInfo.getTransactionStatus();
if (propagation == TransactionDefinition.PROPAGATION_MANDATORY ||
    propagation == TransactionDefinition.PROPAGATION_REQUIRED ||
    propagation == TransactionDefinition.PROPAGATION_REQUIRES_NEW || 
    txStatus.hasTransaction()) { 
    this.transactionManager.rollback(txInfo.getTransactionStatus());
}

---

Affects: 1.2.6

[spring-projects-issues]

Juergen Hoeller commented

I actually consider this as enhancement rather than as bug. I'll explain why.

"PROPAGATION_SUPPORTS,-Exception" is a bit of a contradiction itself. So you would absolutely want an immediate rollback, but only if you had an actual transaction? What use case justifies this behavior? Note that you don't need to specify a rollback rule here if you want to roll back as part of an outer transaction; it's usually sufficient to have the rollback rule on the outer transaction attribute then. The latter is probably what you intended.

If you - even potentially - need a rollback for the inner transaction even when running on its own, use PROPAGATION_REQUIRED - an actual transaction. PROPAGATION_SUPPORTS essentially says, in such a scenario: "let's rollback if we can and don't care for it else". That is at best feasible for test environments, but not intended for production environments. Hence the warn logging, which more or less says "hey, we encountered a rollback need according to your specified rollback rules - but you didn't allow for an actual transaction".

On the other hand, if you just want to participate in transaction-scoped resource handling, mainly for read-only operations, then use PROPAGATION_SUPPORTS - but without the "-Exception" part. This effectively means you're participating in a transaction scope, but without an actual database transaction backing it. Still gives you shared resources such as scoped Hibernate Sessions etc, but no strong rollback semantics.

It seems to me that the warn logging is appropriate since it points at a semantic mismatch in your configuration. Try using either "PROPAGATION_SUPPORTS" without the "-Exception" part, when there is no chance for a rollback need in the inner transaction itself, or "PROPAGATION_REQUIRED,-Exception" when an actual rollback need might arise.

Juergen

[spring-projects-issues]

Erwin Bolwidt commented

I would like to ask you to classify it as a bug. I'll explain why below:

The same behaviour that I describe occurs when a RuntimeException is thrown. So even with just "PROPAGATION_SUPPORTS", without any exceptions suffixed, you still get the same message at warn level. Worse, it even occurs with PROPAGATION_NEVER.

This is, in my opionion a bug. To fix this, you would automatcically also fix the rollback of non-runtime exceptions, because the internal handling of the two cases is the same.

Apart from that, why is there a semantic mismatch? Compare it with EJB semantics: with SUPPORTS, and an exception that causes a rollback is thrown (only RuntimeExceptions with EJB's), the transaction is rolled back only if there is a transaction. Otherwise, no action is taken. Spring extends this concept with the option to specify which transaction is rolled back, but then why should this option not work for SUPPORTS transactions, while rolling back on RuntimeExceptions does work?

It wouldn't be a very usual situation where one would want to do this, but should spring forbid something that is useful in some circumstances?
Take for example a method that receives a JMS message and performs a database update. Depending on circumstances, it may or may not be necessary to rollback the receive of the JMS message when something in the database update fails. So the transaction propagation is marked as 'SUPPORTS', and
the implementer of the method knows that if the code is run in a pre-existing transaction, the occurrence of a certain checked exception means that the whole transaction should be rolled back. But if there was no transaction context, then the caller didn't want a rollback, and the exception is rethrown unhandled.

Not something every application will want to do, but not an unimaginable situation.

• Erwin

[spring-projects-issues]

Juergen Hoeller commented

OK, I agree that the warn level message might be inappropriate in some scenarios. Hence, I've lowered it to debug level, considering this as expected case that the system administrator doesn't have to be made aware of.

Thanks for the input!

Juergen