[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #17483: [fix][broker] Multiple consumer dispatcher stuck when `unackedMessages` greater than `maxUnackedMessages`

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

michaeljmarshall commented on code in PR #17483:
URL: https://github.com/apache/pulsar/pull/17483#discussion_r967673174


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java:
##########
@@ -634,7 +634,9 @@ protected synchronized boolean trySendMessagesToConsumers(ReadType readType, Lis
             // round-robin dispatch batch size for this consumer
             int availablePermits = c.isWritable() ? c.getAvailablePermits() : 1;
             if (c.getMaxUnackedMessages() > 0) {
-                availablePermits = Math.min(availablePermits, c.getMaxUnackedMessages() - c.getUnackedMessages());
+                // Avoid negative number
+                int remainUnAckedMessages = Math.max(c.getMaxUnackedMessages() - c.getUnackedMessages(), 0);

Review Comment:
   > Because in the current implementation, we will add `unackedMessages` after sent messages. if we use batch messages, the `unackedMessages` is probably greater than `MaxUnackedMessages`.
   
   This was the detail I was missing, thank you for explaining. I read through the code a bit more, and it makes sense.



-- 
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: commits-unsubscribe@pulsar.apache.org

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