From dev-return-68811-archive-asf-public=cust-asf.ponee.io@activemq.apache.org  Wed Jan  9 14:22:35 2019
Return-Path: <dev-return-68811-archive-asf-public=cust-asf.ponee.io@activemq.apache.org>
X-Original-To: archive-asf-public@cust-asf.ponee.io
Delivered-To: archive-asf-public@cust-asf.ponee.io
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by mx-eu-01.ponee.io (Postfix) with SMTP id B0C32180676
	for <archive-asf-public@cust-asf.ponee.io>; Wed,  9 Jan 2019 14:22:34 +0100 (CET)
Received: (qmail 70348 invoked by uid 500); 9 Jan 2019 13:22:29 -0000
Mailing-List: contact dev-help@activemq.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@activemq.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@activemq.apache.org>
List-Post: <mailto:dev@activemq.apache.org>
List-Id: <dev.activemq.apache.org>
Reply-To: dev@activemq.apache.org
Delivered-To: mailing list dev@activemq.apache.org
Received: (qmail 68633 invoked by uid 99); 9 Jan 2019 13:22:29 -0000
Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 09 Jan 2019 13:22:29 +0000
Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33)
	id F13BBE1090; Wed,  9 Jan 2019 13:22:28 +0000 (UTC)
From: franz1981 <git@git.apache.org>
To: dev@activemq.apache.org
Reply-To: dev@activemq.apache.org
References: <git-pr-2490-activemq-artemis@git.apache.org>
In-Reply-To: <git-pr-2490-activemq-artemis@git.apache.org>
Subject: [GitHub] activemq-artemis pull request #2490: V2 196
Content-Type: text/plain
Message-Id: <20190109132228.F13BBE1090@git1-us-west.apache.org>
Date: Wed,  9 Jan 2019 13:22:28 +0000 (UTC)

Github user franz1981 commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2490#discussion_r246377787
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ---
    @@ -2497,45 +2484,36 @@ private void deliver() {
     
     
                       handled++;
    -
    +                  consumers.reset();
                       continue;
                    }
     
                    if (logger.isTraceEnabled()) {
                       logger.trace("Queue " + this.getName() + " is delivering reference " + ref);
                    }
     
    -               // If a group id is set, then this overrides the consumer chosen round-robin
    -
    -               SimpleString groupID = extractGroupID(ref);
    -
    -               if (groupID != null) {
    -                  groupConsumer = groups.get(groupID);
    +               final SimpleString groupID = extractGroupID(ref);
    +               groupConsumer = getGroupConsumer(groupConsumer, groupID);
     
    -                  if (groupConsumer != null) {
    -                     consumer = groupConsumer;
    -                  }
    -               }
    -
    -               if (exclusive && redistributor == null) {
    -                  consumer = consumerList.get(0).consumer;
    +               if (groupConsumer != null) {
    +                  consumer = groupConsumer;
                    }
     
                    HandleStatus status = handle(ref, consumer);
     
                    if (status == HandleStatus.HANDLED) {
     
    -                  deliveriesInTransit.countUp();
    -
    -                  handledconsumer = consumer;
    -
    -                  removeMessageReference(holder, ref);
    -
                       if (redistributor == null) {
                          handleMessageGroup(ref, consumer, groupConsumer, groupID);
                       }
     
    +                  deliveriesInTransit.countUp();
    +
    +
    +                  removeMessageReference(holder, ref);
    +                  handledconsumer = consumer;
                       handled++;
    +                  consumers.reset();
    --- End diff --
    
    if `removeMessageReference` would throw any exeption is fine to have `consumers.reset` not called?
    if not, uses `try...finally`


---