GoogleCloudPlatform / spring-cloud-gcp

New home for Spring Cloud GCP development starting with version 2.0.
Apache License 2.0
414 stars 308 forks source link

Add support for doSetRollbackOnly in SpannerTransactionManager #2358

Open credmond-git opened 10 months ago

credmond-git commented 10 months ago

Describe the bug

When i am in a transaction and it throws an exception, it will catch the exception in TransactionAspectSupport.invokeWithinTransaction and try to rollback

try {
// This is an around advice: Invoke the next interceptor in the chain.
// This will normally result in a target object being invoked.
retVal = invocation.proceedWithInvocation();
}
catch (Throwable ex) {
// target invocation exception
completeTransactionAfterThrowing(txInfo, ex);
throw ex;
}

This will eventually call

Set the given transaction rollback-only. Only called on rollback if the current transaction participates in an existing one.
The default implementation throws an IllegalTransactionStateException, assuming that participating in existing transactions is generally not supported. Subclasses are of course encouraged to provide such support.
Params:
status – the status representation of the transaction
Throws:
TransactionException – in case of system errors
protected void doSetRollbackOnly(DefaultTransactionStatus status) throws TransactionException {
throw new IllegalTransactionStateException(
        "Participating in existing transactions is not supported - when 'isExistingTransaction' " +
        "returns true, appropriate 'doSetRollbackOnly' behavior must be provided");
}

In other transaction managers such as JpaTransactionManager they set the rollback only flag which seems to cause the transaction to be rolled back.

@Override
protected void doSetRollbackOnly(DefaultTransactionStatus status) {
JpaTransactionObject txObject = (JpaTransactionObject) status.getTransaction();
if (status.isDebug()) {
    logger.debug("Setting JPA transaction on EntityManager [" +
            txObject.getEntityManagerHolder().getEntityManager() + "] rollback-only");
}
txObject.setRollbackOnly();
}

It would seem like SpannerTransactionManager should implement something similar. I initially filed a bug with Spring issue 31615, they sound happy to help resolve the issue. Related to issues: spring-gcp issue 907

See the callstack below:

doSetRollbackOnly:1207, AbstractPlatformTransactionManager (org.springframework.transaction.support)
processRollback:844, AbstractPlatformTransactionManager (org.springframework.transaction.support)
rollback:809, AbstractPlatformTransactionManager (org.springframework.transaction.support)
completeTransactionAfterThrowing:678, TransactionAspectSupport (org.springframework.transaction.interceptor)
invokeWithinTransaction:395, TransactionAspectSupport (org.springframework.transaction.interceptor)
invoke:119, TransactionInterceptor (org.springframework.transaction.interceptor)
proceed:184, ReflectiveMethodInvocation (org.springframework.aop.framework)
proceed:751, CglibAopProxy$CglibMethodInvocation (org.springframework.aop.framework)
intercept:703, CglibAopProxy$DynamicAdvisedInterceptor (org.springframework.aop.framework)
deleteProjectReason:-1, ProjectServiceImpl$$SpringCGLIB$$0 (com.my.project.service)
lambda 'shouldThrow' in 'delete project reason that doesn't exists':124, ProjectsIntegrationTest (com.my.project)
shouldThrow:129, ProjectsIntegrationTest (com.my.project)
delete project reason that doesn't exists:124, ProjectsIntegrationTest (com.my.project)
invoke0:-1, NativeMethodAccessorImpl (jdk.internal.reflect)
invoke:77, NativeMethodAccessorImpl (jdk.internal.reflect)
invoke:43, DelegatingMethodAccessorImpl (jdk.internal.reflect)
invoke:568, Method (java.lang.reflect)
invokeMethod:727, ReflectionUtils (org.junit.platform.commons.util)
proceed:60, MethodInvocation (org.junit.jupiter.engine.execution)
proceed:131, InvocationInterceptorChain$ValidatingInvocation (org.junit.jupiter.engine.execution)
intercept:156, TimeoutExtension (org.junit.jupiter.engine.extension)
interceptTestableMethod:147, TimeoutExtension (org.junit.jupiter.engine.extension)
interceptTestMethod:86, TimeoutExtension (org.junit.jupiter.engine.extension)
apply:-1, TestMethodTestDescriptor$$Lambda$233/0x00000237d5151a40 (org.junit.jupiter.engine.descriptor)
lambda$ofVoidMethod$0:103, InterceptingExecutableInvoker$ReflectiveInterceptorCall (org.junit.jupiter.engine.execution)
apply:-1, InterceptingExecutableInvoker$ReflectiveInterceptorCall$$Lambda$234/0x00000237d5151e60 (org.junit.jupiter.engine.execution)
lambda$invoke$0:93, InterceptingExecutableInvoker (org.junit.jupiter.engine.execution)
apply:-1, InterceptingExecutableInvoker$$Lambda$543/0x00000237d52be498 (org.junit.jupiter.engine.execution)
proceed:106, InvocationInterceptorChain$InterceptedInvocation (org.junit.jupiter.engine.execution)
proceed:64, InvocationInterceptorChain (org.junit.jupiter.engine.execution)
chainAndInvoke:45, InvocationInterceptorChain (org.junit.jupiter.engine.execution)
invoke:37, InvocationInterceptorChain (org.junit.jupiter.engine.execution)
invoke:92, InterceptingExecutableInvoker (org.junit.jupiter.engine.execution)
invoke:86, InterceptingExecutableInvoker (org.junit.jupiter.engine.execution)
lambda$invokeTestMethod$7:217, TestMethodTestDescriptor (org.junit.jupiter.engine.descriptor)
execute:-1, TestMethodTestDescriptor$$Lambda$3131/0x00000237d630d240 (org.junit.jupiter.engine.descriptor)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
invokeTestMethod:213, TestMethodTestDescriptor (org.junit.jupiter.engine.descriptor)
execute:138, TestMethodTestDescriptor (org.junit.jupiter.engine.descriptor)
execute:68, TestMethodTestDescriptor (org.junit.jupiter.engine.descriptor)
lambda$executeRecursively$6:151, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:-1, NodeTestTask$$Lambda$339/0x00000237d516e698 (org.junit.platform.engine.support.hierarchical)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$8:141, NodeTestTask (org.junit.platform.engine.support.hierarchical)
invoke:-1, NodeTestTask$$Lambda$338/0x00000237d516e470 (org.junit.platform.engine.support.hierarchical)
around:137, Node (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$9:139, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:-1, NodeTestTask$$Lambda$337/0x00000237d516e048 (org.junit.platform.engine.support.hierarchical)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
executeRecursively:138, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:95, NodeTestTask (org.junit.platform.engine.support.hierarchical)
accept:-1, SameThreadHierarchicalTestExecutorService$$Lambda$343/0x00000237d516f1b0 (org.junit.platform.engine.support.hierarchical)
forEach:1511, ArrayList (java.util)
invokeAll:41, SameThreadHierarchicalTestExecutorService (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$6:155, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:-1, NodeTestTask$$Lambda$339/0x00000237d516e698 (org.junit.platform.engine.support.hierarchical)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$8:141, NodeTestTask (org.junit.platform.engine.support.hierarchical)
invoke:-1, NodeTestTask$$Lambda$338/0x00000237d516e470 (org.junit.platform.engine.support.hierarchical)
around:137, Node (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$9:139, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:-1, NodeTestTask$$Lambda$337/0x00000237d516e048 (org.junit.platform.engine.support.hierarchical)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
executeRecursively:138, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:95, NodeTestTask (org.junit.platform.engine.support.hierarchical)
accept:-1, SameThreadHierarchicalTestExecutorService$$Lambda$343/0x00000237d516f1b0 (org.junit.platform.engine.support.hierarchical)
forEach:1511, ArrayList (java.util)
invokeAll:41, SameThreadHierarchicalTestExecutorService (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$6:155, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:-1, NodeTestTask$$Lambda$339/0x00000237d516e698 (org.junit.platform.engine.support.hierarchical)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$8:141, NodeTestTask (org.junit.platform.engine.support.hierarchical)
invoke:-1, NodeTestTask$$Lambda$338/0x00000237d516e470 (org.junit.platform.engine.support.hierarchical)
around:137, Node (org.junit.platform.engine.support.hierarchical)
lambda$executeRecursively$9:139, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:-1, NodeTestTask$$Lambda$337/0x00000237d516e048 (org.junit.platform.engine.support.hierarchical)
execute:73, ThrowableCollector (org.junit.platform.engine.support.hierarchical)
executeRecursively:138, NodeTestTask (org.junit.platform.engine.support.hierarchical)
execute:95, NodeTestTask (org.junit.platform.engine.support.hierarchical)
submit:35, SameThreadHierarchicalTestExecutorService (org.junit.platform.engine.support.hierarchical)
execute:57, HierarchicalTestExecutor (org.junit.platform.engine.support.hierarchical)
execute:54, HierarchicalTestEngine (org.junit.platform.engine.support.hierarchical)
execute:147, EngineExecutionOrchestrator (org.junit.platform.launcher.core)
execute:127, EngineExecutionOrchestrator (org.junit.platform.launcher.core)
execute:90, EngineExecutionOrchestrator (org.junit.platform.launcher.core)
lambda$execute$0:55, EngineExecutionOrchestrator (org.junit.platform.launcher.core)
accept:-1, EngineExecutionOrchestrator$$Lambda$291/0x00000237d515caf0 (org.junit.platform.launcher.core)
withInterceptedStreams:102, EngineExecutionOrchestrator (org.junit.platform.launcher.core)
execute:54, EngineExecutionOrchestrator (org.junit.platform.launcher.core)
execute:114, DefaultLauncher (org.junit.platform.launcher.core)
execute:86, DefaultLauncher (org.junit.platform.launcher.core)
execute:86, DefaultLauncherSession$DelegatingLauncher (org.junit.platform.launcher.core)
processAllTestClasses:119, JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor (org.gradle.api.internal.tasks.testing.junitplatform)
access$000:94, JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor (org.gradle.api.internal.tasks.testing.junitplatform)
stop:89, JUnitPlatformTestClassProcessor (org.gradle.api.internal.tasks.testing.junitplatform)
stop:62, SuiteTestClassProcessor (org.gradle.api.internal.tasks.testing)
invoke0:-1, NativeMethodAccessorImpl (jdk.internal.reflect)
invoke:77, NativeMethodAccessorImpl (jdk.internal.reflect)
invoke:43, DelegatingMethodAccessorImpl (jdk.internal.reflect)
invoke:568, Method (java.lang.reflect)
dispatch:36, ReflectionDispatch (org.gradle.internal.dispatch)
dispatch:24, ReflectionDispatch (org.gradle.internal.dispatch)
dispatch:33, ContextClassLoaderDispatch (org.gradle.internal.dispatch)
invoke:94, ProxyDispatchAdapter$DispatchingInvocationHandler (org.gradle.internal.dispatch)
stop:-1, $Proxy5 (jdk.proxy2)
run:193, TestWorker$3 (org.gradle.api.internal.tasks.testing.worker)
executeAndMaintainThreadName:129, TestWorker (org.gradle.api.internal.tasks.testing.worker)
execute:100, TestWorker (org.gradle.api.internal.tasks.testing.worker)
execute:60, TestWorker (org.gradle.api.internal.tasks.testing.worker)
execute:56, ActionExecutionWorker (org.gradle.process.internal.worker.child)
call:113, SystemApplicationClassLoaderWorker (org.gradle.process.internal.worker.child)
call:65, SystemApplicationClassLoaderWorker (org.gradle.process.internal.worker.child)
run:69, GradleWorkerMain (worker.org.gradle.process.internal.worker)
main:74, GradleWorkerMain (worker.org.gradle.process.internal.worker)
credmond-git commented 10 months ago

It looks like the older hibernate 5 had something similar to call setRollbackOnly()

https://github.com/spring-projects/spring-framework/blob/7b14606d92bfe36fea048137e794cfff18e9e42a/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java#L704

@Override
    protected void doSetRollbackOnly(DefaultTransactionStatus status) {
        HibernateTransactionObject txObject = (HibernateTransactionObject) status.getTransaction();
        if (status.isDebug()) {
            logger.debug("Setting Hibernate transaction on Session [" +
                    txObject.getSessionHolder().getSession() + "] rollback-only");
        }
        txObject.setRollbackOnly();
    }

Also DataSourceTransactionManager implements it with a similar flow.

https://github.com/spring-projects/spring-framework/blob/7b14606d92bfe36fea048137e794cfff18e9e42a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java#L359C1-L367C3

    @Override
    protected void doSetRollbackOnly(DefaultTransactionStatus status) {
        DataSourceTransactionObject txObject = (DataSourceTransactionObject) status.getTransaction();
        if (status.isDebug()) {
            logger.debug("Setting JDBC transaction [" + txObject.getConnectionHolder().getConnection() +
                    "] rollback-only");
        }
        txObject.setRollbackOnly();
    }