Copilot commented on code in PR #24859:
URL: https://github.com/apache/pulsar/pull/24859#discussion_r2480388235
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java:
##########
@@ -690,23 +714,72 @@ 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(
+ // Returns true if at least one tenant or namespace is registered to
resource group.
Review Comment:
[nitpick] The comment should end with a period for consistency with other
comments in the codebase.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java:
##########
@@ -690,23 +714,72 @@ 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(
+ // Returns true if at least one tenant or namespace is registered to
resource group.
+ private boolean hasActiveResourceGroups() {
+ return !tenantToRGsMap.isEmpty() || !namespaceToRGsMap.isEmpty();
+ }
+
+ /**
+ * Whether the periodic ResourceGroupService tasks (aggregation & quota
calculation) should run.
+ * True only when:
+ * 1. the scheduler flag is set,
+ * 2. at least one Resource Group exists locally, and
+ * 3. at least one tenant or namespace is registered to Resource Group.
+ */
+ private boolean shouldRunPeriodicTasks() {
+ return schedulersRunning.get()
+ && !resourceGroupsMap.isEmpty()
+ && hasActiveResourceGroups();
+ }
+
+ // Start periodic aggregation/quota schedulers if we actually need them.
Review Comment:
[nitpick] The comment should end with a period for consistency with other
comments in the codebase.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java:
##########
@@ -690,23 +714,72 @@ 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(
+ // Returns true if at least one tenant or namespace is registered to
resource group.
+ private boolean hasActiveResourceGroups() {
+ return !tenantToRGsMap.isEmpty() || !namespaceToRGsMap.isEmpty();
+ }
+
+ /**
+ * Whether the periodic ResourceGroupService tasks (aggregation & quota
calculation) should run.
+ * True only when:
+ * 1. the scheduler flag is set,
+ * 2. at least one Resource Group exists locally, and
+ * 3. at least one tenant or namespace is registered to Resource Group.
+ */
+ private boolean shouldRunPeriodicTasks() {
+ return schedulersRunning.get()
+ && !resourceGroupsMap.isEmpty()
+ && hasActiveResourceGroups();
+ }
+
+ // Start periodic aggregation/quota schedulers if we actually need them.
+ private void maybeStartSchedulers() {
+ if (!hasActiveResourceGroups()) {
+ return;
+ }
+ if (schedulersRunning.compareAndSet(false, true)) {
+ final long periodInSecs =
pulsar.getConfiguration().getResourceUsageTransportPublishIntervalInSecs();
+ this.aggregateLocalUsagePeriodInSeconds =
this.resourceUsagePublishPeriodInSeconds = periodInSecs;
+ this.aggregateLocalUsagePeriodicTask =
pulsar.getExecutor().scheduleAtFixedRate(
catchingAndLoggingThrowables(this::aggregateResourceGroupLocalUsages),
- periodInSecs,
- periodInSecs,
- this.timeUnitScale);
- this.calculateQuotaPeriodicTask =
this.pulsar.getExecutor().scheduleAtFixedRate(
+ periodInSecs, periodInSecs, timeUnitScale);
+ this.calculateQuotaPeriodicTask =
pulsar.getExecutor().scheduleAtFixedRate(
catchingAndLoggingThrowables(this::calculateQuotaForAllResourceGroups),
- periodInSecs,
- periodInSecs,
- this.timeUnitScale);
- maxIntervalForSuppressingReportsMSecs =
-
TimeUnit.SECONDS.toMillis(this.resourceUsagePublishPeriodInSeconds) *
MaxUsageReportSuppressRounds;
+ periodInSecs, periodInSecs, timeUnitScale);
+ maxIntervalForSuppressingReportsMSecs =
+
TimeUnit.SECONDS.toMillis(this.resourceUsagePublishPeriodInSeconds) *
MaxUsageReportSuppressRounds;
+ if (log.isInfoEnabled()) {
+ log.info("Started ResourceGroupService periodic tasks with
period={} {}", periodInSecs, timeUnitScale);
+ }
+ }
+ }
+ // Stop schedulers when no tenant or namespace registrations remain.
Review Comment:
[nitpick] The comment should end with a period for consistency with other
comments in the codebase.
--
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]
