Expected behavior
Using reactive transaction management, when a commit step fails in the TransactionalOperator, then the commit failure exception should be propagated. This can be for example a ConcurrencyFailureException.
Actual behavior (Spring 5.3.9)
When a commit fails, the commit exception is logged & dropped in TransactionalOperatorImpl, and instead an IllegalTransactionStateException is propagated.
Practical case
When under high concurrency, our application is observing uninformative IllegalTransactionStateException like this:
org.springframework.transaction.IllegalTransactionStateException: Transaction is already completed - do not call commit or rollback more than once per transaction
at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.rollback(AbstractReactiveTransactionManager.java:492)
Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException:
Assembly trace from producer [reactor.core.publisher.MonoLift] :
reactor.core.publisher.Mono.error
org.springframework.transaction.reactive.AbstractReactiveTransactionManager.rollback(AbstractReactiveTransactionManager.java:492)
Error has been observed at the following site(s):
|_ Mono.error ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.rollback(AbstractReactiveTransactionManager.java:492)
|_ Mono.onErrorMap ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.rollbackOnException(TransactionalOperatorImpl.java:119)
|_ Mono.then ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.lambda$null$2(TransactionalOperatorImpl.java:83)
|_ Mono.onErrorResume ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.lambda$null$3(TransactionalOperatorImpl.java:83)
|_ Mono.flatMap ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.lambda$transactional$4(TransactionalOperatorImpl.java:81)
|_ Mono.flatMap ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.transactional(TransactionalOperatorImpl.java:75)
|_ Mono.contextWrite ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.transactional(TransactionalOperatorImpl.java:85)
|_ Mono.contextWrite ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.transactional(TransactionalOperatorImpl.java:86)
The above illegal transaction state seems to be overriding the propagation of this exception:
ERROR --- o.s.t.r.TransactionalOperatorImpl : Application exception overridden by rollback exception
java.lang.RuntimeException: Async resource cleanup failed after onComplete
at reactor.core.publisher.FluxUsingWhen$CommitInner.onError(FluxUsingWhen.java:533)
Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException:
Assembly trace from producer [reactor.core.publisher.MonoLift] :
reactor.core.publisher.Mono.usingWhen
org.springframework.transaction.reactive.TransactionalOperatorImpl.lambda$null$3(TransactionalOperatorImpl.java:81)
Error has been observed at the following site(s):
|_ Mono.usingWhen ⇢ at org.springframework.transaction.reactive.TransactionalOperatorImpl.lambda$null$3(TransactionalOperatorImpl.java:81)
Stack trace:
at reactor.core.publisher.FluxUsingWhen$CommitInner.onError(FluxUsingWhen.java:533)
...
at io.r2dbc.postgresql.util.FluxDiscardOnCancel$FluxDiscardOnCancelSubscriber.onComplete(FluxDiscardOnCancel.java:99)
at io.r2dbc.postgresql.client.ReactorNettyClient$Conversation.complete(ReactorNettyClient.java:719)
at io.r2dbc.postgresql.client.ReactorNettyClient$BackendMessageSubscriber.emit(ReactorNettyClient.java:984)
...
Caused by: org.springframework.dao.ConcurrencyFailureException: R2DBC commit; could not serialize access due to read/write dependencies among transactions;
at org.springframework.r2dbc.connection.ConnectionFactoryUtils.convertR2dbcException(ConnectionFactoryUtils.java:218)
Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException:
Assembly trace from producer [reactor.core.publisher.MonoLift] :
reactor.core.publisher.Mono.onErrorMap
org.springframework.r2dbc.connection.R2dbcTransactionManager.doCommit(R2dbcTransactionManager.java:279)
Error has been observed at the following site(s):
|_ Mono.onErrorMap ⇢ at org.springframework.r2dbc.connection.R2dbcTransactionManager.doCommit(R2dbcTransactionManager.java:279)
|_ ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.lambda$processCommit$21(AbstractReactiveTransactionManager.java:445)
|_ Mono.defer ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.processCommit(AbstractReactiveTransactionManager.java:439)
|_ Mono.then ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.processCommit(AbstractReactiveTransactionManager.java:439)
|_ Mono.then ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.processCommit(AbstractReactiveTransactionManager.java:448)
|_ Mono.then ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.processCommit(AbstractReactiveTransactionManager.java:474)
|_ Mono.error ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.lambda$processCommit$25(AbstractReactiveTransactionManager.java:480)
|_ Mono.then ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.lambda$processCommit$25(AbstractReactiveTransactionManager.java:480)
|_ Mono.onErrorResume ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.processCommit(AbstractReactiveTransactionManager.java:479)
|_ Mono.then ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.processCommit(AbstractReactiveTransactionManager.java:480)
|_ ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.lambda$commit$20(AbstractReactiveTransactionManager.java:420)
|_ Mono.flatMap ⇢ at org.springframework.transaction.reactive.AbstractReactiveTransactionManager.commit(AbstractReactiveTransactionManager.java:412)
Stack trace:
at org.springframework.r2dbc.connection.ConnectionFactoryUtils.convertR2dbcException(ConnectionFactoryUtils.java:218)
at org.springframework.r2dbc.connection.R2dbcTransactionManager.translateException(R2dbcTransactionManager.java:439)
at org.springframework.r2dbc.connection.R2dbcTransactionManager.lambda$doCommit$9(R2dbcTransactionManager.java:279)
...
Investigation of the cause
The problem described above matches this warning on ReactiveTransactionManager#rollback:
|
* <p><b>Do not call rollback on a transaction if commit threw an exception.</b> |
|
* The transaction will already have been completed and cleaned up when commit |
|
* returns, even in case of a commit exception. Consequently, a rollback call |
|
* after commit failure will lead to an IllegalTransactionStateException. |
Looking at the current implementation of TransactionalOperatorImpl, it looks like indeed this can happen; if transactionManager::commit emits an error then a rollback will be attempted by the onErrorResume:
|
public <T> Mono<T> transactional(Mono<T> mono) { |
|
return TransactionContextManager.currentContext().flatMap(context -> { |
|
Mono<ReactiveTransaction> status = this.transactionManager.getReactiveTransaction(this.transactionDefinition); |
|
// This is an around advice: Invoke the next interceptor in the chain. |
|
// This will normally result in a target object being invoked. |
|
// Need re-wrapping of ReactiveTransaction until we get hold of the exception |
|
// through usingWhen. |
|
return status.flatMap(it -> Mono.usingWhen(Mono.just(it), ignore -> mono, |
|
this.transactionManager::commit, (res, err) -> Mono.empty(), this.transactionManager::rollback) |
|
.onErrorResume(ex -> rollbackOnException(it, ex).then(Mono.error(ex)))); |
|
}) |
Looks related to the discussion in #23562
Possible solution
The operators in TransactionalOperatorImpl#transactional could be restructured in such a way that rollbacks are not attempted after a failure to commit. Maybe this could be accomplished by moving the rollbackOnException to the asyncError parameter in the usingWhen. A similar approach should be applied in TransactionalOperatorImpl#execute.
Additionally, would it make sense to add the application exception as suppressed when the rollback in rollbackOnException emits an exception? This could make it easier to debug this and similar issues because currently the application exception is only linked when the rollback emits an TransactionSystemException, but not in other cases like IllegalTransactionStateException.
Expected behavior
Using reactive transaction management, when a commit step fails in the
TransactionalOperator, then the commit failure exception should be propagated. This can be for example aConcurrencyFailureException.Actual behavior (Spring 5.3.9)
When a commit fails, the commit exception is logged & dropped in
TransactionalOperatorImpl, and instead anIllegalTransactionStateExceptionis propagated.Practical case
When under high concurrency, our application is observing uninformative
IllegalTransactionStateExceptionlike this:The above illegal transaction state seems to be overriding the propagation of this exception:
Investigation of the cause
The problem described above matches this warning on
ReactiveTransactionManager#rollback:spring-framework/spring-tx/src/main/java/org/springframework/transaction/ReactiveTransactionManager.java
Lines 99 to 102 in fb7eea9
Looking at the current implementation of
TransactionalOperatorImpl, it looks like indeed this can happen; iftransactionManager::commitemits an error then a rollback will be attempted by theonErrorResume:spring-framework/spring-tx/src/main/java/org/springframework/transaction/reactive/TransactionalOperatorImpl.java
Lines 74 to 84 in fb7eea9
Looks related to the discussion in #23562
Possible solution
The operators in
TransactionalOperatorImpl#transactionalcould be restructured in such a way that rollbacks are not attempted after a failure to commit. Maybe this could be accomplished by moving therollbackOnExceptionto theasyncErrorparameter in theusingWhen. A similar approach should be applied inTransactionalOperatorImpl#execute.Additionally, would it make sense to add the application exception as suppressed when the rollback in
rollbackOnExceptionemits an exception? This could make it easier to debug this and similar issues because currently the application exception is only linked when the rollback emits anTransactionSystemException, but not in other cases likeIllegalTransactionStateException.