spring-projects / spring-data-relational

Spring Data Relational. Home of Spring Data JDBC and Spring Data R2DBC.
https://spring.io/projects/spring-data-jdbc
Apache License 2.0
764 stars 344 forks source link

`@Transactional` SimpleR2dbcRepository methods do not roll back on `then(Mono.error(…))` #1786

Closed anaconda875 closed 5 months ago

anaconda875 commented 5 months ago

Given

@Builder
@Data
public class Person {
    @Id
    private Long id;
    private String name;
}
@Repository
public interface PersonRepository extends R2dbcRepository<Person, Long> {

    Mono<Person> findById(String id);

    @Query("UPDATE person SET name = 'tgbn'")
    @Modifying
    @Transactional
    Mono<Integer> updateAc();
}
@RestController
public class PersonReactiveController {

    @Autowired
    private PersonRepository personRepository;

    @GetMapping(path = "/people", produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
//  @Transactional
    public Mono<Person> persons() {
        Mono<Person> generated = personRepository.save(Person.builder().name("generated").build());
        return generated.flatMap(i -> Mono.error(new RuntimeException("rrrrrrrrrr")));
    }

    @PutMapping(path = "/update")
//  @Transactional
    public Mono<Person> update() {
        return personRepository.updateAc().then(Mono.error(new RuntimeException("ttttttttt")));
    }
}

In controller methods, I throw RuntimeException via Mono.error(). Expected:

  1. Spring should rollback personRepository.save() operation (since SimpleR2dbcRepository.save() is annotated by @Transactional)
  2. Spring should rollback personRepository.updateAc() operation (since it is annotated by @Transactional, too)

Actual: No rollback is done (I verified carefully) Bonus:

  1. If I add @Transactional to controller methods, rollbacks are performed.
  2. Tested with Spring boot 2.4.4 and 3.2.0 (r2dbc-postgresql, spring-boot-starter-webflux, spring-boot-starter-data-r2dbc, spring-boot-starter-actuator), still observed the same behaviors.

Is it a bug or a limitation of Spring r2dbc/reactor?

mp911de commented 5 months ago

This is correct and as designed behavior. Let's take a closer look at what happens here:

personRepository.updateAc().then(Mono.error(new RuntimeException("ttttttttt")))

Without an outer @Transactional on the controller method, a transaction starts when a subscriber subscribes to personRepository.updateAc(). The transaction runs until the resulting Mono emits a completion signal. After completion (and therefore after the transaction is committed) Mono.error(…) is being executed. The error signal is emitted outside of the transaction and so it does not affect transactional cleanup.

When you in contrast annotate your controller methods with @Transactional, then the controller method body already is part of the transaction.

One additional note: A Mono behaves pretty similar to CompletableFuture. Emission of a data signal already completes the Mono without a need to emit an additional completion signal. That is the reason why flatMap(i -> Mono.error(…))) is running after transaction completion.

Let me know whether that makes sense to you.

anaconda875 commented 5 months ago

If annotating @Transactional on Repository itself cannot do rollback, what is the purpose of putting it on SimpleR2dbcRepository?

mp911de commented 5 months ago

We apply a read-only optimization via @Transactional(readOnly = true) to allow transaction managers and drivers participate in read-only transactions to reduce overhead.

Also, there are several methods that run multiple SQL statements such as deleteById(Publisher<ID> idPublisher) that we want to keep within the same transaction.

I'm closing the ticket since the original question has been answered.

anaconda875 commented 5 months ago

I tried to add debug-breakpoints to TransactionInterceptor and found that those code was not executed even I had called save or deleteById(Publisher<ID> idPublisher). Did I miss something?