You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Andy Gumbrecht <ag...@tomitribe.com> on 2015/11/18 12:21:57 UTC
ActiveMQ connection wrapper
Looking for suggestions as to where and how it could be best to wrap any
connections from AMQ.
We need to do this in order to ensure we can kill connections on a
server shutdown.
Basically anywhere this gets injected there needs to be a wrapper
returned in it's place.
public interface ConnectionFactory {
Connection createConnection() throws JMSException;
Connection createConnection(String userName, String password)
throws JMSException;
}
Looking at JndiRequestHandler, but not sure this is the right place.
Thanks for your help.
Andy.
--
Andy Gumbrecht
https://twitter.com/AndyGeeDe
http://www.tomitribe.com
Re: ActiveMQ connection wrapper
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le 20 nov. 2015 07:25, "Andy" <an...@gmx.de> a écrit :
>
> Not sure what you mean by not thread safe as there are never any calls
made on the instances out of context? If anything is done out of context
then it is supposed to be noisy. There is no magic in the delegate wrappers.
>
Can happen with @Async
> Also have no idea where about the geronimo mechanism. The
org.apache.activemq.ra.ActiveMQConnectionFactory is otherwise injected
directly. As previously queried in the thread, if you have a better way
then please provide it or at least provide more information on how to do it
- can't see into your brain so you'll have to open it up ;-)
We nevzr have directly this instance normally. If it happens - no jta case
- then we can still use a flag for the wrapping instead of forcing it.
For geronimo check interceptors in ConnectionManager.
>
> Where else would you keep a reference that is available after everything
is shut down?
>
I wouldnt. I would worse case keep it untile the CF is destroyed.
>
> On 20/11/2015 16:03, Romain Manni-Bucau wrote:
>>
>> Well while it doesnt use any static map ok.
>>
>> We have all the needed code in geronimo to track it properly without
eveven
>> being bound to AMQ and modify injector code at all. Easy alternative is
>> doing the wrapping in the resource defition. This sounds even a good
>> transversal feature for debug purposes - like LogSql for datasource. In
any
>> case I wouldnt have it enforced.
>>
>> Side note: the impl is not thread safe and TCK doesnt test much of it so
>> not sure it is a criteria. Master has a @Ignore test for this case
waiting
>> for AMQ upgrade, can at least validate a simple case.
>>
>> Does it make sense?
>>
>> Ps: several master fixed can be linked to this as well like RA sorting
for
>> destroying of resources
>> Le 20 nov. 2015 04:23, "Jean-Louis Monteiro" <jl...@tomitribe.com> a
>> écrit :
>>
>>> The logging about leaked connections is definitely useful for users so
they
>>> can fix.
>>> Le 20 nov. 2015 13:16, "Jonathan Gallimore" <jg...@tomitribe.com> a
>>> écrit :
>>>
>>>> +1
>>>>
>>>> Jon
>>>>
>>>> On Fri, Nov 20, 2015 at 12:15 PM, Andy Gumbrecht <
>>>
>>> agumbrecht@tomitribe.com
>>>>
>>>> wrote:
>>>>
>>>>> The wrapper is passing all tests on 1.7.x. I'd therefore like to keep
>>>
>>> it
>>>>>
>>>>> as it only hardens TomEE against connection/session misuse and also
>>>>> provides valuable logging information if issues exist.
>>>>>
>>>>> I'll wait for feedback from everyone before I forward port the
wrapper.
>>>>>
>>>>>
>>>>> Andy.
>>>>>
>>>>> --
>>>>> Andy Gumbrecht
>>>>> https://twitter.com/AndyGeeDe
>>>>> http://www.tomitribe.com
>>>>>
>>>>>
>>>>
>>>> --
>>>> Jonathan Gallimore
>>>> http://twitter.com/jongallimore
>>>> http://www.tomitribe.com
>>>>
>
> --
> Andy Gumbrecht
> https://twitter.com/AndyGeeDe
>
Re: ActiveMQ connection wrapper
Posted by Jean-Louis Monteiro <jl...@tomitribe.com>.
I like the idea to log at the end where the user leaked connections.
If that can be done in the RA, it's also fine for me.
--
Jean-Louis Monteiro
http://twitter.com/jlouismonteiro
http://www.tomitribe.com
On Fri, Nov 20, 2015 at 4:24 PM, Andy <an...@gmx.de> wrote:
> Not sure what you mean by not thread safe as there are never any calls
> made on the instances out of context? If anything is done out of context
> then it is supposed to be noisy. There is no magic in the delegate wrappers.
>
> Also have no idea where about the geronimo mechanism. The
> org.apache.activemq.ra.ActiveMQConnectionFactory is otherwise injected
> directly. As previously queried in the thread, if you have a better way
> then please provide it or at least provide more information on how to do it
> - can't see into your brain so you'll have to open it up ;-)
>
> Where else would you keep a reference that is available after everything
> is shut down?
>
>
> On 20/11/2015 16:03, Romain Manni-Bucau wrote:
>
>> Well while it doesnt use any static map ok.
>>
>> We have all the needed code in geronimo to track it properly without
>> eveven
>> being bound to AMQ and modify injector code at all. Easy alternative is
>> doing the wrapping in the resource defition. This sounds even a good
>> transversal feature for debug purposes - like LogSql for datasource. In
>> any
>> case I wouldnt have it enforced.
>>
>> Side note: the impl is not thread safe and TCK doesnt test much of it so
>> not sure it is a criteria. Master has a @Ignore test for this case waiting
>> for AMQ upgrade, can at least validate a simple case.
>>
>> Does it make sense?
>>
>> Ps: several master fixed can be linked to this as well like RA sorting for
>> destroying of resources
>> Le 20 nov. 2015 04:23, "Jean-Louis Monteiro" <jl...@tomitribe.com> a
>> écrit :
>>
>> The logging about leaked connections is definitely useful for users so
>>> they
>>> can fix.
>>> Le 20 nov. 2015 13:16, "Jonathan Gallimore" <jg...@tomitribe.com> a
>>> écrit :
>>>
>>> +1
>>>>
>>>> Jon
>>>>
>>>> On Fri, Nov 20, 2015 at 12:15 PM, Andy Gumbrecht <
>>>>
>>> agumbrecht@tomitribe.com
>>>
>>>> wrote:
>>>>
>>>> The wrapper is passing all tests on 1.7.x. I'd therefore like to keep
>>>>>
>>>> it
>>>
>>>> as it only hardens TomEE against connection/session misuse and also
>>>>> provides valuable logging information if issues exist.
>>>>>
>>>>> I'll wait for feedback from everyone before I forward port the wrapper.
>>>>>
>>>>>
>>>>> Andy.
>>>>>
>>>>> --
>>>>> Andy Gumbrecht
>>>>> https://twitter.com/AndyGeeDe
>>>>> http://www.tomitribe.com
>>>>>
>>>>>
>>>>>
>>>> --
>>>> Jonathan Gallimore
>>>> http://twitter.com/jongallimore
>>>> http://www.tomitribe.com
>>>>
>>>>
> --
> Andy Gumbrecht
> https://twitter.com/AndyGeeDe
>
>
Re: ActiveMQ connection wrapper
Posted by Andy <an...@gmx.de>.
Not sure what you mean by not thread safe as there are never any calls
made on the instances out of context? If anything is done out of context
then it is supposed to be noisy. There is no magic in the delegate wrappers.
Also have no idea where about the geronimo mechanism. The
org.apache.activemq.ra.ActiveMQConnectionFactory is otherwise injected
directly. As previously queried in the thread, if you have a better way
then please provide it or at least provide more information on how to do
it - can't see into your brain so you'll have to open it up ;-)
Where else would you keep a reference that is available after everything
is shut down?
On 20/11/2015 16:03, Romain Manni-Bucau wrote:
> Well while it doesnt use any static map ok.
>
> We have all the needed code in geronimo to track it properly without eveven
> being bound to AMQ and modify injector code at all. Easy alternative is
> doing the wrapping in the resource defition. This sounds even a good
> transversal feature for debug purposes - like LogSql for datasource. In any
> case I wouldnt have it enforced.
>
> Side note: the impl is not thread safe and TCK doesnt test much of it so
> not sure it is a criteria. Master has a @Ignore test for this case waiting
> for AMQ upgrade, can at least validate a simple case.
>
> Does it make sense?
>
> Ps: several master fixed can be linked to this as well like RA sorting for
> destroying of resources
> Le 20 nov. 2015 04:23, "Jean-Louis Monteiro" <jl...@tomitribe.com> a
> écrit :
>
>> The logging about leaked connections is definitely useful for users so they
>> can fix.
>> Le 20 nov. 2015 13:16, "Jonathan Gallimore" <jg...@tomitribe.com> a
>> écrit :
>>
>>> +1
>>>
>>> Jon
>>>
>>> On Fri, Nov 20, 2015 at 12:15 PM, Andy Gumbrecht <
>> agumbrecht@tomitribe.com
>>> wrote:
>>>
>>>> The wrapper is passing all tests on 1.7.x. I'd therefore like to keep
>> it
>>>> as it only hardens TomEE against connection/session misuse and also
>>>> provides valuable logging information if issues exist.
>>>>
>>>> I'll wait for feedback from everyone before I forward port the wrapper.
>>>>
>>>>
>>>> Andy.
>>>>
>>>> --
>>>> Andy Gumbrecht
>>>> https://twitter.com/AndyGeeDe
>>>> http://www.tomitribe.com
>>>>
>>>>
>>>
>>> --
>>> Jonathan Gallimore
>>> http://twitter.com/jongallimore
>>> http://www.tomitribe.com
>>>
--
Andy Gumbrecht
https://twitter.com/AndyGeeDe
Re: ActiveMQ connection wrapper
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Well while it doesnt use any static map ok.
We have all the needed code in geronimo to track it properly without eveven
being bound to AMQ and modify injector code at all. Easy alternative is
doing the wrapping in the resource defition. This sounds even a good
transversal feature for debug purposes - like LogSql for datasource. In any
case I wouldnt have it enforced.
Side note: the impl is not thread safe and TCK doesnt test much of it so
not sure it is a criteria. Master has a @Ignore test for this case waiting
for AMQ upgrade, can at least validate a simple case.
Does it make sense?
Ps: several master fixed can be linked to this as well like RA sorting for
destroying of resources
Le 20 nov. 2015 04:23, "Jean-Louis Monteiro" <jl...@tomitribe.com> a
écrit :
> The logging about leaked connections is definitely useful for users so they
> can fix.
> Le 20 nov. 2015 13:16, "Jonathan Gallimore" <jg...@tomitribe.com> a
> écrit :
>
> > +1
> >
> > Jon
> >
> > On Fri, Nov 20, 2015 at 12:15 PM, Andy Gumbrecht <
> agumbrecht@tomitribe.com
> > >
> > wrote:
> >
> > > The wrapper is passing all tests on 1.7.x. I'd therefore like to keep
> it
> > > as it only hardens TomEE against connection/session misuse and also
> > > provides valuable logging information if issues exist.
> > >
> > > I'll wait for feedback from everyone before I forward port the wrapper.
> > >
> > >
> > > Andy.
> > >
> > > --
> > > Andy Gumbrecht
> > > https://twitter.com/AndyGeeDe
> > > http://www.tomitribe.com
> > >
> > >
> >
> >
> > --
> > Jonathan Gallimore
> > http://twitter.com/jongallimore
> > http://www.tomitribe.com
> >
>
Re: ActiveMQ connection wrapper
Posted by Jean-Louis Monteiro <jl...@tomitribe.com>.
The logging about leaked connections is definitely useful for users so they
can fix.
Le 20 nov. 2015 13:16, "Jonathan Gallimore" <jg...@tomitribe.com> a
écrit :
> +1
>
> Jon
>
> On Fri, Nov 20, 2015 at 12:15 PM, Andy Gumbrecht <agumbrecht@tomitribe.com
> >
> wrote:
>
> > The wrapper is passing all tests on 1.7.x. I'd therefore like to keep it
> > as it only hardens TomEE against connection/session misuse and also
> > provides valuable logging information if issues exist.
> >
> > I'll wait for feedback from everyone before I forward port the wrapper.
> >
> >
> > Andy.
> >
> > --
> > Andy Gumbrecht
> > https://twitter.com/AndyGeeDe
> > http://www.tomitribe.com
> >
> >
>
>
> --
> Jonathan Gallimore
> http://twitter.com/jongallimore
> http://www.tomitribe.com
>
Re: ActiveMQ connection wrapper
Posted by Jonathan Gallimore <jg...@tomitribe.com>.
+1
Jon
On Fri, Nov 20, 2015 at 12:15 PM, Andy Gumbrecht <ag...@tomitribe.com>
wrote:
> The wrapper is passing all tests on 1.7.x. I'd therefore like to keep it
> as it only hardens TomEE against connection/session misuse and also
> provides valuable logging information if issues exist.
>
> I'll wait for feedback from everyone before I forward port the wrapper.
>
>
> Andy.
>
> --
> Andy Gumbrecht
> https://twitter.com/AndyGeeDe
> http://www.tomitribe.com
>
>
--
Jonathan Gallimore
http://twitter.com/jongallimore
http://www.tomitribe.com
Re: ActiveMQ connection wrapper
Posted by Andy Gumbrecht <ag...@tomitribe.com>.
The wrapper is passing all tests on 1.7.x. I'd therefore like to keep it
as it only hardens TomEE against connection/session misuse and also
provides valuable logging information if issues exist.
I'll wait for feedback from everyone before I forward port the wrapper.
Andy.
--
Andy Gumbrecht
https://twitter.com/AndyGeeDe
http://www.tomitribe.com
Re: ActiveMQ connection wrapper
Posted by Romain Manni-Bucau <rm...@gmail.com>.
well pushed few enhancements all around this thing on master but it
was really around so would be great to first know if it helps and then
validate the leaking thread is due to AMQ missing a catch (
https://issues.apache.org/jira/browse/AMQ-6051 )
If that is the case we will just fix and upgrade AMQ to the next release.
Romain Manni-Bucau
@rmannibucau | Blog | Github | LinkedIn | Tomitriber
2015-11-18 16:03 GMT-08:00 agumbrecht <ag...@tomitribe.com>:
> The current commit would need more work to complete. However, I feel a revert
> coming on. Is there something we can actually do without requiring a new AMQ
> version?
>
> The current wrapper seems to resolve the issue on testing the dangling
> thread shutdown, but needs more work to cope with Topic and Queue
> Connections.
>
> Andy.
>
>
>
> -----
> --
> Andy Gumbrecht
>
> http://www.tomitribe.com
> agumbrecht@tomitribe.com
> https://twitter.com/AndyGeeDe
>
> TomEE treibt Tomitribe ! | http://tomee.apache.org
> --
> View this message in context: http://tomee-openejb.979440.n4.nabble.com/ActiveMQ-connection-wrapper-tp4676832p4676854.html
> Sent from the TomEE Dev mailing list archive at Nabble.com.
Re: ActiveMQ connection wrapper
Posted by agumbrecht <ag...@tomitribe.com>.
The current commit would need more work to complete. However, I feel a revert
coming on. Is there something we can actually do without requiring a new AMQ
version?
The current wrapper seems to resolve the issue on testing the dangling
thread shutdown, but needs more work to cope with Topic and Queue
Connections.
Andy.
-----
--
Andy Gumbrecht
http://www.tomitribe.com
agumbrecht@tomitribe.com
https://twitter.com/AndyGeeDe
TomEE treibt Tomitribe ! | http://tomee.apache.org
--
View this message in context: http://tomee-openejb.979440.n4.nabble.com/ActiveMQ-connection-wrapper-tp4676832p4676854.html
Sent from the TomEE Dev mailing list archive at Nabble.com.
Re: ActiveMQ connection wrapper
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Geronimo connector already does that with its connections proxying. There
is a phantom tracker IIRC.
Do you know in which case we would leak connections? Last time I checked it
was only possible if missing a close which is a programming miss. Is that
the case we want to deal with?
Le 18 nov. 2015 04:32, "agumbrecht" <ag...@tomitribe.com> a écrit :
> This seems to be the best candidate location to wrap the ConnectionFactory:
>
> org/apache/openejb/InjectionProcessor.java:243
>
> Adding if(ConnectionFactory.class.isInstance(value)){value = ..wrapper..}
> would work here.
>
> However, is there a way to 'event' the wrapping without too much overhead?
> -
> I'm thinking of a more general way of saying - hey pre-injection event
> listener here is an object value I am about to inject into this class, do
> you want to wrap it first.
>
> Andy.
>
>
>
>
>
> -----
> --
> Andy Gumbrecht
>
> http://www.tomitribe.com
> agumbrecht@tomitribe.com
> https://twitter.com/AndyGeeDe
>
> TomEE treibt Tomitribe ! | http://tomee.apache.org
> --
> View this message in context:
> http://tomee-openejb.979440.n4.nabble.com/ActiveMQ-connection-wrapper-tp4676832p4676834.html
> Sent from the TomEE Dev mailing list archive at Nabble.com.
>
Re: ActiveMQ connection wrapper
Posted by agumbrecht <ag...@tomitribe.com>.
This seems to be the best candidate location to wrap the ConnectionFactory:
org/apache/openejb/InjectionProcessor.java:243
Adding if(ConnectionFactory.class.isInstance(value)){value = ..wrapper..}
would work here.
However, is there a way to 'event' the wrapping without too much overhead? -
I'm thinking of a more general way of saying - hey pre-injection event
listener here is an object value I am about to inject into this class, do
you want to wrap it first.
Andy.
-----
--
Andy Gumbrecht
http://www.tomitribe.com
agumbrecht@tomitribe.com
https://twitter.com/AndyGeeDe
TomEE treibt Tomitribe ! | http://tomee.apache.org
--
View this message in context: http://tomee-openejb.979440.n4.nabble.com/ActiveMQ-connection-wrapper-tp4676832p4676834.html
Sent from the TomEE Dev mailing list archive at Nabble.com.