alex-sira commented 1 day ago

Bug with the transaction management in spring-data-jpa.

I'm using spring-boot-starter-parent 3.3.5. The bug is: when an exception is thrown in a method inside a transaction, is not possible to stop it with a catch clause, but it will be propogate and will resolve in an UnexpectedRollbackException out of this transaction.

I'll try to explain it better: this is the stack-trace of my Exception:

org.springframework.transaction.UnexpectedRollbackException: Transaction silently rolled back because it has been marked as rollback-only
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:804) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:758) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:663) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:413) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:768) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:720) ~[spring-aop-6.1.14.jar:6.1.14]
    at it.alessandro.test.transaction_propagation_test.service.ValidationNameService$$SpringCGLIB$$0.isValidName(<generated>) ~[classes/:na]
    at it.alessandro.test.transaction_propagation_test.service.CreateMyNameEntityService.createName(CreateMyNameEntityService.java:31) ~[classes/:na]
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) ~[na:na]
    at java.base/java.lang.reflect.Method.invoke(Method.java:580) ~[na:na]
    at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:355) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:196) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:768) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:379) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119) ~[spring-tx-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:768) ~[spring-aop-6.1.14.jar:6.1.14]
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:720) ~[spring-aop-6.1.14.jar:6.1.14]
    at it.alessandro.test.transaction_propagation_test.service.CreateMyNameEntityService$$SpringCGLIB$$0.createName(<generated>) ~[classes/:na]
    at it.alessandro.test.transaction_propagation_test.scheduler.MyNameEntityScheduler.saveEntity(MyNameEntityScheduler.java:22) ~[classes/:na]
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) ~[na:na]
    at java.base/java.lang.reflect.Method.invoke(Method.java:580) ~[na:na]
    at org.springframework.scheduling.support.ScheduledMethodRunnable.runInternal(ScheduledMethodRunnable.java:130) ~[spring-context-6.1.14.jar:6.1.14]
    at org.springframework.scheduling.support.ScheduledMethodRunnable.lambda$run$2(ScheduledMethodRunnable.java:124) ~[spring-context-6.1.14.jar:6.1.14]
    at io.micrometer.observation.Observation.observe(Observation.java:499) ~[micrometer-observation-1.13.6.jar:1.13.6]
    at org.springframework.scheduling.support.ScheduledMethodRunnable.run(ScheduledMethodRunnable.java:124) ~[spring-context-6.1.14.jar:6.1.14]
    at org.springframework.scheduling.support.DelegatingErrorHandlingRunnable.run(DelegatingErrorHandlingRunnable.java:54) ~[spring-context-6.1.14.jar:6.1.14]
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.runAndReset$$$capture(FutureTask.java:358) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java) ~[na:na]
    at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[na:na]
    at java.base/java.lang.Thread.run(Thread.java:1570) ~[na:na]

I created an application to reproduce it:

https://github.com/alex-sira/transaction-propagation-test

Explanation of the issue

While in the transaction opened by the upper method

it.alessandro.test.transaction_propagation_test.service.CreateMyNameEntityService#createName

the lower methods should save two entities:

MyNumberEntity: this is a clean save, without errors
MyNameEntity: this is an Entity with a unique constraint. Every time this is violated the while loop will catch the exception and retry to save it with a different name.

The issue is that the exception thrown by the database in the method

it.alessandro.test.transaction_propagation_test.service.ValidationNameService#isValidName

is a DataIntegrityViolation that must be catch in the try-catch block, but it is propagated in the upper method:

it.alessandro.test.transaction_propagation_test.service.CreateMyNameEntityService#createName

causing the rollback of the transaction with the exception:

org.springframework.transaction.UnexpectedRollbackException: Transaction silently rolled back because it has been marked as rollback-only

These are my classes:

Entities and Repositories:

@Entity
@Table(name="MY_NAME_ENTITY")
@Data
public class MyNameEntity {
    @Id
    @GeneratedValue(strategy=GenerationType.IDENTITY)
    @Column(name="id")
    private Long id;
    @Column(name="name", unique=true)
    private String name;
    @Column(name="schedulation_number")
    private int schedulationNumber;
}

@Entity
@Table(name="MY_NUMBER_ENTITY")
@Data
public class MyNumberEntity {
    @Id
    @GeneratedValue(strategy=GenerationType.IDENTITY)
    @Column(name="id")
    private Long id;
    @Column(name="schedulation_number")
    private int schedulationNumber;
}

@Repository
public interface MyNameEntityRepository extends JpaRepository<MyNameEntity,Long> {
}

@Repository
public interface MyNumberEntityRepository extends JpaRepository<MyNumberEntity,Long> {
}

Scheduler

@Service
@Slf4j
@RequiredArgsConstructor
@ConditionalOnProperty(value="scheduler.name.enabled", matchIfMissing=true)
public class MyNameEntityScheduler {
    private int schedulationOccurrence = 0;
    private final CreateMyNameEntityService createMyNameEntityService;

    @Scheduled(initialDelay=10000, fixedDelay=20000)
    public void saveEntity() {
        log.info("Start schedulation number: [{}]", schedulationOccurrence);
        try {
            createMyNameEntityService.createName(schedulationOccurrence);
        } catch (Exception e) {
            log.error("Exception during the schedulation: [{}]", schedulationOccurrence, e);
        } finally {
            log.info("End schedulation number: [{}]", schedulationOccurrence++);
        }
    }
}

Upper transactional service:

@Service
@Slf4j
@RequiredArgsConstructor
public class CreateMyNameEntityService {

    private final MyNumberEntityService myNumberEntityService;
    private final ValidationNameService validationNameService;

    @Transactional
    public void createName(int schedulatioNumber) throws InterruptedException {
        MyNumberEntity myNumberEntity = new MyNumberEntity();
        myNumberEntity.setSchedulationNumber(schedulatioNumber);
        myNumberEntityService.save(myNumberEntity);
        boolean isValid;
        int attempt = 0;
        do {
            attempt++;
            String name = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmm"));
            isValid = validationNameService.isValidName(name, schedulatioNumber);

            if (isValid) {
                break;
            }
            Thread.sleep(20000);
        } while (attempt < 10);
    }
}

First service that will save the entity without constraint

@Service
@Slf4j
@RequiredArgsConstructor
public class MyNumberEntityService {

    private final MyNumberEntityRepository myNumberEntityRepository;

    public MyNumberEntity save(MyNumberEntity myNumberEntity) {
        log.info("Saving MyNumberEntity with number: [{}]", myNumberEntity.getSchedulationNumber());
        myNumberEntity.setId(null);
        myNumberEntityRepository.save(myNumberEntity);
        log.info("Saved MyNumberEntity with id: [{}]", myNumberEntity.getId());
        return myNumberEntity;
    }
}

Service that should try the save operation until it works:

@Service
@Slf4j
@RequiredArgsConstructor
public class ValidationNameService {

    private final MyNameEntityService myNameEntityService;

    @Transactional(propagation=Propagation.REQUIRES_NEW)
    public boolean isValidName(String name, int schedulationNumber) {
        boolean isValid = true;
        try {
            myNameEntityService.save(name, schedulationNumber);
        } catch (Exception e) {
            log.warn("Name: [{}] cannot be used because it already exists", name);
            isValid = false;
        }
        return isValid;
    }
}

@Service
@Slf4j
@RequiredArgsConstructor
public class MyNameEntityService {

    private final MyNameEntityRepository myNameEntityRepository;

    public MyNameEntity save(String name, int schedulationNumber) {
        log.info("Saving MyNameEntity with name: [{}]", name);
        MyNameEntity myNameEntity = new MyNameEntity();
        myNameEntity.setName(name);
        myNameEntity.setSchedulationNumber(schedulationNumber);
        myNameEntityRepository.save(myNameEntity);
        log.info("Saved MyNameEntity with id: [{}]", myNameEntity.getId());
        return myNameEntity;
    }
}

This is the pom.xml:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <parent>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-parent</artifactId>
        <version>3.3.5</version>
        <relativePath/> <!-- lookup parent from repository -->
    </parent>
    <groupId>it.alessandro.test</groupId>
    <artifactId>transaction-propagation-test</artifactId>
    <version>0.0.1-SNAPSHOT</version>
    <name>transaction-propagation-test</name>
    <description>Demo project for Spring Boot</description>
    <url/>
    <licenses>
        <license/>
    </licenses>
    <developers>
        <developer/>
    </developers>
    <scm>
        <connection/>
        <developerConnection/>
        <tag/>
        <url/>
    </scm>
    <properties>
        <java.version>17</java.version>
    </properties>
    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-data-jpa</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-web</artifactId>
        </dependency>

        <dependency>
            <groupId>org.projectlombok</groupId>
            <artifactId>lombok</artifactId>
            <optional>true</optional>
            <version>1.18.36</version>
        </dependency>

        <!-- https://mvnrepository.com/artifact/com.h2database/h2 -->
        <dependency>
            <groupId>com.h2database</groupId>
            <artifactId>h2</artifactId>
            <version>2.3.232</version>
        </dependency>

        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-test</artifactId>
            <scope>test</scope>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.springframework.boot</groupId>
                <artifactId>spring-boot-maven-plugin</artifactId>
                <configuration>
                    <excludes>
                        <exclude>
                            <groupId>org.projectlombok</groupId>
                            <artifactId>lombok</artifactId>
                        </exclude>
                    </excludes>
                </configuration>
            </plugin>
        </plugins>
    </build>

</project>

wilkinsona commented 1 day ago

The constraint violation causes the transaction in which isValidName is running to be marked as rollback-only. When the transaction manager tries to commit the transaction it notes that it's rollback-only and throws an UnexpectedRollbackException. Your code that's calling isValidName isn't in a try-catch block so this exception reaches the transaction handling for createName. The exception's unexpected so that transaction is rolled back as well.

You can avoid the problem by calling isValidName defensively, for example:

    @Transactional
    public void createName(int schedulatioNumber) throws InterruptedException {
        MyNumberEntity myNumberEntity = new MyNumberEntity();
        myNumberEntity.setSchedulationNumber(schedulatioNumber);
        myNumberEntityService.save(myNumberEntity);
        boolean isValid = false;
        int attempt = 0;
        do {
            attempt++;
            String name = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmm"));
            try {
                isValid = validationNameService.isValidName(name, schedulatioNumber);
            }
            catch (Exception ex) {
                // Continue
            }
            if (isValid) {
                break;
            }
            Thread.sleep(20000);
        } while (attempt < 10);
    }

If you have any further questions, please follow up on Stack Overflow. As mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements.

alex-sira commented 1 day ago

The constraint violation causes the transaction in which isValidName is running to be marked as rollback-only. When the transaction manager tries to commit the transaction it notes that it's rollback-only and throws an UnexpectedRollbackException. Your code that's calling isValidName isn't in a try-catch block so this exception reaches the transaction handling for createName. The exception's unexpected so that transaction is rolled back as well.

You can avoid the problem by calling isValidName defensively, for example:
  @Transactional
  public void createName(int schedulatioNumber) throws InterruptedException {
      MyNumberEntity myNumberEntity = new MyNumberEntity();
      myNumberEntity.setSchedulationNumber(schedulatioNumber);
      myNumberEntityService.save(myNumberEntity);
      boolean isValid = false;
      int attempt = 0;
      do {
          attempt++;
          String name = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmm"));
          try {
              isValid = validationNameService.isValidName(name, schedulatioNumber);
          }
          catch (Exception ex) {
              // Continue
          }
          if (isValid) {
              break;
          }
          Thread.sleep(20000);
      } while (attempt < 10);
  }
If you have any further questions, please follow up on Stack Overflow. As mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements.

Thank you @wilkinsona for your response, but I still think this is a bug in the transaction management of JPA.

Yes, I already tried your solution because I understood that the UnexpectedRollbackException is thrown by the proxy after the end of the method isValidName and in this way it works correctly.

The issue is that the transaction in the isValidName must not be marked as rollback, because the transaction starts in this method, with the REQUIRED_NEW, and the exception that has to rollback it is not propagated out of the method with the @Transactional annotation because I have catched it inside the method isValidName and suppressed it

wilkinsona commented 1 day ago

because I have catched it inside the method isValidName and suppressed it

That doesn't matter. When a ConstraintViolationException is thrown, Hibernate will mark the transaction as rollback-only. This happens before your code has a chance to catch the exception.

nosan commented 1 day ago

@alex-sira If you make the following changes:

@Service
@Slf4j
@RequiredArgsConstructor
public class ValidationNameService {

    private final MyNameEntityService myNameEntityService;

    public boolean isValidName(String name, int schedulationNumber) {
        boolean isValid = true;
        try {
            myNameEntityService.save(name, schedulationNumber);
        } catch (Exception e) {
            log.warn("Name: [{}] cannot be used because it already exists", name);
            isValid = false;
        }
        return isValid;
    }

}

@Service
@Slf4j
@RequiredArgsConstructor
public class MyNameEntityService {

    private final MyNameEntityRepository myNameEntityRepository;

    @Transactional(propagation = Propagation.REQUIRES_NEW)
    public MyNameEntity save(String name, int schedulationNumber) {
        log.info("Saving MyNameEntity with name: [{}]", name);
        MyNameEntity myNameEntity = new MyNameEntity();
        myNameEntity.setName(name);
        myNameEntity.setSchedulationNumber(schedulationNumber);
        myNameEntityRepository.save(myNameEntity);
        log.info("Saved MyNameEntity with id: [{}]", myNameEntity.getId());
        return myNameEntity;
    }

}

UnexpectedRollbackException will not be thrown anymore.

spring-projects / spring-boot

Try-catch don't block Transaction Rollback: UnexpectedRollbackException in each case an exception is thrown and catch in a transaction #43228