lhotari commented on code in PR #23774:
URL: https://github.com/apache/pulsar/pull/23774#discussion_r1901683130
##########
pulsar-broker/src/main/java/org/apache/pulsar/common/naming/TopicBundleAssignmentStrategy.java:
##########
@@ -23,5 +23,7 @@
public interface TopicBundleAssignmentStrategy {
NamespaceBundle findBundle(TopicName topicName, NamespaceBundles
namespaceBundles);
+ long getHashCode(String name);
Review Comment:
It's better to make the name of this method more specific. It's better to
use the type `TopicName` as well and provide a default implementation. Please
also add javadoc with proper description of the method.
```suggestion
default long calculateBundleHashCode(TopicName topicName) {
return Hashing.crc32().hashString(topicName.toString(),
StandardCharsets.UTF_8).padToLong();
}
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/common/naming/ConsistentHashingTopicBundleAssigner.java:
##########
@@ -23,18 +23,26 @@
import org.apache.pulsar.broker.PulsarService;
public class ConsistentHashingTopicBundleAssigner implements
TopicBundleAssignmentStrategy {
+ private PulsarService pulsar;
@Override
public NamespaceBundle findBundle(TopicName topicName, NamespaceBundles
namespaceBundles) {
- long hashCode = Hashing.crc32().hashString(topicName.toString(),
StandardCharsets.UTF_8).padToLong();
- NamespaceBundle bundle = namespaceBundles.getBundle(hashCode);
+ NamespaceBundle bundle =
namespaceBundles.getBundle(getHashCode(topicName.toString()));
if (topicName.getDomain().equals(TopicDomain.non_persistent)) {
bundle.setHasNonPersistentTopic(true);
}
return bundle;
}
+ @Override
+ public long getHashCode(String name) {
+ return
pulsar.getNamespaceService().getNamespaceBundleFactory().getHashFunc()
Review Comment:
this doesn't look great since it's a ["train wreck"
anti-pattern](https://wiki.c2.com/?TrainWreck) to have long call chains to
reach out to a dependency. could the hash function be passed in the constructor
instead?
--
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]
