[fix][broker] Run ResourceGroup tasks only when tenants/namespaces registered #24859

vinkal-chudgar · 2025-10-16T12:24:17Z

Motivation

ResourceGroupService unconditionally schedules two periodic tasks (aggregating local usage and calculating quotas) during broker initialization, executing them every resourceUsageTransportPublishIntervalInSecs (default 60 seconds) regardless of whether any tenants or namespaces are registered to resource groups. This wastes resources on brokers where the resource group feature is unused.

This PR implements explicit lifecycle management: periodic tasks now start only when the first tenant or namespace is registered to any resource group, and stop when the last registration is removed. This eliminates unnecessary periodic task execution and resource consumption on brokers where the resource group feature is idle.

Modifications

Lazy scheduling: Periodic tasks (aggregateResourceGroupLocalUsages, calculateQuotaForAllResourceGroups) are no longer unconditionally started in initialize(). They start only when tenant or namespace registrations are added (via registerTenant/registerNameSpace calls) and stop when the last registration is removed.
Automatic cleanup: Tasks stop automatically when the last attachment is removed, eliminating background work when resource groups become unused.
CAS-guarded concurrency: Task lifecycle is controlled via an AtomicBoolean with compare-and-set to avoid double start/stop and blocking.
Dynamic config: Existing dynamic rescheduling based on resourceUsageTransportPublishIntervalInSecs is preserved; guards prevent reschedule attempts when schedulers are stopped.
Docs: Updated the ServiceConfiguration field doc to reflect “run only when in use” and to clarify behavior when a ResourceUsageTransportManager is configured.

Modified files

ResourceGroupService.java: add maybeStartSchedulers(), maybeStopSchedulersIfIdle(), hasActiveAttachments(), plus small guard/logging improvements.
ServiceConfiguration.java: clarify resourceUsageTransportPublishIntervalInSecs doc.
ResourceGroupServiceTest.java: add new tests and adapt existing tests for lazy lifecycle.

Verifying this change

Make sure that the change passes the CI checks.

This change is verified by unit tests. Tests are sleep free, run with per test isolation, and use 60s timeouts.

New unit tests (ResourceGroupServiceTest.java)

testLazyStartStopAndReschedule() verifies lazy start, deterministic reschedule on interval change, and stop on last detach.
testNoStartOnRGCreateOnly() ensures no schedulers start when a resource group has no attachments and direct calls do not auto start.
testStartOnTenantAttachment() confirms that a tenant only attachment starts both schedulers and detaching the last tenant stops them.
testStopOnLastDetachWithMixedRefs() checks that schedulers keep running while any attachment exists and stop only after the last is removed.
testNoRescheduleWhenStopped() ensures interval changes do not schedule tasks when the service is idle.

Updated unit tests (ResourceGroupServiceTest.java)

testClose(): start schedulers via an attachment, call close(), then assert scheduled task references are set to null and isSchedulersRunning() is false.
testResourceGroupOps(): The test now calls calculateQuotaForAllResourceGroups() before unregistering all attachments, rather than after.

Test utilities

isSchedulersRunning() accessor annotated with com.google.common.annotations.VisibleForTesting.
createResourceGroupService() helper that returns a fresh ResourceGroupService(pulsar) per test.

Personal CI Results

Tested in Personal CI fork: vinkal-chudgar#1

Status:

43 checks passed (build, tests, validation)
3 coverage upload jobs failed due to missing Codecov token (expected in forks)
All core test suites passed successfully

Evidence: https://github.com/vinkal-chudgar/pulsar/pull/1/checks

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

Configuration note: Only the documentation string for resourceUsageTransportPublishIntervalInSecs was updated. No default values were changed.

Documentation

doc
doc-required
doc-not-needed
doc-complete

Note: Inline configuration documentation was updated in ServiceConfiguration.java. No website docs need changes

Matching PR in forked repository

PR in forked repository: vinkal-chudgar#1

…idle ResourceGroupService previously executed periodic tasks unconditionally every resourceUsageTransportPublishIntervalInSecs. This change starts both tasks only when at least one tenant or namespace is attached to any resource group, and stops them when no attachments remain. Each task reschedules itself if the interval changes at runtime. close() now cancels and clears both futures. Fixes apache#24693 Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com>

Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com>

github-actions · 2025-10-16T12:24:44Z

@vinkal-chudgar Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

lhotari · 2025-10-16T14:47:17Z

Thanks for the great contribution! I didn't do a full review yet. A few minor comments.

vinkal-chudgar · 2025-10-17T07:09:31Z

Thanks for the great contribution! I didn't do a full review yet. A few minor comments.

Thanks for the quick look! I'll address the comments when available.

vinkal-chudgar · 2025-10-23T05:24:53Z

@lhotari - When convenient, could you please review this PR?

pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java

…ks; adopt 'registered' terminology - Moved the duplicated early-return checks from both periodic methods aggregateResourceGroupLocalUsages and calculateQuotaForAllResourceGroups into a single helper: shouldRunPeriodicTasks - Rename hasActiveAttachments() to hasActiveResourceGroups() - Update comments/logs to use 'registered' consistently. No functional change. Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com>

vinkal-chudgar · 2025-10-31T06:59:56Z

@lhotari - Thanks for the review. I've addressed both comments. Could you please take a look when you have a moment?

lhotari

LGTM. Good work @vinkal-chudgar

Copilot

Pull Request Overview

This PR introduces lazy initialization and lifecycle management for ResourceGroupService periodic schedulers. The schedulers now start only when the first tenant or namespace is attached to a resource group and stop automatically when all attachments are removed, reducing unnecessary overhead when resource groups exist but are not actively used.

Adds lazy scheduler startup/shutdown triggered by tenant/namespace registrations
Implements deterministic rescheduling when publish intervals change dynamically
Ensures proper cleanup of scheduler state in close() method

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
ResourceGroupService.java	Implements lazy scheduler lifecycle with `maybeStartSchedulers()` and `maybeStopSchedulersIfIdle()` methods, adds guards to periodic tasks, and ensures proper cleanup with null-setting of futures
ResourceGroupServiceTest.java	Adds comprehensive tests for lazy scheduler lifecycle, rescheduling behavior, and close() cleanup; fixes test ordering issue in `testResourceGroupOps()`
ServiceConfiguration.java	Updates documentation for `resourceUsageTransportPublishIntervalInSecs` to explain the new lazy scheduler behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java

lhotari · 2025-10-31T10:03:36Z

/pulsarbot rerun-failure-checks

codecov-commenter · 2025-10-31T10:19:19Z

Codecov Report

❌ Patch coverage is 80.43478% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.28%. Comparing base (a091ea7) to head (aeb3547).
⚠️ Report is 28 commits behind head on master.

Files with missing lines	Patch %	Lines
...sar/broker/resourcegroup/ResourceGroupService.java	80.43%	0 Missing and 9 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #24859      +/-   ##
============================================
- Coverage     74.29%   74.28%   -0.01%     
- Complexity    33825    33855      +30     
============================================
  Files          1913     1913              
  Lines        149281   149365      +84     
  Branches      17325    17342      +17     
============================================
+ Hits         110902   110961      +59     
- Misses        29540    29546       +6     
- Partials       8839     8858      +19

Flag	Coverage Δ
inttests	`26.31% <10.86%> (-0.10%)`	⬇️
systests	`22.77% <10.86%> (-0.04%)`	⬇️
unittests	`73.81% <80.43%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...org/apache/pulsar/broker/ServiceConfiguration.java	`98.20% <ø> (ø)`
...sar/broker/resourcegroup/ResourceGroupService.java	`78.27% <80.43%> (+5.17%)`	⬆️

... and 88 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…gistered (#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142)

…gistered (apache#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142) (cherry picked from commit f535106)

…gistered (apache#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142) (cherry picked from commit aaabe70)

…gistered (apache#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142) (cherry picked from commit f535106)

vinkal-chudgar added 4 commits October 15, 2025 14:30

ci: trigger workflows

606d59f

ci: retrigger PR checks

85e91c6

Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com>

refine initialize() comments for clarity

90b31af

Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com>

github-actions bot added the doc-label-missing label Oct 16, 2025

github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Oct 16, 2025

vinkal-chudgar marked this pull request as ready for review October 16, 2025 14:30

lhotari reviewed Oct 28, 2025

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java Outdated Show resolved Hide resolved

pulsar-broker/src/main/java/org/apache/pulsar/broker/resourcegroup/ResourceGroupService.java Outdated Show resolved Hide resolved

lhotari approved these changes Oct 31, 2025

View reviewed changes

lhotari requested a review from Copilot October 31, 2025 07:23

lhotari added this to the 4.2.0 milestone Oct 31, 2025

lhotari added release/4.1.2 release/4.0.8 labels Oct 31, 2025

lhotari assigned vinkal-chudgar Oct 31, 2025

lhotari added the release/3.0.15 label Oct 31, 2025

lhotari requested review from rdhabalia and vineeth1995 October 31, 2025 07:25

Copilot AI reviewed Oct 31, 2025

View reviewed changes

lhotari merged commit 06f7142 into apache:master Oct 31, 2025
103 of 107 checks passed

lhotari added the release/3.3.10 label Oct 31, 2025

lhotari pushed a commit that referenced this pull request Oct 31, 2025

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces re…

f535106

…gistered (#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142)

lhotari pushed a commit that referenced this pull request Oct 31, 2025

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces re…

97f29a1

…gistered (#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142)

lhotari added cherry-picked/branch-4.1 cherry-picked/branch-4.0 labels Oct 31, 2025

lhotari pushed a commit that referenced this pull request Oct 31, 2025

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces re…

aaabe70

…gistered (#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142)

lhotari added the cherry-picked/branch-3.0 label Oct 31, 2025

lhotari pushed a commit that referenced this pull request Oct 31, 2025

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces re…

026de44

…gistered (#24859) Signed-off-by: Vinkal Chudgar <vinkal.chudgar@gmail.com> (cherry picked from commit 06f7142)

lhotari added the cherry-picked/branch-3.3 label Oct 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces registered #24859

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces registered #24859

vinkal-chudgar commented Oct 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

lhotari commented Oct 16, 2025

Uh oh!

vinkal-chudgar commented Oct 17, 2025

Uh oh!

vinkal-chudgar commented Oct 23, 2025

Uh oh!

Uh oh!

Uh oh!

vinkal-chudgar commented Oct 31, 2025

Uh oh!

lhotari left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhotari commented Oct 31, 2025

Uh oh!

codecov-commenter commented Oct 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces registered #24859

[fix][broker] Run ResourceGroup tasks only when tenants/namespaces registered #24859

Conversation

vinkal-chudgar commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Personal CI Results

Does this pull request potentially affect one of the following parts:

Documentation

Matching PR in forked repository

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

lhotari commented Oct 16, 2025

Uh oh!

vinkal-chudgar commented Oct 17, 2025

Uh oh!

vinkal-chudgar commented Oct 23, 2025

Uh oh!

Uh oh!

Uh oh!

vinkal-chudgar commented Oct 31, 2025

Uh oh!

lhotari left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhotari commented Oct 31, 2025

Uh oh!

codecov-commenter commented Oct 31, 2025

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vinkal-chudgar commented Oct 16, 2025 •

edited

Loading