Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][broker] Improve the extensibility of the TopicBundleAssignmentStrategy interface class #23773

Open
2 tasks done
rayluoluo opened this issue Dec 24, 2024 · 0 comments · May be fixed by #23774
Open
2 tasks done
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Comments

@rayluoluo
Copy link
Contributor

rayluoluo commented Dec 24, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

During the lookup process of the pulsar broker, the hash value needs to be calculated based on the topic name. For example:

  1. Bundle obtaining triggered by a lookup request:

    Lookup -> NamespaceService#getBrokerServiceUrlAsync -> NamespaceService#getBundleAsync ->
    NamespaceBundles#findBundle -> TopicBundleAssignmentStrategy#findBundle -> NamespaceBundles#getBundle(long hash)

  2. When loading a topic, the broker needs to determine whether it owns the topic partition.

    PulsarService#loadNamespaceTopics -> NamespaceBundle#includes -> NamespaceBundleFactory#getLongHashCode -> NamespaceBundle.keyRange#contains(long hash)

The current code implementation has the following problems:

  1. The hash algorithm is fixed. When the load balancing algorithm is extended, the bundle to which the partition belongs cannot be adjusted. As a result, other algorithms such as RoundRobin cannot be extended.

    The NamespaceBundleFactory#getLongHashCode method uses a fixed algorithm to calculate the hash value. Therefore, it is difficult to extend the implementation of the TopicBundleAssignmentStrategy interface class that uses different hash algorithms without modifying the NamespaceBundleFactory#getLongHashCode method, which violates the open and closed principles.

image

  1. Bad code smell (shot-like modification): The hash algorithm is implemented in the findBundle and getLongHashCode methods. The system must ensure that the calculated hash results are the same. Otherwise, split-brain occurs in the cluster. Therefore, if the hash algorithm needs to be modified, the code has a bad smell.

Solution

It is recommended that the implementation of the NamespaceBundleFactory#getLongHashCode method be moved to the implementation class of the interface TopicBundleAssignmentStrategy. Therefore, we may add a new method long getHashCode(String name) to the TopicBundleAssignmentStrategy interface class. The implementation of the hash algorithm is no longer fixed in the NamespaceBundleFactory#getLongHashCode method. Instead, the getHashCode method implemented by different algorithms is invoked.

Alternatives

No response

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@rayluoluo rayluoluo added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Dec 24, 2024
rayluoluo added a commit to rayluoluo/pulsar that referenced this issue Dec 24, 2024
rayluoluo pushed a commit to rayluoluo/pulsar that referenced this issue Dec 27, 2024
rayluoluo added a commit to rayluoluo/pulsar that referenced this issue Dec 27, 2024
rayluoluo added a commit to rayluoluo/pulsar that referenced this issue Jan 7, 2025
rayluoluo added a commit to rayluoluo/pulsar that referenced this issue Jan 7, 2025
rayluoluo added a commit to rayluoluo/pulsar that referenced this issue Jan 7, 2025
rayluoluo added a commit to rayluoluo/pulsar that referenced this issue Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
1 participant