You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Claus Ibsen <cl...@gmail.com> on 2015/01/04 14:46:34 UTC

Re: [1/2] camel git commit: CAMEL-8204 Write Error log if the JMS correlationId is not unique.

Hi

I suggest that the logging level is warn instead of error.

Also I think we should change the api of the timeout map so the put
method returns the previous value. Then the check is a 1 operation
instead of a get + put which is not atomic.

eg change this
org.apache.camel.TimeoutMap#put

To return V instead of void.



On Sun, Jan 4, 2015 at 1:43 PM,  <ni...@apache.org> wrote:
> Repository: camel
> Updated Branches:
>   refs/heads/master 00ed85017 -> e6aefecef
>
>
> CAMEL-8204 Write Error log if the JMS correlationId is not unique.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/302b4649
> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/302b4649
> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/302b4649
>
> Branch: refs/heads/master
> Commit: 302b46493790227bb7ffec5d48c243059d158b6c
> Parents: 00ed850
> Author: Willem Jiang <wi...@gmail.com>
> Authored: Sun Jan 4 20:41:59 2015 +0800
> Committer: Willem Jiang <wi...@gmail.com>
> Committed: Sun Jan 4 20:41:59 2015 +0800
>
> ----------------------------------------------------------------------
>  .../org/apache/camel/component/jms/reply/QueueReplyManager.java   | 3 +++
>  .../camel/component/jms/reply/TemporaryQueueReplyManager.java     | 3 +++
>  2 files changed, 6 insertions(+)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/camel/blob/302b4649/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> ----------------------------------------------------------------------
> diff --git a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> index e8675b9..59513b2 100644
> --- a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> +++ b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> @@ -51,6 +51,9 @@ public class QueueReplyManager extends ReplyManagerSupport {
>          // add to correlation map
>          QueueReplyHandler handler = new QueueReplyHandler(replyManager, exchange, callback,
>                  originalCorrelationId, correlationId, requestTimeout);
> +        if (correlation.get(correlationId) != null) {
> +            log.error("The correlationId [{}] is not unique, some reply message would be ignored and the request thread could be blocked.", correlationId);
> +        }
>          correlation.put(correlationId, handler, requestTimeout);
>          return correlationId;
>      }
>
> http://git-wip-us.apache.org/repos/asf/camel/blob/302b4649/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> ----------------------------------------------------------------------
> diff --git a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> index f7430eb..269b8fe 100644
> --- a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> +++ b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> @@ -61,6 +61,9 @@ public class TemporaryQueueReplyManager extends ReplyManagerSupport {
>                                  String originalCorrelationId, String correlationId, long requestTimeout) {
>          // add to correlation map
>          TemporaryQueueReplyHandler handler = new TemporaryQueueReplyHandler(this, exchange, callback, originalCorrelationId, correlationId, requestTimeout);
> +        if (correlation.get(correlationId) != null) {
> +            log.error("The correlationId [{}] is not unique, some reply message would be ignored and the request thread could be blocked.", correlationId);
> +        }
>          correlation.put(correlationId, handler, requestTimeout);
>          return correlationId;
>      }
>



-- 
Claus Ibsen
-----------------
Red Hat, Inc.
Email: cibsen@redhat.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen
hawtio: http://hawt.io/
fabric8: http://fabric8.io/

Re: [1/2] camel git commit: CAMEL-8204 Write Error log if the JMS correlationId is not unique.

Posted by Willem Jiang <wi...@gmail.com>.
Hi Claus,

Thanks for pointing that out, I will update the code with your suggestion.


Willem Jiang

Red Hat, Inc.
FuseSource is now part of Red Hat
Web: http://www.fusesource.com | http://www.redhat.com
Blog: http://willemjiang.blogspot.com (http://willemjiang.blogspot.com/)
(English)
          http://jnn.iteye.com (http://jnn.javaeye.com/) (Chinese)
Twitter: willemjiang
Weibo: 姜宁willem

On Sun, Jan 4, 2015 at 9:46 PM, Claus Ibsen <cl...@gmail.com> wrote:

> Hi
>
> I suggest that the logging level is warn instead of error.
>
> Also I think we should change the api of the timeout map so the put
> method returns the previous value. Then the check is a 1 operation
> instead of a get + put which is not atomic.
>
> eg change this
> org.apache.camel.TimeoutMap#put
>
> To return V instead of void.
>
>
>
> On Sun, Jan 4, 2015 at 1:43 PM,  <ni...@apache.org> wrote:
> > Repository: camel
> > Updated Branches:
> >   refs/heads/master 00ed85017 -> e6aefecef
> >
> >
> > CAMEL-8204 Write Error log if the JMS correlationId is not unique.
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/camel/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/302b4649
> > Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/302b4649
> > Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/302b4649
> >
> > Branch: refs/heads/master
> > Commit: 302b46493790227bb7ffec5d48c243059d158b6c
> > Parents: 00ed850
> > Author: Willem Jiang <wi...@gmail.com>
> > Authored: Sun Jan 4 20:41:59 2015 +0800
> > Committer: Willem Jiang <wi...@gmail.com>
> > Committed: Sun Jan 4 20:41:59 2015 +0800
> >
> > ----------------------------------------------------------------------
> >  .../org/apache/camel/component/jms/reply/QueueReplyManager.java   | 3
> +++
> >  .../camel/component/jms/reply/TemporaryQueueReplyManager.java     | 3
> +++
> >  2 files changed, 6 insertions(+)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/camel/blob/302b4649/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> > ----------------------------------------------------------------------
> > diff --git
> a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> > index e8675b9..59513b2 100644
> > ---
> a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> > +++
> b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/QueueReplyManager.java
> > @@ -51,6 +51,9 @@ public class QueueReplyManager extends
> ReplyManagerSupport {
> >          // add to correlation map
> >          QueueReplyHandler handler = new QueueReplyHandler(replyManager,
> exchange, callback,
> >                  originalCorrelationId, correlationId, requestTimeout);
> > +        if (correlation.get(correlationId) != null) {
> > +            log.error("The correlationId [{}] is not unique, some reply
> message would be ignored and the request thread could be blocked.",
> correlationId);
> > +        }
> >          correlation.put(correlationId, handler, requestTimeout);
> >          return correlationId;
> >      }
> >
> >
> http://git-wip-us.apache.org/repos/asf/camel/blob/302b4649/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> > ----------------------------------------------------------------------
> > diff --git
> a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> > index f7430eb..269b8fe 100644
> > ---
> a/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> > +++
> b/components/camel-jms/src/main/java/org/apache/camel/component/jms/reply/TemporaryQueueReplyManager.java
> > @@ -61,6 +61,9 @@ public class TemporaryQueueReplyManager extends
> ReplyManagerSupport {
> >                                  String originalCorrelationId, String
> correlationId, long requestTimeout) {
> >          // add to correlation map
> >          TemporaryQueueReplyHandler handler = new
> TemporaryQueueReplyHandler(this, exchange, callback, originalCorrelationId,
> correlationId, requestTimeout);
> > +        if (correlation.get(correlationId) != null) {
> > +            log.error("The correlationId [{}] is not unique, some reply
> message would be ignored and the request thread could be blocked.",
> correlationId);
> > +        }
> >          correlation.put(correlationId, handler, requestTimeout);
> >          return correlationId;
> >      }
> >
>
>
>
> --
> Claus Ibsen
> -----------------
> Red Hat, Inc.
> Email: cibsen@redhat.com
> Twitter: davsclaus
> Blog: http://davsclaus.com
> Author of Camel in Action: http://www.manning.com/ibsen
> hawtio: http://hawt.io/
> fabric8: http://fabric8.io/
>