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

Remove High contention on TransactionManager class #4496

Merged
merged 1 commit into from
Oct 11, 2019

Conversation

johnaohara
Copy link
Member

… causes high contention on TransactionManager class.

The current Narayana integration frequently calls TransactionalInterceptorBase.invokeInOurTx(), which looks up the current configured TX timeout via
((TransactionManagerImple) com.arjuna.ats.jta.TransactionManager.transactionManager()).getTimeout()

This is a very expensive call, as com.arjuna.ats.jta.TransactionManager.transactionManager() is synchronized on the com.arjuna.ats.jta.TransactionManager class.

We could use the @ApplicationScoped TransationManager injected into io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase if the API defined getTimeout(), but it does not. At present @inject TransationManager transactionManager can not be cast to TransactionManagerImple as it is a proxy.

I am not 100% happy with this PR, as it changes the injected bean to an impl bean instead of an api bean, but it does remove the monitor contention under heavy load.

@stuartwdouglas @mkouba can you think of any other way of resolving this issue?

@johnaohara johnaohara changed the title Inject Application Scoped TransactionManager, resolving for each call… Remove High contention on TransactionManager class Oct 10, 2019
@johnaohara
Copy link
Member Author

Have changed producer from `@ApplicationScoped' to '@singleton' so proxy is not generated by arc, and we cast TransactionManager to TransactionManagerImple successfully.

@johnaohara johnaohara added backport? kind/bug Something isn't working labels Oct 10, 2019
@johnaohara johnaohara added this to the 0.25.0 milestone Oct 10, 2019
… causes high contention on TransactionManager class
@gsmet
Copy link
Member

gsmet commented Oct 10, 2019

Removing the backport label as 0.24.0 has been tagged this morning.

@stuartwdouglas
Copy link
Member

@mmusgrov do you happen to know why this method is synchronized? Looking at the code it does not look like it needs to be.

@stuartwdouglas stuartwdouglas merged commit e2c9b03 into quarkusio:master Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants