[GitHub] sijie commented on a change in pull request #1029: Fix MessageRouter hash inconsistent on C++/Java client

[GitBox <gi...@apache.org>]

sijie commented on a change in pull request #1029: Fix MessageRouter hash inconsistent on C++/Java client
URL: https://github.com/apache/incubator-pulsar/pull/1029#discussion_r159749755
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/SinglePartitionMessageRouterImpl.java
 ##########
 @@ -18,26 +18,37 @@
  */
 package org.apache.pulsar.client.impl;
 
+import java.nio.charset.StandardCharsets;
 import java.util.Random;
 
+import com.google.common.hash.HashCode;
+import com.google.common.hash.HashFunction;
+import com.google.common.hash.Hashing;
 import org.apache.pulsar.client.api.Message;
 import org.apache.pulsar.client.api.MessageRouter;
 
-public class SinglePartitionMessageRouterImpl implements MessageRouter {
+public class SinglePartitionMessageRouterImpl extends MessageRouterBase {
 
     private final int partitionIndex;
-    private final int numPartitions;
 
-    public SinglePartitionMessageRouterImpl(int numPartitions) {
+    public SinglePartitionMessageRouterImpl(int numPartitions, boolean useMurmurHash) {
+        super(numPartitions, useMurmurHash);
         this.partitionIndex = new Random().nextInt(numPartitions);
-        this.numPartitions = numPartitions;
     }
 
     @Override
     public int choosePartition(Message msg) {
         // If the message has a key, it supersedes the single partition routing policy
         if (msg.hasKey()) {
-            return ((msg.getKey().hashCode() & Integer.MAX_VALUE) % numPartitions);
+            if (useMurmurHash) {
+                HashFunction hf = Hashing.murmur3_32(0);
 
 Review comment:
   I think this logic here is not allocation-friendly. It generates two objects `HashFunction` and `HashCode` per message here.
   
   I would suggest using a MurmurHash class that is a static method take takes a bytes array or a string to complete a hash value, rather than using guava `HashFunction` or `Hasher`. E.g.
   
   ```
   
   public class MurmurHash3 {
   
        public static int hash32(ByteBuf data, int offset, int length, int seed) { ... }
   
   }
   ```
   
   Another suggestion - it would be good to write a jmh microbenchmark, testing the difference between using MurmurHash3 and simple hash.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services