spring-projects / spring-modulith

Modular applications with Spring Boot
https://spring.io/projects/spring-modulith
Apache License 2.0
823 stars 143 forks source link

Marking event as completed may fail when used with `ChainedTransactionManager` #886

Open maciejwalkowiak opened 1 month ago

maciejwalkowiak commented 1 month ago

I run into a following issue:

Project has a little unusual setup:

The code looks more or less like this:

@Service
public class BarService {
    private final FooService fooService;

    @Transactional(transactionManager = "chainedTransactionManager")
    void bar() {
        fooService.foo();
    }
}

@Service
public class FooService {
    private final ApplicationEventPublisher eventPublisher;

    @Transactional
    void foo() {
        SomethingHappened event = new SomethingHappened(UUID.randomUUID().toString());
        eventPublisher.publishEvent(event);
    }
}

@Component
public class FooListener {

    @ApplicationModuleListener
    void handle(SomethingHappened event) {
        LOGGER.info("Handling: {}", event);
    }
}

When barService.bar() is called, in logs I can see that handling of the event in the listener may happen after FooService#foo method finishes and before BarService#bar finishes, but the event publication is inserted after BarService#bar method finishes execution. So there is a chance, that marking event as completed happens before the event publication is inserted.

Sample that reproduces this issue: https://github.com/maciej-scratches/modulith-chainedtransaction-manager-issue/blob/main/src/test/java/org/example/FooServiceTest.java#L43

Due to asynchronous nature, it's not easy to reproduce. When test runs 100 times, on my machine usually fails once or twice.

Considering that ChainedTransactionManager is deprecated perhaps this issue is irrelevant, but I am raising it as it maybe affects also other arrangements I am not aware of - if this is not the case - feel free to close it.

odrotbohm commented 1 month ago

I'll have to look into the example for specifics, but generally speaking, ChainedTransactionManager is not something we support explicitly. In fact, it's been previously used to achieve orchestration similar to what we now achieve with the event publication registry. I guess what I'd recommend in a scenario like this (two DataSources I assume) is to commit on the first and have an @ApplicationModuleListener overriding the @Transactional to the second transaction manager.