Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JdbcLockRegistry is unable to retry a lock when there is a serialization problem and you are using JPATransactionManager instead of DataSourceTransactionManager #3733

Closed
estigma88 opened this issue Feb 28, 2022 · 11 comments · Fixed by #3782

Comments

@estigma88
Copy link

estigma88 commented Feb 28, 2022

In what version(s) of Spring Integration are you seeing this issue?

Spring boot version: 2.6.2
Spring data jpa: 2.6.0
Spring Integration version: 5.5.6

Describe the bug

JdbcLockRegistry is unable to retry a lock when there is a serialization problem and you are using JPATransactionManager instead of DataSourceTransactionManager.

As I have an application that uses the JdbcLockRegistry and JPA with Hibernate, by default, the application uses JPATransactionManager, but seems like for some transactional errors, JdbcLockRegistry is unable to retry the lock, as you can see in the following stack trace:

Caused by: org.springframework.dao.CannotAcquireLockException: Failed to lock mutex at 19414b49-c76b-3b77-a05d-3aca07a855aa; nested exception is org.springframework.orm.jpa.JpaSystemException: Unable to commit against JDBC Connection; nested exception is org.hibernate.TransactionException: Unable to commit against JDBC Connection
	at org.springframework.integration.jdbc.lock.JdbcLockRegistry$JdbcLock.rethrowAsLockException(JdbcLockRegistry.java:197)
	at org.springframework.integration.jdbc.lock.JdbcLockRegistry$JdbcLock.tryLock(JdbcLockRegistry.java:262)
	at com.geniussports.geniuslive.ingressmanager.domain.pipeline.ingress.IngressPipelineService.create(IngressPipelineService.kt:35)
	at com.geniussports.geniuslive.ingressmanager.application.adapters.pipeline.ingress.gateway.graphql.PipelineMutationResolver.createFromJSON(PipelineMutationResolver.kt:51)
	at jdk.internal.reflect.GeneratedMethodAccessor254.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.springframework.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:282)
	at com.netflix.graphql.dgs.internal.DgsSchemaProvider.invokeDataFetcher(DgsSchemaProvider.kt:402)
	at com.netflix.graphql.dgs.internal.DgsSchemaProvider.access$invokeDataFetcher(DgsSchemaProvider.kt:64)
	at com.netflix.graphql.dgs.internal.DgsSchemaProvider$createBasicDataFetcher$1.get(DgsSchemaProvider.kt:279)
	at com.netflix.graphql.dgs.metrics.micrometer.DgsGraphQLMetricsInstrumentation$instrumentDataFetcher$1.get(DgsGraphQLMetricsInstrumentation.kt:101)
	at graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation.lambda$instrumentDataFetcher$0(DataLoaderDispatcherInstrumentation.java:86)
	at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:270)
	... 76 more
Caused by: org.springframework.orm.jpa.JpaSystemException: Unable to commit against JDBC Connection; nested exception is org.hibernate.TransactionException: Unable to commit against JDBC Connection
	at org.springframework.orm.jpa.vendor.HibernateJpaDialect.convertHibernateAccessException(HibernateJpaDialect.java:331)
	at org.springframework.orm.jpa.vendor.HibernateJpaDialect.translateExceptionIfPossible(HibernateJpaDialect.java:233)
	at org.springframework.orm.jpa.JpaTransactionManager.doCommit(JpaTransactionManager.java:566)
	at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:743)
	at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:711)
	at org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManager.commit(TracePlatformTransactionManager.java:121)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:654)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:407)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:753)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:698)
	at org.springframework.integration.jdbc.lock.DefaultLockRepository$$EnhancerBySpringCGLIB$$29ab5ce4.acquire(<generated>)
	at org.springframework.integration.jdbc.lock.JdbcLockRegistry$JdbcLock.doLock(JdbcLockRegistry.java:268)
	at org.springframework.integration.jdbc.lock.JdbcLockRegistry$JdbcLock.tryLock(JdbcLockRegistry.java:249)
	... 88 more
Caused by: org.hibernate.TransactionException: Unable to commit against JDBC Connection
			3 lines skipped for [org.hibernate]
	at org.springframework.orm.jpa.JpaTransactionManager.doCommit(JpaTransactionManager.java:562)
	... 100 more
Caused by: org.postgresql.util.PSQLException: ERROR: could not serialize access due to read/write dependencies among transactions
  Detail: Reason code: Canceled on identification as a pivot, during commit attempt.
  Hint: The transaction might succeed if retried.
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2674)
	at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2364)
	at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:354)
	at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:314)
	at org.postgresql.jdbc.PgConnection.executeTransactionCommand(PgConnection.java:853)
	at org.postgresql.jdbc.PgConnection.commit(PgConnection.java:875)
	at com.zaxxer.hikari.pool.ProxyConnection.commit(ProxyConnection.java:387)
	at com.zaxxer.hikari.pool.HikariProxyConnection.commit(HikariProxyConnection.java)
			1 line skipped for [org.hibernate]

As far as I can tell, seems like to retry a lock we expect the following exceptions to be thrown, as you can see here:

TransientDataAccessException | TransactionTimedOutException | TransactionSystemException

However, Hibernate is throwing a org.hibernate.TransactionException, and after wrapped in the JpaSystemException, which is not part of any of the previous exceptions hierarchy, and therefore, the retry is not applied. I created this bug on the Hibernate side as I think we might not have a lot of options in Spring side.

Now, regarding workarounds, I can tell the following:

  1. Override the JPATransactionManager to catch this particular error and throw the right exception, like org.springframework.dao.CannotAcquireLockException, which is a TransientDataAccessException. This stack overflow question shows how. This solution seems pretty fragile.
  2. Creating a new XXXXXLockRespository, which inherit from DefaultLockRepository, and override the @Transactional annotation to explicitly tell which transaction manager to use, and declare a new DataSourceTransactionManager only to be used for the new XXXXXLockRepository.

Not sure if we have more options.

To Reproduce

Use a JPATransactionManager instead of DataSourceTransactionManager for JdbcLockRegistry, and generating a serialization database error.

Expected behavior

JdbcLockRegistry should work with JPATransactionManager and DataSourceTransactionManager

@estigma88 estigma88 added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Feb 28, 2022
@artembilan
Copy link
Member

How about to treat that org.springframework.orm.jpa.JpaSystemException as we do now with that TransactionSystemException?
Seems like both of them are relevant to the same problem in end on DB.
I guess if you would use DataSourceTransactionManager instead for the same serialization database error, you would end up with the TransactionSystemException.

See also this thread: #3613

@artembilan artembilan added status: waiting-for-reporter Needs a feedback from the reporter and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Feb 28, 2022
@estigma88
Copy link
Author

How about to treat that org.springframework.orm.jpa.JpaSystemException as we do now with that TransactionSystemException?

What do you mean with treat them equal? which part of the code/process are you referring to?

I guess if you would use DataSourceTransactionManager instead for the same serialization database error, you would end up with the TransactionSystemException.

Really? I checked the DataSourceTransactionManager code and they handle directly the SQLException, which I think is the problem here, as Hibernate is throwing a TransactionException instead.

@artembilan
Copy link
Member

I mean this:

TransientDataAccessException | TransactionTimedOutException | TransactionSystemException | JpaSystemException 

That's because I see that TransactionSystemException is throws in similar way in the JdbcTransactionManager as a fallback as it is with the JpaSystemException in the HibernateJpaDialect.

Yeah... I see what you mean about that HibernateJpaDialect.convertHibernateAccessException(). Right, they don't check for a cause. Taking that suggestion back.

At the same time: how about to have a custom HibernateJpaDialect and override its convertHibernateAccessException() to re-throw whatever we would expect in the JdbcLockRegistry.

@estigma88
Copy link
Author

Yeah, the HibernateJpaDialect override seems another option

@estigma88
Copy link
Author

We decided to go for the following solution: overriding the DefaultLockRepository to handle a different TransactionManager.

By default, using DataSourceTransactionManager catches well the serializable exception, however, the DefaultLockRepository grabs the transaction manager from the Spring context, there is not an easy way to override it, so, we do the following:

First, override the DefaultLockRepository class, as follows:

class CustomTransactionManagerLockRepository(private val dataSource: DataSource) : DefaultLockRepository(dataSource) {
    @Transactional(propagation = Propagation.REQUIRES_NEW, transactionManager = "distributedLockTransactionManager")
    override fun close() {
        super.close()
    }

    @Transactional(propagation = Propagation.REQUIRES_NEW, transactionManager = "distributedLockTransactionManager")
    override fun delete(lock: String?) {
        super.delete(lock)
    }

    @Transactional(
        propagation = Propagation.REQUIRES_NEW,
        isolation = Isolation.SERIALIZABLE,
        transactionManager = "distributedLockTransactionManager"
    )
    override fun acquire(lock: String?): Boolean {
        return super.acquire(lock)
    }

    @Transactional(
        propagation = Propagation.REQUIRES_NEW,
        readOnly = true,
        transactionManager = "distributedLockTransactionManager"
    )
    override fun isAcquired(lock: String?): Boolean {
        return super.isAcquired(lock)
    }

    @Transactional(propagation = Propagation.REQUIRES_NEW, transactionManager = "distributedLockTransactionManager")
    override fun deleteExpired() {
        super.deleteExpired()
    }

    @Transactional(propagation = Propagation.REQUIRES_NEW, transactionManager = "distributedLockTransactionManager")
    override fun renew(lock: String?): Boolean {
        return super.renew(lock)
    }
}

There, we explicitly set which transaction manager we want to use for the lock.

Second, we create the DataSourceTransactionManager and the custom LockRepository, as follows:

   @Bean("distributedLockTransactionManager")
    fun transactionManager(
        @Qualifier("pipelineJsonDataSource") dataSource: HikariDataSource,
    ): PlatformTransactionManager {
        return DataSourceTransactionManager(dataSource)
    }

    @Bean
    fun customTransactionManagerLockRepository(
        @Qualifier("pipelineJsonDataSource") dataSource: HikariDataSource,
        @Value(value = "\${spring.jpa.properties.hibernate.default_schema}") schema: String
    ): DefaultLockRepository {
        val defaultLockRepository = CustomTransactionManagerLockRepository(dataSource)
        defaultLockRepository.setPrefix("$schema.INT_")
        return defaultLockRepository
    }

We think this is less invasive than overriding the HibernateJpaDialect.

@artembilan
Copy link
Member

I think that's the one of those reasons why we have a LockRepository abstraction extracted.
It is indeed possible that DefaultLockRepository might not work with the default transactionManager bean.

I think as an out-of-the-box fix we may reconsider those @Transactional in favor of explicit TransactionTemplate usage...

If you agree, we may reopen the issue and proceed with the possible, flexible solution.
This way what you would need is just inject an appropriate TransactionManager in this DefaultLockRepository and there won't be confuses with the global one.

@estigma88
Copy link
Author

That sounds great, reopening the issue

@estigma88 estigma88 reopened this Apr 11, 2022
@artembilan artembilan added this to the 6.0 M3 milestone Apr 11, 2022
artembilan added a commit to artembilan/spring-integration that referenced this issue Apr 11, 2022
Fixes spring-projects#3733

The `@Transactional` resolves a primary `TransactionManager` bean
from the application context which might not be sufficient for all
the use-case.

To make it work with the custom (or specific) `TransactionManager`
we have to extend a `DefaultLockRepository` and override all those
`@Transactional` method

* Change the logic of the `DefaultLockRepository` from `@Transactional`
to the `TransactionTemplate` and use provided `TransactionManager`
or resolve one from the application context
* Adjust tests to use explicit `TransactionManager` and call
`afterSingletonsInstantiated()` to initialize a default `TransactionTemplate`
* Mention the change in the docs
garyrussell added a commit that referenced this issue Apr 12, 2022
* GH-3733 Configure TxManager for DefLockRepository

Fixes #3733

The `@Transactional` resolves a primary `TransactionManager` bean
from the application context which might not be sufficient for all
the use-case.

To make it work with the custom (or specific) `TransactionManager`
we have to extend a `DefaultLockRepository` and override all those
`@Transactional` method

* Change the logic of the `DefaultLockRepository` from `@Transactional`
to the `TransactionTemplate` and use provided `TransactionManager`
or resolve one from the application context
* Adjust tests to use explicit `TransactionManager` and call
`afterSingletonsInstantiated()` to initialize a default `TransactionTemplate`
* Mention the change in the docs

* * Extracted all the `TransactionTemplate`s to the properties for caching
* Add `BeanInitializationException` for no-unique `PlatformTransactionManager`
bean in the `afterSingletonsInstantiated()`

* Fix language in the exception message

Co-authored-by: Gary Russell <grussell@vmware.com>

Co-authored-by: Gary Russell <grussell@vmware.com>
@HUSTluoqingqing
Copy link

I still have this problem with 'org.springframework.integration:spring-integration-jdbc:5.5.16'

@HUSTluoqingqing
Copy link

Was it fixed in version 6?

@artembilan
Copy link
Member

This was a breaking change, therefore it didn't make it into 5.5.x.
The respective commit definitely points that fix has made it into 6.x generation: 4b57363

@HUSTluoqingqing
Copy link

This was a breaking change, therefore it didn't make it into 5.5.x.
The respective commit definitely points that fix has made it into 6.x generation: 4b57363

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants