Re: JTA JMS Spec question, connection leakage

2019-09-04 Thread Jonathan S. Fisher
Nice work! So one final grievance. This has to do with MDBs... we see this message spammed to sysout all the time when an MDB consumes a message: Sep 04, 2019 9:41:37 AM org.apache.geronimo.connector.work.WorkerContext run INFO: Removing non-required WorkContextHandler with no context: org.apache

Re: JTA JMS Spec question, connection leakage

2019-09-04 Thread Jonathan Gallimore
Yep, should be on 7.0.x, 7.1.x and master. Jon On Wed, Sep 4, 2019 at 3:35 PM Jonathan S. Fisher wrote: > Nice, thanks for cleaning that merge up. This got ported to both 7.1.x and > master? > > > > On Wed, Sep 4, 2019 at 8:10 AM Jonathan Gallimore < > jonathan.gallim...@gmail.com> wrote: > > >

Re: JTA JMS Spec question, connection leakage

2019-09-04 Thread Jonathan S. Fisher
Nice, thanks for cleaning that merge up. This got ported to both 7.1.x and master? On Wed, Sep 4, 2019 at 8:10 AM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > Ok, think I found it. The code in AutoConnectionTracker.handleObtained() > changed ever so slightly, but the effect was p

Re: JTA JMS Spec question, connection leakage

2019-09-04 Thread Jonathan Gallimore
Ok, think I found it. The code in AutoConnectionTracker.handleObtained() changed ever so slightly, but the effect was proxyConnection() didn't get called. I've fixed that, and the test passes. I've also run the JMS Arquillian tests which work ok too. Cheers! Jon On Wed, Sep 4, 2019 at 12:16 PM J

Re: JTA JMS Spec question, connection leakage

2019-09-04 Thread Jonathan Gallimore
Thanks Jonathan! I merged this in, and ported to 7.1.x. I've run into a test failure with org.apache.openejb.resource.GeronimoConnectionManagerFactoryTest. No idea what the root cause is yet - checking it out. I haven't pushed this to master yet (will try and resolve that test issue first). Thanks

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread David Jencks
Thanks for the explanation; I think your change should be good for everyone, not just amq. Iirc it used to be common (very bad) practice to get a connection handle and cache it in a non Singleton session bean and expect the connection infrastructure to associate it to whatever managed connectio

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
Ahhh... The CDI spec for JMSContext says it's either scoped to either the current RequestScope, or in our case TransactionScope. I'll take an educated guess and bet TransactionScoped beans are destroyed using a [Transaction] Synchronization's afterCompletion() call. When the TransactionScope is des

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread David Jencks
Well, beforeCompletion() is called as a result of commit() being called on the transaction, presumably by an EJB “interceptor”, and handleClosed() is called as a result of the “user level” connection being closed. I’m used to the latter being called by user code... perhaps with all the CDI and d

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
Honestly I have no idea. And the interface specification is silent unfortunately: https://github.com/apache/geronimo-txmanager/blob/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/connectiontracking/ConnectionTracker.java On Tue, Sep 3, 2019 at 3:32 PM David Jencks w

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread David Jencks
You might have already explained this, but… why is beforeCompletion() called before handleReleased()? If that’s happening, I’d expect something is wrong. However, I haven’t looked at this code in years. thanks! David Jencks > On Sep 3, 2019, at 12:27 PM, Jonathan S. Fisher wrote: > > Two m

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
Cool! And you're welcome! JMS, ActiveMQ, and XA transactions are pretty key for us; they form the building blocks for us to scale horizontally (Kafka like patterns). I just ran a 100,000 messages through the code in the PR without problems or memory leaks, so I'm not worried about that Arquillian

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan Gallimore
I knew you'd know some magic to help with this - thank you! Just looking at your PR now. Will give it a test shortly, but it looks good to me. Jon On Tue, Sep 3, 2019 at 3:08 PM Jonathan S. Fisher wrote: > There are actually a few log messages we regularly ignore all the time from > the transac

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
Two more updates: For the log message, it looks like beforeCompletion() Is being called before handleReleased(), leading to that warning. I ran a couple thousand messages through and took a heap dump and didn't get any leak suspects, so I think it's working correctly despite the warning. I'll add

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
If I bump the number of messages up to 10k or so I get a VM Crash... I cannot figure out how to get arquillian to give me a heap dump on exit though. On Tue, Sep 3, 2019 at 10:01 AM Jonathan S. Fisher wrote: > https://github.com/apache/tomee/pull/546/files > > This passes consistently for me wit

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
https://github.com/apache/tomee/pull/546/files This passes consistently for me with no issues On Tue, Sep 3, 2019 at 9:08 AM Jonathan S. Fisher wrote: > There are actually a few log messages we regularly ignore all the time > from the transaction manager ::wince face:: I'm not sure if we should

Re: JTA JMS Spec question, connection leakage

2019-09-03 Thread Jonathan S. Fisher
There are actually a few log messages we regularly ignore all the time from the transaction manager ::wince face:: I'm not sure if we should be concerned with that one. On your test, first, how is the broker xml declared? Often something that trips our newbies up to TomEE is having a persistent br

Re: JTA JMS Spec question, connection leakage

2019-09-02 Thread Jonathan Gallimore
In case it means anything to anyone, the unexpected output I'm getting is an abandoned connection notification: WARNING: Transaction complete, but connection still has handles associated: ManagedConnectionInfo: org.apache.geronimo.connector.outbound.ManagedConnectionInfo@5b5a4aed. mc: [org.apache.

Re: JTA JMS Spec question, connection leakage

2019-09-02 Thread Jonathan Gallimore
I had a play around with this, and got my previously failing JMSContextInjectionTest to work -- some of the time. ( https://github.com/apache/tomee/blob/e58ff848a3a80938d4d99fc9bcfeaade5a72d644/arquillian/arquillian-tomee-tests/arquillian-tomee-jms-tests/src/test/java/org/apache/openejb/arquillian/

Re: JTA JMS Spec question, connection leakage

2019-08-29 Thread Jonathan S. Fisher
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

Re: JTA JMS Spec question, connection leakage

2019-08-28 Thread Jonathan S. Fisher
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 upgrade

Re: JTA JMS Spec question, connection leakage

2019-08-27 Thread Jonathan S. Fisher
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 wrote: > I just checked Wildfly, they do the same thing as Liberty. I agree with > y

Re: JTA JMS Spec question, connection leakage

2019-08-27 Thread Jonathan S. Fisher
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)" meth

Re: JTA JMS Spec question, connection leakage

2019-08-27 Thread David Jencks
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 me

Re: JTA JMS Spec question, connection leakage

2019-08-27 Thread Jonathan Gallimore
Hi Jonathan Thanks for this. I think I had run into some of the things you fixed I little while back. I'll take a look at your change later today. Happy for you to commit and I can review after, too. :) Cheers The other Jonathan On Tue, Aug 27, 2019 at 2:59 AM Jonathan S. Fisher wrote: > > h

Re: JTA JMS Spec question, connection leakage

2019-08-27 Thread David Jencks
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

Re: JTA JMS Spec question, connection leakage

2019-08-26 Thread Jonathan S. Fisher
https://github.com/exabrial/tomee/commit/08dd4c744818702f3be5edfd8a1c4cf2b69d524d With these patches the JMS2.0 API works pretty well now :) If anyone wants to review, please go ahead, otherwise I'll commit them tomorrow. cheers, - [The other] Jonathan On Mon, Aug 26, 2019 at 3:45 PM Jonathan S.

Re: JTA JMS Spec question, connection leakage

2019-08-26 Thread Jonathan S. Fisher
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()

Re: JTA JMS Spec question, connection leakage

2019-08-26 Thread Jonathan S. Fisher
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 fre

JTA JMS Spec question, connection leakage

2019-08-23 Thread Jonathan S. Fisher
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 comm