Green build here: https://ci.apache.org/builders/tomee-7.0.x-ubuntu/builds/52 Not sure why this got is all pissy: https://ci.apache.org/builders/tomee-7.0.x-ubuntu-jvm8
Seems to be a different test failure each time. Builds fine on OSX/AdoptOpenJdk8 locally. Any hints? On Wed, Aug 28, 2019 at 10:08 AM Jonathan S. Fisher <[email protected]> wrote: > Alrighty. I've got "transactionSupport" ready. Apparently that hasn't > worked for some time. > > The nice thing too is that gives the user an "out" if they want to revert > to the non-spec behavior: Right now, connection factories are non-xa, by > the spec says they should be by default. If someone upgrades and desires > the old non-xa behavior, they can set transactionSupport=none on the > connection factory. > > I'm fairly confident in this merge request, the only part I really don't > know about is the enlistResource() when I enlist XASession. It works > though, and from the examples I've seen it seems to be the correct way to > enlist a resource. I've just never messed with that API before. > > Anyway, eyes appreciated, after I update my PRs on github I'll give it a > day for review and then merge it in. > > Thanks! > > > > > On Tue, Aug 27, 2019 at 9:27 PM Jonathan S. Fisher <[email protected]> > wrote: > >> Just noticed in JmsConnectionFactoryBuilder this is present already with >> the attribute "transactionSupport". I need to tie that into my patch before >> it's merged >> >> On Tue, Aug 27, 2019 at 4:30 PM Jonathan S. Fisher <[email protected]> >> wrote: >> >>> 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. >>> >> >> >> -- >> 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.
