I'm sorry for not getting back to you sooner.
>From the description of the proposal. You have figured out the problems
with the current
solution. From my understanding, only the first one is in the scope of this
proposal.
The other two problems, one is about the issue from ThresholdShedder and
another
one is about the load data sync issue.
It would be better to have a section "Scope" to underline that the proposal
is
trying to resolve the first issue. Or we can just describe it in the
motivation section like
---
There are a couple of problems with the current load balancer 1, 2, 3. This
proposal is
trying to resolve the first issue by introducing a pluggable topic bundle
assigner to
allow users to customize the topic bundle assignment according to their use
case.
Problem 2 and 3 is not in the scope of this proposal.
---
Just to make it more clear for reviewers to understand what is in the scope
and not in
the scope.
I remember there was an implementation section for assigning partitions to
different bundles. It will be an excellent example for users to understand
what a customized
topic bundle assigner looks like. I think we can add it back as an example
(not implementation)
For the newly introduced configuration in broker.conf.
I think we'd better use `topicBundleAssignmentStrategy` instead of `
partitionAssignerClassName.`
because it is only for partitions. Users can also implement their own
BundleAssignmentStrategy
for
non-partitioned topics.
For the new methods in the new strategy class:
---
void onBundleSplit(NamespaceBundle bundle);
void onBundleUnload(NamespaceBundle bundle);
void onBundleOwned(NamespaceBundle namespaceBundle);
---
If I understand correctly, there are some cases users might need to handle
the above events
to do some topics unloading or other operations. Is it better to introduce
a listener interface
to the LoadManager? So that we don't need to add the above method in the
AssignmentStrategy
class. For example:
---
new MyTopicBundleAssignmentStrategy(PulsarService pulsar) {
pulsar.getLoadManager().registerListener(...)
}
---
Not all TopicBundleAssignmentStrategy implementations need to handle the
events, right?
So that we can make the new interface better to follow the SRP (Single
Responsibility Principle).
Thanks,
Penghui
On Fri, Apr 21, 2023 at 1:50 PM linlin <lin...@apache.org> wrote:
> Hi Pulsar Community,
>
> This thread is to start the vote for PIP 255.
> After discussion, we believe that it is a better way to plug-in the
> partition assignment strategy,
> and let customers complete the specific implementation.
>
> So the title is change from "Assign topic partitions to bundle by round
> robin" to
> "Make the partition assignment strategy pluggable"
>
> Discussion:
> https://lists.apache.org/thread/pxx715jrqjc219pymskz8xcz57jdmcfy
> Issue: https://github.com/apache/pulsar/issues/19806
>
> Thanks,
> Lin Lin
>
