You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "Demogorgon314 (via GitHub)" <gi...@apache.org> on 2026/04/14 09:46:21 UTC
[PR] [improve][broker] Use full bundle name for namespace bundle destination affinity in ModularLoadManagerImpl [pulsar]
Demogorgon314 opened a new pull request, #25518:
URL: https://github.com/apache/pulsar/pull/25518
### Motivation
When unloading a namespace bundle with `destinationBroker` under `ModularLoadManagerImpl`, the affinity key was stored by `bundleRange` only.
Different namespaces can share the same bundle range, especially for single-bundle namespaces such as `0x00000000_0xffffffff`. In that case, the destination affinity can collide across namespaces, and the next lookup may
consume the affinity for the wrong bundle.
As a result, `unload --destinationBroker` may appear ineffective or unstable when transferring multiple bundles that share the same range.
### Modifications
- Store namespace bundle destination affinity by full bundle name instead of bare `bundleRange`.
- Consume the destination affinity in `ModularLoadManagerWrapper#getLeastLoaded` by full bundle name as well.
- Add an admin-side test to verify the affinity key passed to the load manager is the full bundle name.
- Add a load manager test to verify affinity lookup is resolved by full bundle name.
### Does this pull request potentially affect one of the following parts:
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*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
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][broker] Use full bundle name for namespace bundle destination affinity in ModularLoadManagerImpl [pulsar]
Posted by "Demogorgon314 (via GitHub)" <gi...@apache.org>.
Demogorgon314 commented on code in PR #25518:
URL: https://github.com/apache/pulsar/pull/25518#discussion_r3079109527
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -1395,6 +1395,15 @@ public CompletableFuture<Void> setNamespaceBundleAffinityAsync(String bundleRang
if (StringUtils.isBlank(destinationBroker)) {
return CompletableFuture.completedFuture(null);
}
+ final String bundleName;
+ try {
+ bundleName = pulsar().getNamespaceService().getNamespaceBundleFactory()
+ .getBundle(namespaceName.toString(), bundleRange)
+ .toString();
+ } catch (RuntimeException e) {
+ return FutureUtil.failedFuture(e);
+ }
Review Comment:
But if some one called `setNamespaceBundleAffinityAsync` on out of `thenCompose` it will cause unexpect behavior.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][broker] Use full bundle name for namespace bundle destination affinity in ModularLoadManagerImpl [pulsar]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #25518:
URL: https://github.com/apache/pulsar/pull/25518#discussion_r3078777498
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -1395,6 +1395,15 @@ public CompletableFuture<Void> setNamespaceBundleAffinityAsync(String bundleRang
if (StringUtils.isBlank(destinationBroker)) {
return CompletableFuture.completedFuture(null);
}
+ final String bundleName;
+ try {
+ bundleName = pulsar().getNamespaceService().getNamespaceBundleFactory()
+ .getBundle(namespaceName.toString(), bundleRange)
+ .toString();
+ } catch (RuntimeException e) {
+ return FutureUtil.failedFuture(e);
+ }
Review Comment:
No need to catch unchecked exception here.
This method is called in `thenCompose`, any uncaught exception will be reflected in the final returned future.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][broker] Use full bundle name for namespace bundle destination affinity in ModularLoadManagerImpl [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #25518:
URL: https://github.com/apache/pulsar/pull/25518
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][broker] Use full bundle name for namespace bundle destination affinity in ModularLoadManagerImpl [pulsar]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #25518:
URL: https://github.com/apache/pulsar/pull/25518#discussion_r3078581449
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -1395,6 +1395,9 @@ public CompletableFuture<Void> setNamespaceBundleAffinityAsync(String bundleRang
if (StringUtils.isBlank(destinationBroker)) {
return CompletableFuture.completedFuture(null);
}
+ String bundleName = pulsar().getNamespaceService().getNamespaceBundleFactory()
+ .getBundle(namespaceName.toString(), bundleRange)
+ .toString();
Review Comment:
`setNamespaceBundleAffinityAsync` now computes `bundleName` synchronously via `getNamespaceBundleFactory().getBundle(...)`. If `bundleRange` is null/invalid this can throw before a `CompletableFuture` is returned, which can turn a bad request into an immediate server-side exception and also violates the project guidance that async methods should propagate failures via the returned future. Consider validating `bundleRange` up-front and wrapping bundle lookup in try/catch to return `FutureUtil.failedFuture(...)`, or defer bundleName computation into the async chain (e.g., inside `thenApply`).
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org