bharanic-dev commented on a change in pull request #11331:
URL: https://github.com/apache/pulsar/pull/11331#discussion_r671456164
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroup.java
##########
@@ -140,6 +140,7 @@ public ResourceGroup(ResourceGroup other) {
protected void
updateResourceGroup(org.apache.pulsar.common.policies.data.ResourceGroup
rgConfig) {
this.setResourceGroupConfigParameters(rgConfig);
+ this.resourceGroupPublishLimiter = new
ResourceGroupPublishLimiter(rgConfig, rgs.getPulsar().getExecutor());
Review comment:
the resourceGroupPublishLimiter reference is cached in the topic to
avoid a lookup in the datapath for each message that is processed. Instead of
creating a new PublishLimiter, invoke the update() method to update the limits.
##########
File path:
pulsar-broker/src/test/java/org/apache/pulsar/broker/resourcegroup/RGUsageMTAggrWaitForAllMesgsTest.java
##########
@@ -57,7 +57,7 @@
// The tenants and namespaces in those topics are associated with a set of
resource-groups (RGs).
// After sending/receiving all the messages, traffic usage statistics, and
Prometheus-metrics
// are verified on the RGs.
-public class RGUsageMTAggrWaitForAllMesgs extends ProducerConsumerBase {
+public class RGUsageMTAggrWaitForAllMesgsTest extends ProducerConsumerBase {
Review comment:
I have not fully understood what this unit test does. But if you haven't
already, please add unit test to test that the new limits work for the
resourcegroup publish limiter for updates via config and updates via quota
calculation.
--
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]
