Return-Path: X-Original-To: apmail-camel-dev-archive@www.apache.org Delivered-To: apmail-camel-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9CA0E6D65 for ; Tue, 2 Aug 2011 11:02:10 +0000 (UTC) Received: (qmail 94495 invoked by uid 500); 2 Aug 2011 11:02:09 -0000 Delivered-To: apmail-camel-dev-archive@camel.apache.org Received: (qmail 94062 invoked by uid 500); 2 Aug 2011 11:01:55 -0000 Mailing-List: contact dev-help@camel.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@camel.apache.org Delivered-To: mailing list dev@camel.apache.org Received: (qmail 94039 invoked by uid 99); 2 Aug 2011 11:01:51 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 02 Aug 2011 11:01:51 +0000 X-ASF-Spam-Status: No, hits=-0.7 required=5.0 tests=FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of claus.ibsen@gmail.com designates 209.85.210.49 as permitted sender) Received: from [209.85.210.49] (HELO mail-pz0-f49.google.com) (209.85.210.49) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 02 Aug 2011 11:01:44 +0000 Received: by pzk33 with SMTP id 33so13830863pzk.36 for ; Tue, 02 Aug 2011 04:01:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type:content-transfer-encoding; bh=2egKvM1xVl7yQqz5xsyGtYovS6oKTkD2a93Ezk1XFus=; b=AlR0IsTSglAOZWJowwNAFOhz2Mil3mJ1oPug7ZgeGNLeioVGROPeHcgf9ePAoQRSeL WeyZa5zNLd16Il+os0G4iJujMnU2fqj0RomEU1S884iUqQ4cwucc6a4D81OJDcskmsmt Qg44/1nAWyyaPLdTqbnkY2bKSsbVVKwL03l70= Received: by 10.68.8.72 with SMTP id p8mr10043338pba.0.1312282883052; Tue, 02 Aug 2011 04:01:23 -0700 (PDT) MIME-Version: 1.0 Received: by 10.68.54.65 with HTTP; Tue, 2 Aug 2011 04:01:03 -0700 (PDT) In-Reply-To: References: <20110802011802.AD4B02388980@eris.apache.org> From: Claus Ibsen Date: Tue, 2 Aug 2011 13:01:03 +0200 Message-ID: Subject: Re: svn commit: r1152991 - in /camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms: JmsConsumer.java JmsEndpoint.java To: dev@camel.apache.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Virus-Checked: Checked by ClamAV on apache.org 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 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, =A0 wrote: >> Author: dkulp >> Date: Tue Aug =A02 01:18:01 2011 >> New Revision: 1152991 >> >> URL: http://svn.apache.org/viewvc?rev=3D1152991&view=3Drev >> Log: >> Use an async process to destroy the JMS DefaultMessageListenerContainer >> as the destroy method can take quite a while. =A0(might be an issue in >> ActiveMQ). =A0 This chops the build time on my machine from 15min 28s >> to =A010min 36s. >> >> Modified: >> =A0 =A0camel/trunk/components/camel-jms/src/main/java/org/apache/camel/c= omponent/jms/JmsConsumer.java >> =A0 =A0camel/trunk/components/camel-jms/src/main/java/org/apache/camel/c= omponent/jms/JmsEndpoint.java >> >> Modified: camel/trunk/components/camel-jms/src/main/java/org/apache/came= l/component/jms/JmsConsumer.java >> URL: http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/m= ain/java/org/apache/camel/component/jms/JmsConsumer.java?rev=3D1152991&r1= =3D1152990&r2=3D1152991&view=3Ddiff >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- camel/trunk/components/camel-jms/src/main/java/org/apache/camel/comp= onent/jms/JmsConsumer.java (original) >> +++ camel/trunk/components/camel-jms/src/main/java/org/apache/camel/comp= onent/jms/JmsConsumer.java Tue Aug =A02 01:18:01 2011 >> @@ -127,7 +127,7 @@ public class JmsConsumer extends Default >> =A0 =A0 protected void doStop() throws Exception { >> =A0 =A0 =A0 =A0 if (listenerContainer !=3D null) { >> =A0 =A0 =A0 =A0 =A0 =A0 listenerContainer.stop(); >> - =A0 =A0 =A0 =A0 =A0 =A0listenerContainer.destroy(); >> + =A0 =A0 =A0 =A0 =A0 =A0((JmsEndpoint)getEndpoint()).destroyMessageList= enerContainer(listenerContainer); >> =A0 =A0 =A0 =A0 } >> >> =A0 =A0 =A0 =A0 // null container and listener so they are fully re crea= ted if this consumer is restarted >> >> Modified: camel/trunk/components/camel-jms/src/main/java/org/apache/came= l/component/jms/JmsEndpoint.java >> URL: http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/m= ain/java/org/apache/camel/component/jms/JmsEndpoint.java?rev=3D1152991&r1= =3D1152990&r2=3D1152991&view=3Ddiff >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- camel/trunk/components/camel-jms/src/main/java/org/apache/camel/comp= onent/jms/JmsEndpoint.java (original) >> +++ camel/trunk/components/camel-jms/src/main/java/org/apache/camel/comp= onent/jms/JmsEndpoint.java Tue Aug =A02 01:18:01 2011 >> @@ -81,6 +81,7 @@ public class JmsEndpoint extends Default >> =A0 =A0 // scheduled executor to check for timeout (reply not received) >> =A0 =A0 private ScheduledExecutorService replyManagerExecutorService; >> =A0 =A0 private final AtomicBoolean running =3D new AtomicBoolean(); >> + =A0 =A0private volatile boolean destroying; >> >> =A0 =A0 public JmsEndpoint() { >> =A0 =A0 =A0 =A0 this(null, null); >> @@ -155,9 +156,30 @@ public class JmsEndpoint extends Default >> =A0 =A0 } >> >> =A0 =A0 public JmsConsumer createConsumer(Processor processor) throws Ex= ception { >> + =A0 =A0 =A0 =A0synchronized (this) { >> + =A0 =A0 =A0 =A0 =A0 =A0while (destroying) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wait(); >> + =A0 =A0 =A0 =A0 =A0 =A0} >> + =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 =A0 DefaultMessageListenerContainer listenerContainer =3D cr= eateMessageListenerContainer(); >> =A0 =A0 =A0 =A0 return createConsumer(processor, listenerContainer); >> =A0 =A0 } >> + >> + =A0 =A0private void destroyMessageListenerContainerInternal(DefaultMes= sageListenerContainer listenerContainer) { >> + =A0 =A0 =A0 =A0listenerContainer.destroy(); >> + =A0 =A0 =A0 =A0destroying =3D false; >> + =A0 =A0 =A0 =A0synchronized (this) { >> + =A0 =A0 =A0 =A0 =A0 =A0notifyAll(); >> + =A0 =A0 =A0 =A0} >> + =A0 =A0} >> + =A0 =A0public void destroyMessageListenerContainer(final DefaultMessag= eListenerContainer listenerContainer) { >> + =A0 =A0 =A0 =A0destroying =3D true; >> + =A0 =A0 =A0 =A0this.getReplyManagerExecutorService().execute(new Runna= ble() { >> + =A0 =A0 =A0 =A0 =A0 =A0public void run() { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0destroyMessageListenerContainerInternal= (listenerContainer); >> + =A0 =A0 =A0 =A0 =A0 =A0} >> + =A0 =A0 =A0 =A0}); >> + =A0 =A0} >> >> =A0 =A0 public DefaultMessageListenerContainer createMessageListenerCont= ainer() throws Exception { >> =A0 =A0 =A0 =A0 return configuration.createMessageListenerContainer(this= ); >> @@ -1007,6 +1029,7 @@ public class JmsEndpoint extends Default >> =A0 =A0 =A0 =A0 return super.createEndpointUri(); >> =A0 =A0 } >> >> + >> >> >> =A0} >> >> >> > > > > -- > Claus Ibsen > ----------------- > FuseSource > Email: cibsen@fusesource.com > Web: http://fusesource.com > Twitter: davsclaus, fusenews > Blog: http://davsclaus.blogspot.com/ > Author of Camel in Action: http://www.manning.com/ibsen/ > --=20 Claus Ibsen ----------------- FuseSource Email: cibsen@fusesource.com Web: http://fusesource.com Twitter: davsclaus, fusenews Blog: http://davsclaus.blogspot.com/ Author of Camel in Action: http://www.manning.com/ibsen/