spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.55k stars 38.13k forks source link

Can not rollback transaction in Mono.zip(...) when use several reactive transaction managers #28782

Closed RomanPilyutik closed 1 month ago

RomanPilyutik commented 2 years ago

Currently in project we have separate configured r2dbc connection factories to different schemas in db. And have several reactive transaction managers.

We met the next issue: with an open transaction through the first transaction manager, we execute Mono.zip to merge two streams. First one work with first db shema (in opened transaction), the second one - with another db schema and open separate transaction through the second transaction manager. And in case of any runtime exception in first stream we get

Expected Behavior:

rollback both transactions

Actual Behavior:

Second transaction rollbacked, but the first one throw exception on rollback: java.lang.IllegalStateException: Transaction synchronization is not active.

Reason:

Seems like as on exception in the first stream from Mono.zip, the second stream is canceled and clears synchronizations in common TransactionContext. As synchronizations is null then first transaction is failed on rollback.

Example to reproduce can be found here https://github.com/RomanPilyutik/spring-tx-issue. Just run SynchronizaionsApplicationTests.

org.springframework.boot: 2.6.3 kotlin: 1.6.0 reactor-core 3.4.14

RomanPilyutik commented 2 years ago

Added another test that reproduce the same issue while using only one reactive transaction manager

snicoll commented 11 months ago

@RomanPilyutik thanks for the sample but can you please simplify the sample to reduce the number of custom configuration and rely on auto-configurations rather than custom configuration. The sample also had a bunch of deprecated API usage and I couldn't start it:

Container 5950bef5e72d3e4ed469df6e6ffb0fd8ac8b71a25d498786ef2d88a21cea7a2a of 7da691d53e1c7bcefa49ad6cdabbea2b_synchronizaions_-test_db-1 is not running nor restarting. Logs:
2023-11-27 16:52:02+00:00 [Note] [Entrypoint]: Entrypoint script for MySQL Server 8.2.0-1.el8 started.
2023-11-27 16:52:03+00:00 [Note] [Entrypoint]: Switching to dedicated user 'mysql'
2023-11-27 16:52:03+00:00 [Note] [Entrypoint]: Entrypoint script for MySQL Server 8.2.0-1.el8 started.
2023-11-27 16:52:03+00:00 [Note] [Entrypoint]: Initializing database files
2023-11-27 16:52:06+00:00 [Note] [Entrypoint]: Database files initialized
2023-11-27 16:52:06+00:00 [Note] [Entrypoint]: Starting temporary server
2023-11-27 16:52:07+00:00 [Note] [Entrypoint]: Temporary server started.
'/var/lib/mysql/mysql.sock' -> '/var/run/mysqld/mysqld.sock'

2023-11-27 16:52:08+00:00 [Note] [Entrypoint]: /usr/local/bin/docker-entrypoint.sh: running /docker-entrypoint-initdb.d/create-user.sh

Thanks.

RomanPilyutik commented 11 months ago

Hi @snicoll. Just created another branch where I left only one r2dbc auto-configuration https://github.com/RomanPilyutik/spring-tx-issue/tree/test_with_single_auto_r2dbc_config

And it's weird that you couldn't start the test. I rechecked right now and test is runnable. Just ran the command ./gradlew test. Could you please try to run it again on the new branch that I provided above

snicoll commented 10 months ago

Thanks for following up, but It still does not work for me, the container won't start. I am on ARM so it might be related to the mysql image not supporting it.

I am not a Kotlin expert, so I can't be sure that what I am seeing is idiomatic but it looks really complicated, especially the find that does an insert as a new transaction in that flatMap there. And the new example doesn't have a rollback either as far as I can see.

I am afraid it's still quite confusing, if you could trim down the example again, perhaps someone with more Kotlin knowledge could have a look.

spring-projects-issues commented 10 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

RomanPilyutik commented 10 months ago

I updated mysql image and pushed to https://github.com/RomanPilyutik/spring-tx-issue/tree/test_with_single_auto_r2dbc_config. Now my test should be runnable on your machine.

Let me describe the problem in updated test: In general nested transactions behave unpredictably. In my test through Mono.zip I'm trying to insert entity_2 in the new nested transaction and find entity_1 in the scope of parent transaction. When I can't find entity_1 I got error EntityNotFoundException from DB repository. In described case I expect the rollback of both transactions and catch my EntityNotFoundException in test but instead of that:

jhoeller commented 9 months ago

As a general note: For reactive transactions, there is only a single TransactionContext in the pipeline which is to be managed by a single transaction manager. Any interleaved use is bound to leave inconsistent state - but with a single transaction manager, it should actually be able to detect and react to the existing TransactionContext, participating in the outer transaction including a global rollback at that level.

simonbasle commented 9 months ago

I tried to investigate this issue but I couldn't (yet) get to the bottom of it. However I do think there might be something missing in the cancellation path of synchronizing reactive transactions.

Pending further progress, as a potential workaround can you try replacing the Mono.zip with a Mono.zipDelayError and confirm to us both outer and inner transactions behave as expected?

spring-projects-issues commented 8 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

RomanPilyutik commented 8 months ago

Let me provide here the code from my example

testRepository2.findByIdForUpdate(id)
            .flatMap {
                Mono.zip(
                    testRepository2.insert(Test2Entity(UUID.randomUUID(), "TEST1", LocalDateTime.now())) //Stream1
                        .flatMap {
                            testRepository.findById(UUID.randomUUID()) //not found error
                        },
                    testRepository2.insert(Test2Entity(UUID.randomUUID(), "TEST2", LocalDateTime.now())) //Stream2
                        .`as`(transactionOperator::inNewTransaction)
                )
            }
            .`as`(transactionOperator::inTransaction)
            .then() 

Since we have a race condition then we can get next 3 situations:

If I use Mono.zipDelayError in my example then I'll always get the third result (described above).

simonbasle commented 2 months ago

This sounds like zipDelayError would at least prevent the case were the connection stays open. Does it lead to a result that is acceptable / leaves your system in a consistent state ?

spring-projects-issues commented 2 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

RomanPilyutik commented 2 months ago

Yes, at least zipDelayError provide consistent behavior/

simonbasle commented 1 month ago

Given the complexity of the investigation and the lack of a clear root cause and avenue of fixing, I'm closing this issue with the recommendation to use the zipDelayError workaround.