Skip to content

DataSourceTransactionManager does not consider SQLExceptionTranslator on commit #24064

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

Closed
pbillen opened this issue Nov 23, 2019 · 4 comments
Closed
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Milestone

Comments

@pbillen
Copy link

pbillen commented Nov 23, 2019

Disclaimer: I understand that questions should be asked on SO, but I believe the following is rather a bug report than a question. It has also been asked on https://stackoverflow.com/questions/59006479/how-to-properly-retry-a-serializable-transaction-with-spring.


I am developing against a PostgreSQL v12 database. I am using SERIALIZABLE transactions. The general idea is that when PostgreSQL detects a serialization anomaly, one should retry the complete transaction.

I am using Spring's AbstractFallbackSQLExceptionTranslator to translate database exceptions to Spring's exception classes. This exception translator should translate the PostgreSQL error 40001/serialization_failure to a ConcurrencyFailureException. Spring JDBC maintains a mapping file to map the PostgreSQL-specific code 40001 to a generic cannotSerializeTransactionCodes class of database exceptions, which translates into a ConcurrencyFailureException for the API user.

My idea was to rely on the Spring Retry project to retry a SERIALIZABLE transaction which is halted due to a serialization error as following:

@Target({ElementType.METHOD, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Documented
@Retryable(include = ConcurrencyFailureException.class, maxAttempts = ..., backoff = ...)
@Transactional(isolation = Isolation.SERIALIZABLE)
public @interface SerializableTransactionRetry {
}

In service implementation, I would simply replace @Transactional by @SerializableTransactionRetry and be done with it.

Now, back to PostgreSQL. Essentially, there are two stages at which a serialization anomaly can be detected:

  1. during the execution of a statement
  2. during the commit phase of a transaction

It seems that Spring's AbstractFallbackSQLExceptionTranslator is properly translating a serialization anomaly which is detected during the execution of a statement, but fails to translate one during the commit phase. Consider the following stack trace:

org.springframework.transaction.TransactionSystemException: Could not commit JDBC transaction; nested exception is 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.springframework.jdbc.datasource.DataSourceTransactionManager.doCommit(DataSourceTransactionManager.java:332)
	at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:746)
	at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:714)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:533)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:304)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:98)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.retry.interceptor.RetryOperationsInterceptor$1.doWithRetry(RetryOperationsInterceptor.java:91)
	at org.springframework.retry.support.RetryTemplate.doExecute(RetryTemplate.java:287)
	at org.springframework.retry.support.RetryTemplate.execute(RetryTemplate.java:164)
	at org.springframework.retry.interceptor.RetryOperationsInterceptor.invoke(RetryOperationsInterceptor.java:118)
	at org.springframework.retry.annotation.AnnotationAwareRetryOperationsInterceptor.invoke(AnnotationAwareRetryOperationsInterceptor.java:153)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:688)

As you can see, PostgreSQL detects a serialization anomaly (ERROR: could not serialize access due to ...), but this is translated by Spring into a TransactionSystemException instead of a ConcurrencyFailureException.

I could alter the SerializableTransactionRetry annotation above to include a TransactionSystemException as well, but I believe that would be wrong, as now we will be retrying upon any kind of transaction error, which is not what we want here.

Is this a shortcoming in Spring's AbstractFallbackSQLExceptionTranslator? I am using Spring 5.2.1.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 23, 2019
@jhoeller jhoeller changed the title AbstractFallbackSQLExceptionTranslator fails to translate a serialization violation happening during commit phase DataSourceTransactionManager does not consider SQLExceptionTranslator on commit Nov 23, 2019
@jhoeller jhoeller self-assigned this Nov 23, 2019
@jhoeller jhoeller added in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 23, 2019
@jhoeller
Copy link
Contributor

jhoeller commented Nov 23, 2019

DataSourceTransactionManager has not been designed to translate a commit-level SQLException in a fine-grained fashion. Good point, we could evaluate SQLExceptionTranslator before falling back to a TransactionSystemException there, similar to how we apply exception conversion for Hibernate flush exceptions in HibernateTransactionManager. The difference is that Hibernate/JPA flushes often involve insert/update/delete statements sent to the database at commit time, with the actual resource commit just being a formal completion step, whereas a JDBC commit is just the technical completion step... but might still involve some pre-commit constraint validation, admittedly.

So we could introduce a configurable SQLExceptionTranslator on DataSourceTransactionManager but we'd have to make sure that there are no common regressions, in particular not fallback UncategorizedSQLException thrown to commit callers that expect a TransactionSystemException. This will involve a different entry point into SQLExceptionTranslator which just performs rule-driven translation without any fallback rules, leaving the rest up to DataSourceTransactionManager.

@jhoeller jhoeller added this to the 5.3 M1 milestone Nov 23, 2019
@pbillen
Copy link
Author

pbillen commented Nov 23, 2019

@jhoeller Thank you for your insightful feedback. I did not really realize that the translation manager is actually not used for commit-level exceptions.

Your suggestion makes perfect sense. Thanks for adding it to the 5.3 roadmap!

@jhoeller
Copy link
Contributor

jhoeller commented May 6, 2020

I'm introducing a separate JdbcTransactionManager class as a subclass of DataSourceTransactionManager, integrating with the common infrastructure in the jdbc.support package (in particular using an explicitly configured or lazily initialized SQLExceptionTranslator for commit/rollback exceptions) and therefore also living there itself (for package interdependency reasons). This is intended to go with JdbcTemplate and related accessors, matching their behavior.

As a part of this change, our default SQLExceptionTranslator implementations do not throw a fallback UncategorizedSQLException anymore but rather leave this up to the caller (as intended by the SQLExceptionTranslator interface definition and as explicitly handled by all internal callers already).

@lukaseder
Copy link

As a part of this change, our default SQLExceptionTranslator implementations do not throw a fallback UncategorizedSQLException anymore but rather leave this up to the caller (as intended by the SQLExceptionTranslator interface definition and as explicitly handled by all internal callers already).

I wonder if there could have been another solution that didn't involve breaking the contract on the SQLExceptionTranslator::translate method, which some third parties may have depended upon, so far:

This UncategorizedSQLException could still be returned from the method, but recognised by your default implementations. The interesting bit here is that the translator "dishes it out", but "cannot take it" 😉:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants