Skip to content

[fix][broker] Support namespace unsubscribe when bundles are unloaded#25276

Open
zhanghaou wants to merge 4 commits intoapache:masterfrom
zhanghaou:fix-namespace-unsub
Open

[fix][broker] Support namespace unsubscribe when bundles are unloaded#25276
zhanghaou wants to merge 4 commits intoapache:masterfrom
zhanghaou:fix-namespace-unsub

Conversation

@zhanghaou
Copy link
Contributor

Motivation

Unsubscribing at namespace/bundle level fails when the target bundle is not loaded by any broker.

This PR makes unsubscribe paths handle unloaded bundles by acquiring ownership when needed and loading topics before unsubscribing.

Modifications

  1. Convert namespace/bundle unsubscribe paths to async internal APIs and update v1/v2 admin endpoints to use async response flow.
  2. Allow bundle ownership acquisition for unsubscribe operations when bundle is not loaded.
  3. Load topics from metadata and ensure they are in memory before removing subscriptions.
  4. Add tests to verify unsubscribe works on unloaded bundles (both namespace and bundle scope).

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 28, 2026
张浩 added 2 commits February 28, 2026 17:03
@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 89.41176% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.61%. Comparing base (4cbe124) to head (db6fe06).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...pache/pulsar/broker/admin/impl/NamespacesBase.java 84.74% 5 Missing and 4 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #25276      +/-   ##
============================================
- Coverage     72.81%   72.61%   -0.21%     
- Complexity    34134    34452     +318     
============================================
  Files          1959     1959              
  Lines        155543   155552       +9     
  Branches      17741    17739       -2     
============================================
- Hits         113263   112947     -316     
- Misses        33322    33571     +249     
- Partials       8958     9034      +76     
Flag Coverage Δ
inttests 25.77% <0.00%> (-0.22%) ⬇️
systests 22.29% <63.52%> (-0.25%) ⬇️
unittests 73.60% <89.41%> (-0.19%) ⬇️

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

Files with missing lines Coverage Δ
.../org/apache/pulsar/broker/admin/v1/Namespaces.java 53.39% <100.00%> (+0.94%) ⬆️
.../org/apache/pulsar/broker/admin/v2/Namespaces.java 90.81% <100.00%> (+0.22%) ⬆️
...pache/pulsar/broker/admin/impl/NamespacesBase.java 77.92% <84.74%> (+0.32%) ⬆️

... and 93 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.

}

@Test(dataProvider = "numBundles")
public void testUnsubscribeNamespaceBundleOnUnloadedBundle(Integer numBundles) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can add some unit tests.

  1. partitioned topic (ensure all subscriptions are deleted on each partition);
  2. Multi-topic cross-bundle;

}

return pulsar().getNamespaceService().getFullListOfTopics(nsName)
.thenCompose(topicsInNamespace -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the logic of NamespaceService#getOwnedTopicListForNamespaceBundle be reused here (or switch to using targetBundle.includes)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants