JBTM-3723 Remove contention point on TransactionManager#transactionManager #2067
Conversation
|
Started testing this pull request with AS_TESTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with LRA profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with TOMCAT profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=TOMCAT,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with QA_JTA profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with QA_JTS_OPENJDKORB profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTS_OPENJDKORB,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with CORE profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with JACOCO profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with XTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk11.latest,label=linux/1257/ |
|
Started testing this pull request with RTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=jdk11.latest,label=linux/1257/ |
|
TOMCAT profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=TOMCAT,jdk=jdk11.latest,label=linux/1257/ |
|
RTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=jdk11.latest,label=linux/1257/ |
|
LRA profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=jdk11.latest,label=linux/1257/ |
|
CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk11.latest,label=linux/1257/ |
|
XTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk11.latest,label=linux/1257/ |
|
QA_JTA profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=jdk11.latest,label=linux/1257/ |
There was a problem hiding this comment.
@Sanne I think it's fine to remove the keyword.
We already perform double checked locking when looking up the transaction manager [1]
Way back in Jul 2006 we used to do the lookup directly:
_transactionManager = (javax.transaction.TransactionManager) Thread.currentThread().getContextClassLoader().loadClass(JNDIManager.getTransactionManagerImplementationClassname()).newInstance();
hence we needed the synchronization, but now we do it via our environment bean
return jtaPropertyManager.getJTAEnvironmentBean().getTransactionManager();
which does the double lock check.
|
@jmfinelli this looks like a good change, do you mind if I merge it? |
|
AS_TESTS profile tests failed (http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk11.latest,label=linux/1257/): AS tests failed The test failures are unrelated |
Not at all! LGTM as well! |
|
@mmusgrov Right, I had also noticed that both @jmfinelli thanks! FWIW I'll leave backporting choices to you all, but bear in mind it would be great to have this fix available in EAP8. |
It will be included in our next release, i.e. Narayana 6.0 (which will be part of EAP8) |
|
QA_JTS_OPENJDKORB profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTS_OPENJDKORB,jdk=jdk11.latest,label=linux/1257/ |
|
JACOCO profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=jdk11.latest,label=linux/1257/ |
Fix proposal for https://issues.redhat.com/browse/JBTM-3723