search

atomikos / transactions-essentials

Development repository for next major release of
https://www.atomikos.com/Main/TransactionsEssentials
Other
461 stars 139 forks source link

Transitive ReadOnly calls lead to error #167

Open aubelix opened 2 years ago

aubelix commented 2 years ago

We have 3 seperated services. If readOnly calls are done from Services 1 and Service 2 to Service 3 we get an error in the prepare phase. Service 1 wants to do the prepare and because of the 'allReadOnly' logic in ActiveStateHandler the Coordinator is removed in TransactionServiceImp.removeCoordinator(). When Service 2 wants to do prepare() on Service 3, the Coordinator is missing and TransactionServiceImp.getCoordinatorImpForRoot() will not find the coordinator. Tested with 5.0.9

Testcase ist available in https://github.com/aubelix/transactions-over-rest-with-hybrid-jaxrs-stack Branch: 'transitive' Please start the two applications in jaxrs-hibernate and jaxrs-hibernate2 Then start SampleRestClientApplication

Testcase ist implemented in com.atomikos.jaxrshibernateclient.service.PaymentService.testTransitiveReadOnlyCalls()

A fix could be to add a Map called readOnlyPreparedCoordinatirsByCoordinatorId in TransactionServiceImp#removeCoordinator() so we can answer the second prepare() call. But how should the cleanup be done?

GuyPardon commented 2 years ago

The best solution would probably be to disable the readOnly optimisation in this architecture.

There are two ways of doing so:

I think the second option is better, but it does require more documentation and has the risk of confusing people who run into this issue.

A better way is probably to disable readOnly for all remoting by default, but allow overriding it via a configuration property.

Your thoughts?

GuyPardon commented 2 years ago

FYI in the mean time we fixed this with a custom (disabling) property to switch off readOnly behaviour in the commercial code base.

aubelix commented 2 years ago

I think the best solution is to do the orchestration of prepare/commit/rollback only in the initiator. Example: Service A calls B, B calls C, and A calls B. The Initiator (start of transaction) is A. All information is propagated to A by the Atomikos-Extent. So A knows, that B and C are part of it's transaction. The orchestraction of the remote 2PC should therefore only be done by the initiator A. Then we have no problem of double prepares or commits. This is also a step to a more "stateless" solution, For Cloud deployments like Kubernetes it would be the best to have all required state in the initiator and keep the service stateless. I know this is a far way, but it is possible if you use a Database that supports full XA, so that the connection can be given back to the pool and can be resumed later.

GuyPardon commented 1 year ago

For the record: this has been fixed by avoiding the readOnly optimisation in this specific type of scenario. No extra property needed, it is detected automatically in our 6.0 release.