Hi I have noticed since this commit, some of the JMS unit tests start to fail. Seen that in camel-jms and also in tests/camel-itest
On Tue, Aug 2, 2011 at 8:43 AM, Claus Ibsen <[email protected]> wrote: > Hi > > The stop operation on the consumer is synchronous to ensure the > operation goes well. > I am not to keen that this is changed just with the mind of improving > your unit test times. > This could break behavior for people using it in production. > > Also you grab the executor service on the reply manager. That is only > to be used when use if you do request/reply over JMS, and for that > purpose only. > > Instead I think we need a way of configuring this on the consumer or > endpoint with an option to control, such as: > - destroyListenerAsync > Mind think of a better name. > > And then don't use the ReplyManageerExecutorService but a dedicated > single threaded executor, you can grab from the executor service > manager. > > Also the change warrant a JIRA ticket imho. Changes to JMS is > important we keep track of in the release notes, for end users. > As JMS is used a lot. > > > On Tue, Aug 2, 2011 at 3:18 AM, <[email protected]> wrote: >> Author: dkulp >> Date: Tue Aug 2 01:18:01 2011 >> New Revision: 1152991 >> >> URL: http://svn.apache.org/viewvc?rev=1152991&view=rev >> Log: >> Use an async process to destroy the JMS DefaultMessageListenerContainer >> as the destroy method can take quite a while. (might be an issue in >> ActiveMQ). This chops the build time on my machine from 15min 28s >> to 10min 36s. >> >> Modified: >> >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsConsumer.java >> >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsEndpoint.java >> >> Modified: >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsConsumer.java >> URL: >> http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsConsumer.java?rev=1152991&r1=1152990&r2=1152991&view=diff >> ============================================================================== >> --- >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsConsumer.java >> (original) >> +++ >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsConsumer.java >> Tue Aug 2 01:18:01 2011 >> @@ -127,7 +127,7 @@ public class JmsConsumer extends Default >> protected void doStop() throws Exception { >> if (listenerContainer != null) { >> listenerContainer.stop(); >> - listenerContainer.destroy(); >> + >> ((JmsEndpoint)getEndpoint()).destroyMessageListenerContainer(listenerContainer); >> } >> >> // null container and listener so they are fully re created if this >> consumer is restarted >> >> Modified: >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsEndpoint.java >> URL: >> http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsEndpoint.java?rev=1152991&r1=1152990&r2=1152991&view=diff >> ============================================================================== >> --- >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsEndpoint.java >> (original) >> +++ >> camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsEndpoint.java >> Tue Aug 2 01:18:01 2011 >> @@ -81,6 +81,7 @@ public class JmsEndpoint extends Default >> // scheduled executor to check for timeout (reply not received) >> private ScheduledExecutorService replyManagerExecutorService; >> private final AtomicBoolean running = new AtomicBoolean(); >> + private volatile boolean destroying; >> >> public JmsEndpoint() { >> this(null, null); >> @@ -155,9 +156,30 @@ public class JmsEndpoint extends Default >> } >> >> public JmsConsumer createConsumer(Processor processor) throws Exception { >> + synchronized (this) { >> + while (destroying) { >> + wait(); >> + } >> + } >> DefaultMessageListenerContainer listenerContainer = >> createMessageListenerContainer(); >> return createConsumer(processor, listenerContainer); >> } >> + >> + private void >> destroyMessageListenerContainerInternal(DefaultMessageListenerContainer >> listenerContainer) { >> + listenerContainer.destroy(); >> + destroying = false; >> + synchronized (this) { >> + notifyAll(); >> + } >> + } >> + public void destroyMessageListenerContainer(final >> DefaultMessageListenerContainer listenerContainer) { >> + destroying = true; >> + this.getReplyManagerExecutorService().execute(new Runnable() { >> + public void run() { >> + destroyMessageListenerContainerInternal(listenerContainer); >> + } >> + }); >> + } >> >> public DefaultMessageListenerContainer createMessageListenerContainer() >> throws Exception { >> return configuration.createMessageListenerContainer(this); >> @@ -1007,6 +1029,7 @@ public class JmsEndpoint extends Default >> return super.createEndpointUri(); >> } >> >> + >> >> >> } >> >> >> > > > > -- > Claus Ibsen > ----------------- > FuseSource > Email: [email protected] > Web: http://fusesource.com > Twitter: davsclaus, fusenews > Blog: http://davsclaus.blogspot.com/ > Author of Camel in Action: http://www.manning.com/ibsen/ > -- Claus Ibsen ----------------- FuseSource Email: [email protected] Web: http://fusesource.com Twitter: davsclaus, fusenews Blog: http://davsclaus.blogspot.com/ Author of Camel in Action: http://www.manning.com/ibsen/
