I just checked Wildfly, they do the same thing as Liberty. I agree with your statement for the "completely correct" fix, ideally that's the place to do it, but might take awhile to get a release out.
On another note: I know the spec says, "Ignore all arguments to connection.create*(int mode)" methods. Yet I can think of a lot of scenarios where having a non-JTA connection pool is very handy (for instance, logging over JMS). We have the option to have non-JTA Database connections, I feel though we should be able to declare whether or not a jms connection pool participates in JTA. I'm thinking maybe we should have an `xa=true/false` parameter in the connection pool declaration. Would that be ok? On Tue, Aug 27, 2019 at 3:43 PM David Jencks <[email protected]> wrote: > I checked the Open Liberty TransactionSynchronizationRegistry, and it > interprets “active transaction” to mean “any transaction on the thread, no > matter it’s state”. So I think that it would be best to decide to do the > same in the Geronimo TM, deciding that the java doc is ambiguous as to the > meaning of “active” and the most useful meaning can be picked rather than > the most literal. > > Whether this is practical for the next TomEE, I don’t know. > > David Jencks > > > On Aug 27, 2019, at 8:25 AM, David Jencks <[email protected]> > wrote: > > > > I think the java doc for getResource might have been written > thoughtlessly, and more appropriate behavior would be an ISE only for > STATUS_NO_TRANSACTION; literally the geronimo implementation is too lax, as > “marked rollback” is not status “active”. Is there anyone who’s opinion we > might ask? > > > > I rather thought the “ignore session type” logic was supposed to be put > into the RA, but I don’t recall if or how I dealt with this in Geronimo. > > > > So I’d prefer these issues be dealt with elsewhere but don’t see much > practical alternative to your implementation. > > > > Nice to see someone working on XA:-) > > > > thanks! > > David Jencks > > > >> On Aug 26, 2019, at 1:45 PM, Jonathan S. Fisher <[email protected]> > wrote: > >> > >> I've narrowed down the problem to AutoConnectionTracker. It's not > >> completing, which isn't allowing the connections to be returned to the > pool. > >> > https://github.com/apache/tomee/blob/master/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java#L174 > >> > >> getResource() is throwing an IllegalStateException. The JavaDoc ( > >> > https://docs.oracle.com/javaee/7/api/javax/transaction/TransactionSynchronizationRegistry.html#getResource-java.lang.Object- > ) > >> states it should throw an ISE if a current transaction is not Active. > The > >> transaction is in the state ROLLED_BACK when AutoConnectionTracker > tries to > >> call getResource(). > >> > >> I think the Geronimo implementation ( > >> > https://github.com/apache/geronimo-txmanager/blame/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionManagerImpl.java#L203 > ) > >> maybe be a little too strict. The JTA Spec pdf doesn't offer exact > hints of > >> which statuses ( > >> https://docs.oracle.com/javaee/7/api/javax/transaction/Status.html) > should > >> be have getResource _not_ throw an ISE unfortunately. I was thinking of > >> changing Geronimo's implementation to check for anything > >> but STATUS_UNKNOWN, and STATUS_NO_TRANSACTION. > >> > >> The other way is to cast Transaction to the Geronimo implementation and > use > >> Geronimo specific APIs to get call getResource(). Do you guys have any > >> preference which route I should take to fix? > >> > >> > >> On Mon, Aug 26, 2019 at 9:15 AM Jonathan S. Fisher <[email protected]> > >> wrote: > >> > >>> https://github.com/exabrial/tomee-jms2-bug/tree/connection-pool-leak > >>> > >>> Here's a project that reproduces the bug. This project intentionally > >>> exceeds the transaction timeout (of 1s). Each invocation, the > connection is > >>> not returned to the pool and eventually you run out, causing your > >>> application to freeze. > >>> > >>> > >>> > >>> On Fri, Aug 23, 2019 at 2:37 PM Jonathan S. Fisher <[email protected] > > > >>> wrote: > >>> > >>>> Hello Apache friends :) I have a question about the JTA and JMS/RA > specs: > >>>> > >>>> If you borrow something from a RA, like a JMS Connection, and you're > in > >>>> XA Transaction, is it necessary to call connection.close()? It would > seem > >>>> JTA should be smart enough to know the connection is enrolled for 2 > phase > >>>> commit and should be smart enough to close it, but I'm not sure if > that's > >>>> part of the spec. > >>>> > >>>> In TomEE 7.0.6 we're noticing that if you borrow a JMS Connection with > >>>> connectionFactory.createConnection(), and your code fails to call > close() > >>>> before the transaction completion, the connection leaks. (And > >>>> unfortunately, calling close() after the transaction completes doesn't > >>>> mitigate the problem). It took awhile for us to track this down. > >>>> > >>>> This becomes a huge problem if you're calling external services in > your > >>>> transaction. Let's say you have a reasonable transaction timeout of > 30s > >>>> set. You call three services, and they end up taking 15s a piece. > Even if > >>>> you're doing the right thing and you have connection.close() in a > finally > >>>> block, because your transaction isn't active when you call close, it > leaks > >>>> and it just gets "stuck" as an active connection, which eventually > you hit > >>>> the pool limit and your app freezes. > >>>> > >>>> On a separate note, we noticed if you open a connection outside of the > >>>> scope of a transaction, then start a transaction, then create a > session > >>>> with session_transacted option, the session does not participate in > the JTA > >>>> (which seems out of spec). One most open the connection inside the > >>>> transaction, AND open the session in the transaction, and close the > >>>> connection in the transaction for everything to work. > >>>> > >>>> I'll get a reproducing project created, but I was curious if anyone > knew > >>>> offhand what the spec says. > >>>> > >>>> cheers, and thanks, > >>>> -[the other] Jonathan > >>>> > >>>> -- > >>>> Jonathan | [email protected] > >>>> Pessimists, see a jar as half empty. Optimists, in contrast, see it as > >>>> half full. > >>>> Engineers, of course, understand the glass is twice as big as it > needs to > >>>> be. > >>>> > >>> > >>> > >>> -- > >>> Jonathan | [email protected] > >>> Pessimists, see a jar as half empty. Optimists, in contrast, see it as > >>> half full. > >>> Engineers, of course, understand the glass is twice as big as it needs > to > >>> be. > >>> > >> > >> > >> -- > >> Jonathan | [email protected] > >> Pessimists, see a jar as half empty. Optimists, in contrast, see it as > half > >> full. > >> Engineers, of course, understand the glass is twice as big as it needs > to > >> be. > > > > -- Jonathan | [email protected] Pessimists, see a jar as half empty. Optimists, in contrast, see it as half full. Engineers, of course, understand the glass is twice as big as it needs to be.
