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

WFLY-2651 If there is a transaction active when the setup action first r... #5677

Closed
wants to merge 1 commit into from

Conversation

stuartwdouglas
Copy link
Contributor

...uns don't attempt to close it

@wildfly-ci
Copy link

Build 2153 is now running using a merge of bafd08d

@wildfly-ci
Copy link

Build 2153 outcome was FAILURE using a merge of bafd08d
Summary: Tests failed: 1 (1 new), passed: 5530, ignored: 73 Build time: 1:29:58

Build problems:

Failed tests detected

java.lang.AssertionError: 0 invocations were routed to node-1
java.lang.AssertionError: 0 invocations were routed to node-1
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.assertTrue(Assert.java:41)
    at org.jboss.as.test.clustering.cluster.ejb.remote.RemoteFailoverTestCase.testStatelessFailover(RemoteFailoverTestCase.java:180)
    at org.jboss.as.test.clustering.cluster.ejb.remote.RemoteFailoverTestCase.testStatelessFailover(RemoteFailoverTestCase.java:101)

Failed tests

org.jboss.as.test.clustering.cluster.ejb.remote.RemoteFailoverTestCase(SYNC-tcp).testStatelessFailover: java.lang.AssertionError: 0 invocations were routed to node-1
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.assertTrue(Assert.java:41)

try {
final TransactionManager tm = transactionManager.getOptionalValue();
if (tm != null) {
holder.actuallyCleanUp = isTransactionActive(tm, tm.getStatus());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you want to clean up if the tx exists when setup() was first called, but not if it's opened later? I believe that's what will happen here and it sounds different from what the comment at L103 says.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops.

I wonder why this seemed to fix the issue then. I will fix this up.

@wildfly-ci
Copy link

Build 2188 is now running using a merge of d7907b4

@wildfly-ci
Copy link

Build 2188 outcome was SUCCESS using a merge of d7907b4
Summary: Tests passed: 5531, ignored: 73 Build time: 1:27:27

@bstansberry
Copy link
Contributor

Merged.

@bstansberry bstansberry closed this Jan 9, 2014
@velo
Copy link

velo commented Jun 23, 2014

Is this fix going to be present on any JBoss EAP release?

@ctomc
Copy link
Contributor

ctomc commented Jun 23, 2014

@velo there was no bug reported for this in EAP, and as such it was not backported.
It looks like that it could be as well applied to EAP, but there should be bugzilla entered for it in https://bugzilla.redhat.com/ or a support case via http://access.redhat.com/

We don't automatically back port everything as fixes are not always applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants