gemmellr commented on code in PR #4606:
URL: https://github.com/apache/activemq-artemis/pull/4606#discussion_r1318474790


##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQConsumer.java:
##########
@@ -293,6 +295,7 @@ public int handleDeliver(MessageReference reference, 
ICoreMessage message) {
          }
          //handleDeliver is performed by an executor (see JBPAPP-6030): any 
AMQConsumer can share the session.wireFormat()
          dispatch = OpenWireMessageConverter.createMessageDispatch(reference, 
message, session.wireFormat(), this, 
session.getCoreServer().getNodeManager().getUUID());
+         
dispatch.getMessage().getMessageId().setBrokerSequenceId(deliveredSequenceId.getAndAdd(1));

Review Comment:
   The createMessageDispatch method already looks to set this, to the existing 
different value...perhaps it should be changed to pass this value in there to 
set, helping avoid any inconsistency later.
   
   Also getAndAdd - > getAndIncrement ?



##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java:
##########
@@ -656,6 +656,7 @@ public void removeItself() throws Exception {
          browserDeliverer.close();
       } else {
          messageQueue.removeConsumer(this);
+         messageQueue.deliverAsync();

Review Comment:
   Kind of wonder if it shouldnt be the queue doing this instead of the 
consumer? Also feels like it may be a separate change really.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to