[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3742: ARTEMIS-2007 - refactor to make use of existing refCountForConsumers …

[GitBox <gi...@apache.org>]

gemmellr commented on a change in pull request #3742:
URL: https://github.com/apache/activemq-artemis/pull/3742#discussion_r707443939



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##########
@@ -3082,9 +3055,7 @@ private boolean deliver() {
                   numNoMatch = 0;
                   numAttempts = 0;
 
-                  if (consumer != redistributor) {
-                     ref = handleMessageGroup(ref, consumer, groupConsumer, groupID);
-                  }
+                  ref = handleMessageGroup(ref, consumer, groupConsumer, groupID);

Review comment:
       If the redistributor now handles groups and didnt before, does it need to do anything extra when it is cancelled that it wasnt doing before (since it didnt need to) that e.g other consumers currently do when removed?

##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##########
@@ -218,17 +218,17 @@ private void checkIDSupplier(NodeStore<MessageReference> nodeStore) {
 
    protected final ScheduledDeliveryHandler scheduledDeliveryHandler;
 
-   private AtomicLong messagesAdded = new AtomicLong(0);
+   private final AtomicLong messagesAdded = new AtomicLong(0);
 
-   private AtomicLong messagesAcknowledged = new AtomicLong(0);
+   private final AtomicLong messagesAcknowledged = new AtomicLong(0);
 
-   private AtomicLong ackAttempts = new AtomicLong(0);
+   private final AtomicLong ackAttempts = new AtomicLong(0);

Review comment:
       It should certainly be in a separate commit if it is done, it has nothing to do with this 'refactoring' (arguably neither do the finals).

##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##########
@@ -179,7 +179,7 @@
 
    protected final PageSubscription pageSubscription;
 
-   private ReferenceCounter refCountForConsumers;
+   protected final ReferenceCounter refCountForConsumers;

Review comment:
       Why 'protected'? It was private before and doesnt seem to be used elsewhere now, and there was/is still a getter if it is.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org