vinkal-chudgar commented on code in PR #24859:
URL: https://github.com/apache/pulsar/pull/24859#discussion_r2472316195


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java:
##########
@@ -690,23 +720,60 @@ protected void calculateQuotaForAllResourceGroups() {
         }
     }
 
-    private void initialize() {
-        ServiceConfiguration config = this.pulsar.getConfiguration();
-        long periodInSecs = 
config.getResourceUsageTransportPublishIntervalInSecs();
-        this.aggregateLocalUsagePeriodInSeconds = 
this.resourceUsagePublishPeriodInSeconds = periodInSecs;
-        this.aggregateLocalUsagePeriodicTask = 
this.pulsar.getExecutor().scheduleAtFixedRate(
+    // True if at least one tenant or namespace is attached to any RG.
+    private boolean hasActiveAttachments() {

Review Comment:
   Good call. The broker API uses `registerTenant` / `registerNameSpace`, while 
PIP-82 and REST docs use attach/detach. Per your suggestion, I renamed the 
helper to `hasActiveResourceGroups()` and clarified the comment:
   
   ```
      // Returns true if at least one tenant or namespace is registered to 
resource group.
       private boolean hasActiveResourceGroups() {
           return !tenantToRGsMap.isEmpty() || !namespaceToRGsMap.isEmpty();
       }
   ```
        
   Here “active” means there is at least one tenant or namespace registered to 
a resource group on this broker. In `ResourceGroupService`, local usage is only 
tracked for topics whose tenant or namespace is registered to a resource group; 
if there are no registrations, there is no resource group usage to aggregate on 
this broker and no quota to compute on this broker.
   
   For starting and stopping the periodic tasks, the code keeps the decision 
based on registrations:
   
   - `maybeStartSchedulers()` starts when there is at least one registration.
   - `maybeStopSchedulersIfIdle()` stops when none remain.
   
   At runtime, the periodic methods use `shouldRunPeriodicTasks()`, which 
requires all of the following:
   
   1. the scheduler flag is set,
   2. at least one resource group exists locally,
   3. `hasActiveResourceGroups()` is true (at least one registration).
   
   This aligns with your naming recommendation.
   
   Done in aeb35470be3b07fc68a3e0e73c972f9b14bc4294
   



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