[GitHub] [pulsar] Jason918 commented on a change in pull request #14400: [PIP-74] Dependency of consumer client memory limit: support dynamic limit of consumer receiver queue

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

Jason918 commented on a change in pull request #14400:
URL: https://github.com/apache/pulsar/pull/14400#discussion_r814727187



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -1610,6 +1607,16 @@ public void increaseAvailablePermits(int delta) {
         increaseAvailablePermits(cnx(), delta);
     }
 
+    public void setMaxReceiverQueueSize(int newSize) {

Review comment:
       > Do you think it'd be worth verifying that the `newSize` is positive? I see that the previous implementation did not verify the input, but a non-positive value will completely halt the consumer. Although, it's possible that someone was using that as a feature to effectively "pause" the consumer. Maybe we need to leave it as is. Let me know what you think.
   
   Added `checkArgument`. And we already have `pause` method, it's too tricky to do negative queue size, let's not encourage this.

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -1610,6 +1607,16 @@ public void increaseAvailablePermits(int delta) {
         increaseAvailablePermits(cnx(), delta);
     }
 
+    public void setMaxReceiverQueueSize(int newSize) {

Review comment:
       > Nit: I think we want to add the `@Override` annotation to this method.
   
   done
   




-- 
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