Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enlisting multiple non-xa datasources within a JTA transaction succeeds with local commits to each resource #83

Closed
Mobe91 opened this issue Apr 7, 2019 · 20 comments

Comments

Projects
None yet
2 participants
@Mobe91
Copy link

commented Apr 7, 2019

I am using Atomikos with Spring and 2 Non-XA datasources. I have activated JTA transactions on my JPA configuration.
My expectation is that the Atomikos JTA transaction manager will throw an error when more than 1 Non-XA datasource is enlisted with a JTA transaction but instead, the two datasources are committed independently. Is this behavior somehow configurable? Or maybe I am misunderstanding something. Could not find anything in the docs.

@Mobe91 Mobe91 changed the title Enlisting multiple non-xa datasources withina JTA transaction suceeds with local commits to each resource Enlisting multiple non-xa datasources withina JTA transaction succeeds with local commits to each resource Apr 7, 2019

@Mobe91 Mobe91 changed the title Enlisting multiple non-xa datasources withina JTA transaction succeeds with local commits to each resource Enlisting multiple non-xa datasources within a JTA transaction succeeds with local commits to each resource Apr 7, 2019

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

Do you mean the AtomikosNonXADataSourceBean?

It allows commit, since people typically use it for the following:

  • read-only queries on static databases as part of a JTA transaction (avoids the XA setup of the DBMS)
  • JTA transactions with only one resource
    -in rare cases where the backend is not XA compliant, people also use it (but at risk)

You' re right, we might be a bit more explicit with warnings on two-phase commit. What do you think?

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 8, 2019

Do you mean the AtomikosNonXADataSourceBean?

Yes

I am using non-xa datasource because I actually want to avoid 2PC - I basically just want to have different connection pools for different parts of my application for isolation purposes and with JTA I can avoid having to deal with multiple transaction managers.

What I want to specifically avoid is that an application developer (accidentally or deliberately) uses two non-xa resources in a transaction and expects an atomic commit to happen - currently it looks like that if you do not take a closer look. I would like to be able to configure at least a warning to be logged in this situation if not a rollback of the JTA transaction in progress.

As far as I know, a Java EE environment would produce an exception and rollback in such a situation.

read-only queries on static databases as part of a JTA transaction

This actually sounds useful, maybe it would be ok to allow multiple non-xa resources as long as at most one of them is writeable (again, configurable).

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 8, 2019

Here is a thread for Wildfly AS that talks about this topic: https://developer.jboss.org/thread/183198
They also have a property to allow JTA transactions with multiple non-xa resources (implying non-atomic commits) but it is disabled by default.

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

OK thanks. I just committed a change to make prepare fail for non-xa. This can be overridden by setting the datasource to "readOnly" - where it is legitimate. What do you think?

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

I am using non-xa datasource because I actually want to avoid 2PC - I basically just want to have different connection pools for different parts of my application for isolation purposes and with JTA I can avoid having to deal with multiple transaction managers.

Not sure if I understand this point, but it sounds interesting. Can you clarify?

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 8, 2019

OK thanks. I just committed a change to make prepare fail for non-xa. This can be overridden by setting the datasource to "readOnly" - where it is legitimate. What do you think?

Can I see the change somewhere? At least I don't see it on GitHub 😄
But it sounds fine. Oh and I guess as long as the JTA transaction is read only, the type of the datasources should not matter, right?

Not sure if I understand this point, but it sounds interesting. Can you clarify?

If I used XA datasources then a 2PC would be carried out when multiple datasources participate in the transaction - this is slow and also unnecessary in my case because all datasources actually point to the same database. My backend serves 2 different frontend applications with different categories of users. I want to be able to apply different db connection settings for these applications and also I do not want potential connection leakage in one application to affect the users of the other application.

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

Can I see the change somewhere? At least I don't see it on GitHub

I was waiting for the tests to finish. Now it's on github.

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@Mobe91 if you have time for a quick review then I'd be interested in your thoughts - thanks

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 8, 2019

I ran some tests, looks good. However, in case of a readOnly transaction with writeable datasources an exception is thrown as well - shouldn't this be allowed?

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

What do you mean by a "readOnly transaction with writeable datasources"?

Thanks

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 9, 2019

In Spring, a transaction can be marked as read-only independent of the underlying datasource. E.g. Spring integrates this flag with Hibernate in org.springframework.orm.hibernate5.SpringJtaSessionContext to prevent auto-flushing for read-only transactions. But yeah.. this is merely a hint. Also, the flag is not even forwarded to the underlying JTA infrastructure in org.springframework.transaction.jta.JtaTransactionManager#doJtaBegin so I guess it is irrelevant.

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Well, it is a configuration / design decision to make the datasource "readOnly" by setting the corresponding property. Then it will not throw. Other uses are not really safe with respect to 2-phase commit so I don't see what else we can do?

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 9, 2019

Yeah that is also what I think - so IMHO it is fine with the change you made! 👍
Thanks for the quick fix :) when is the release scheduled that will contain your change?

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 9, 2019

@GuyPardon
I now did a test that involves a readOnly + a non-readOnly datasource. In this case, I also receive an exception during prepare:

Caused by: javax.transaction.RollbackException: Prepare failed because one or more resources refused to commit. This transaction has been rolled back instead. The cause could be either:
1. a transaction timeout (in which case you should see additional timeout warnings in this log file), or
2. inability to reach the resource (in which case you should see network errors), or
3. a resource-internal cause that we can’t inspect
	at com.atomikos.icatch.jta.TransactionImp.rethrowAsJtaRollbackException(TransactionImp.java:47)
	at com.atomikos.icatch.jta.TransactionImp.commit(TransactionImp.java:185)
	at com.atomikos.icatch.jta.TransactionManagerImp.commit(TransactionManagerImp.java:424)
	at com.atomikos.icatch.jta.UserTransactionManager.commit(UserTransactionManager.java:159)
	at org.springframework.transaction.jta.JtaTransactionManager.doCommit(JtaTransactionManager.java:1034)
	... 121 common frames omitted
Caused by: com.atomikos.icatch.RollbackException: Prepare failed because one or more resources refused to commit. This transaction has been rolled back instead. The cause could be either:
1. a transaction timeout (in which case you should see additional timeout warnings in this log file), or
2. inability to reach the resource (in which case you should see network errors), or
3. a resource-internal cause that we can’t inspect
	at com.atomikos.icatch.imp.ActiveStateHandler.prepare(ActiveStateHandler.java:206)
	at com.atomikos.icatch.imp.CoordinatorImp.prepare(CoordinatorImp.java:514)
	at com.atomikos.icatch.imp.CoordinatorImp.terminate(CoordinatorImp.java:673)
	at com.atomikos.icatch.imp.CompositeTransactionImp.commit(CompositeTransactionImp.java:282)
	at com.atomikos.icatch.jta.TransactionImp.commit(TransactionImp.java:169)
	... 124 common frames omitted

I think it should be fine as long as at most one datasource is non-readOnly, what do you think?

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Using the NonXADataSource in non-readOnly mode with an additional resource is not OK because it does not support 2-phase commit, mainly due to recovery issues. None of the so-called "LRC" implementations is safe in that respect.

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 9, 2019

I am probably missing something but why would I need 2PC if only one of my datasources is writeable? Why is it necessary to regard readOnly datasources in the 2PC - they won't apply any changes?
I was expecting that a 1PC is performed for my single writeable datasource and I don't really care if the actions performed on the other datasources are rolled back or committed since they can only be read actions.

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Good point. Let me have a look...

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

Implemented.

@GuyPardon GuyPardon closed this Apr 25, 2019

@Mobe91

This comment has been minimized.

Copy link
Author

commented Apr 25, 2019

@GuyPardon

Good point. Let me have a look...

What exactly did you implement?

@GuyPardon

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

Subtransactions are now supported
Multiple read-only instances can be used in the same transaction
Non-read-only instances will fail if used in two-phase commit

Will push to GitHub soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.