poorbarcode commented on code in PR #24799:
URL: https://github.com/apache/pulsar/pull/24799#discussion_r2415453063


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupPublishLimiter.java:
##########
@@ -30,7 +31,13 @@ public class ResourceGroupPublishLimiter extends 
PublishRateLimiterImpl  {
     private volatile long publishMaxByteRate;
 
     public ResourceGroupPublishLimiter(ResourceGroup resourceGroup, 
MonotonicClock monotonicClock) {
-        super(monotonicClock);
+        super(monotonicClock, producer -> {
+            producer.getCnx().getThrottleTracker().markThrottled(
+                ServerCnxThrottleTracker.ThrottleType.BrokerPublishRate);
+        }, producer -> {
+            producer.getCnx().getThrottleTracker().unmarkThrottled(
+                ServerCnxThrottleTracker.ThrottleType.BrokerPublishRate);

Review Comment:
   > should use ResourceGroupPublishRate here?
   
   Thanks for mentioning this mistake. I have changed it
   
   > just wondering if the ResourceGroup solution allows multiple 
ResourceGroupPublishLimiters being active at once. In that case a single 
ResourceGroupPublishRate wouldn't be sufficient.
   
   Since the `markThrottled` is reentrant if the type is 
`ResourceGroupPublishRate`, it should work. Could you explain more details? 



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