rayluoluo opened a new issue, #23773: URL: https://github.com/apache/pulsar/issues/23773
### Search before asking - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar. ### Motivation During the lookup process, the pulsar broker needs to determine the bundle to which the topic belongs, depending on the hash value calculated based on the topic name. After the [PIP-255](https://github.com/apache/pulsar/issues/19806) makes the partition assignment strategy pluggable, the process of obtaining the bundle triggered by the lookup is as follows: `Lookup -> NamespaceService#getBrokerServiceUrlAsync -> NamespaceService#getBundleAsync -> NamespaceBundles#findBundle -> TopicBundleAssignmentStrategy#findBundle -> NamespaceBundles#getBundle(long hash)` When determining whether the current broker has the topic, the same hash algorithm needs to be used to calculate the hash value based on the topic name. `NamespaceService#getOwnedTopicListForNamespaceBundle -> NamespaceBundle#includes -> NamespaceBundleFactory#getLongHashCode -> Range#contains(long hash)` ## ### Code bad smell: scatter-point modification Unfortunately, in the current implementation of pulsar broker, the preceding two processes that need to calculate the hash value are implemented in different methods. Once the hash calculation method needs to be adjusted, the hash calculation method needs to be modified in different classes, which has a typical bad code smell problem. The detailed description is as follows: Take the default implementation class `ConsistentHashingTopicBundleAssigner` of the `TopicBundleAssignmentStrategy` interface class as an example. During the lookup process, the hash value is calculated in `ConsistentHashingTopicBundleAssigner#findBundle`. https://github.com/apache/pulsar/blob/1967a9309586286580ac0f3b75a34e1f70e63f75/pulsar-broker/src/main/java/org/apache/pulsar/common/naming/ConsistentHashingTopicBundleAssigner.java#L25-L40 However, the process of determining whether the current broker has owned the topic is to calculate the hash value in the `NamespaceBundleFactory#getLongHashCode` method. https://github.com/apache/pulsar/blob/1967a9309586286580ac0f3b75a34e1f70e63f75/pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java#L204 https://github.com/apache/pulsar/blob/1967a9309586286580ac0f3b75a34e1f70e63f75/pulsar-broker/src/main/java/org/apache/pulsar/common/naming/NamespaceBundleFactory.java#L78-L79 https://github.com/apache/pulsar/blob/1967a9309586286580ac0f3b75a34e1f70e63f75/pulsar-broker/src/main/java/org/apache/pulsar/common/naming/NamespaceBundleFactory.java#L294-L296 ## Insufficient interface scalability In addition, the `NamespaceBundleFactory#getLongHashCode` method uses a fixed algorithm to calculate the hash value. Therefore, it is difficult to extend other interface class implementations using different hash algorithms without modifying the `NamespaceBundleFactory#getLongHashCode` method, which violates the open and closed principles.  ### 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. When the `NamespaceBundleFactory#getLongHashCode` method invokes `getHashCode`, `NamespaceBundles` is unaware of the specific hash algorithm implementation. ### Alternatives _No response_ ### Anything else? _No response_ ### Are you willing to submit a PR? - [X] I'm willing to submit a PR! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
