[improve][broker] Make get list from bundle Admin API async #20652
Conversation
| } | ||
| asyncResponse.resume(topicList); | ||
| } catch (Exception e) { | ||
| log.error("[{}] Failed to list topics on namespace bundle {}/{}", clientAppId(), |
There was a problem hiding this comment.
We can remove try/catch block
| .host(webUrl.get().getHost()) | ||
| .port(webUrl.get().getPort()) | ||
| .replaceQueryParam("authoritative", newAuthoritative); | ||
| if (!ExtensibleLoadManagerImpl.isLoadManagerExtensionEnabled(config())) { |
There was a problem hiding this comment.
Do we need to add this condition? And I'm not sure why need to reset the query param "destinationBroker".
There was a problem hiding this comment.
Yes, for the extensible load manager, the transfer operation needs to use destinationBroker as the destination broker.
There was a problem hiding this comment.
Got it, thanks. Before, the param destinationBroker was only used by the bundle unload command, so it's a little confusing.
The bundle unload command with the param destinationBroker can't work with the extensible load manager, right?
There was a problem hiding this comment.
Before, the destinationBroker is set in the leader broker's ModularLoadManagerImpl#bundleBrokerAffinityMap, so it requires redirecting to the leader and setting the destinationBroker before doing in the real unload operation.
The unload operation requires execution in the owner broker, which will redirect to the owner broker after setting the destinationBroker success. If we don't clean the destinationBroker parameter, it will fall into a loop.
The ExtensibleLoadManagerImpl used a different way to transfer the ownership to destinationBroker, so we do not need to clean the destinationBroker parameter when redirecting. Since the destinationBroker do not set in the leader broker.
For the detail, you can check this method: https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L954
There was a problem hiding this comment.
nit: ppl will appreciate it if you can add these details as a comment.
Demogorgon314
force-pushed
the
Demogorgon314/make-getListFromBundle-async
branch
from
c3fa77c to
45aa66a
Compare
Codecov Report
@@ Coverage Diff @@
## master #20652 +/- ##
============================================
+ Coverage 72.60% 73.17% +0.57%
- Complexity 32018 32068 +50
============================================
Files 1855 1867 +12
Lines 138569 138760 +191
Branches 15250 15264 +14
============================================
+ Hits 100605 101537 +932
+ Misses 29945 29185 -760
- Partials 8019 8038 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
|
(cherry picked from commit 4958f45)
Motivation
See: #14365, and fix when using
ExtensibleLoadManagerImplandvalidateNamespaceBundleOwnershipAsyncmethods. It should redirect to the owner broker.Modifications
Make
NonPersistentTopics#getListFromBundleto pure async.Documentation
docdoc-requireddoc-not-neededdoc-complete