You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by lyy4j <gi...@git.apache.org> on 2017/07/28 03:40:46 UTC

[GitHub] incubator-rocketmq pull request #138: add implement SelectMessageQueueByCons...

GitHub user lyy4j opened a pull request:

    https://github.com/apache/incubator-rocketmq/pull/138

    add implement SelectMessageQueueByConsistentHash for MessageQueueSele…

    …ctor
    
    SelectMessageQueueByConsistentHash  user  the  consistent hash  algorithm  to  select msg queue, producer client  can reuse  the instance  for the same topic

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lyy4j/incubator-rocketmq master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-rocketmq/pull/138.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #138
    
----
commit 985f9ed276557cd33c3bc5d1c1f76e396b381051
Author: laiyiyu@100bei.com <la...@100bei.com>
Date:   2017-07-28T03:10:30Z

    add implement SelectMessageQueueByConsistentHash for MessageQueueSelector

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    
    [![Coverage Status](https://coveralls.io/builds/12588933/badge)](https://coveralls.io/builds/12588933)
    
    Coverage decreased (-0.6%) to 38.545% when pulling **985f9ed276557cd33c3bc5d1c1f76e396b381051 on lyy4j:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by lyy4j <gi...@git.apache.org>.
Github user lyy4j commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    @Jaskey ,there are two question ,firstly, what the meaning of havaing try on  ConsistentHashRouter ,I can not find the class;Secondly,  how should I modify my title and link jira issue


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    @lyy4j  please see https://github.com/apache/incubator-rocketmq/blob/0c5e53db6f4d0ed9f25747379a8b679e2da5392d/common/src/main/java/org/apache/rocketmq/common/consistenthash/ConsistentHashRouter.java
    
    
    As to modify the title,just click the edit on the pr, and put [ROCKETMQ-XXX]where XXX the jira number, and, link the jira to the pr description so that the reviewer and visitor know what feature you are going to implement.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by lyy4j <gi...@git.apache.org>.
Github user lyy4j commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    I hava  oopen an issue for this,what should do for next


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    Also you'll need a JIRA issue opened for this, and specified as the description of this pull request.
    Please see other accepted pull requests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    
    [![Coverage Status](https://coveralls.io/builds/12590054/badge)](https://coveralls.io/builds/12590054)
    
    Coverage decreased (-0.7%) to 38.413% when pulling **52d7ff19b6229f97edd0eb35653c2116a3fc99b0 on lyy4j:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by lyy4j <gi...@git.apache.org>.
Github user lyy4j commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
     wanting  to  send   a   message with the same  key  combine with  orderly consumer,   user  can  use   the implements of the MessageQueueSelector, which  has one optimization compared with  SelectMessageQueueByHash when  a broker  crash   ,  it can   reduce the message Arriving out  of the order


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: [ROCKETMQ-138]add implement SelectMessageQueu...

Posted by dongeforever <gi...@git.apache.org>.
Github user dongeforever commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    @lyy4j It is important to put the code of same logic in one place. For easy maintenance, it is suggested to reuse ConsistentHashRouter. If you think that ConsistentHashRouter is not clear and extensible enough, please feel free to polish it.


---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    I will suggest reuse the util class of ConsistentHashRouter which provide a simple interface to implement consistent hashing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: [ROCKETMQ-138]add implement SelectMessageQueu...

Posted by lyy4j <gi...@git.apache.org>.
Github user lyy4j commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    I agree with you  that ConsistentHashRouter has a good enough abstraction, making the code more  cleanner and  extensible. But I think SelectMessageQueueByConsistentHash will not  increase the  difficult of the using and more  easier  to  understand  and friendly to read comparing with using ConsistentHashRouter to implement


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    @lyy4j , also, would you please modify your title and also link jira issue?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #138: [ROCKETMQ-138]add implement SelectMess...

Posted by dongeforever <gi...@git.apache.org>.
Github user dongeforever commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/138#discussion_r140401292
  
    --- Diff: client/src/main/java/org/apache/rocketmq/client/producer/selector/SelectMessageQueueByConsistentHash.java ---
    @@ -0,0 +1,132 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.rocketmq.client.producer.selector;
    +
    +import org.apache.rocketmq.client.producer.MessageQueueSelector;
    +import org.apache.rocketmq.common.message.Message;
    +import org.apache.rocketmq.common.message.MessageQueue;
    +
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.SortedMap;
    +import java.util.TreeMap;
    +
    +/**
    +* wanting to send a message with the same key combine with orderly consumer, user can 
    +* use the implements of the MessageQueueSelector, which has one optimization compared with SelectMessageQueueByHash
    +* when a broker crash , it can reduce the message Arriving out of the order
    +*/
    +public class SelectMessageQueueByConsistentHash implements MessageQueueSelector {
    +
    +    private volatile SortedMap<Integer, String> virtualNodes =
    +            new TreeMap<Integer, String>();
    +
    +    private static final int DEFAULT_VIRTUAL_NODES = 100;
    +
    +    private final int virtualNodeNum;
    +
    +    private volatile HashMap<String, MessageQueue> idToQueueMap = new HashMap<String, MessageQueue>();
    +
    +    public SelectMessageQueueByConsistentHash() {
    +        this.virtualNodeNum = DEFAULT_VIRTUAL_NODES;
    +    }
    +
    +    public SelectMessageQueueByConsistentHash(int virtualNodeNum) {
    +        this.virtualNodeNum = virtualNodeNum;
    +    }
    +
    +    @Override
    +    public MessageQueue select(List<MessageQueue> mqs, Message msg, Object arg) {
    +        synchronized (this) {
    +            if (queueChange(mqs)) {
    +                reloadConsistentHash(mqs);
    +            }
    +
    +            String uniqueQueueId = getMsgQueueIdBy(arg.toString());
    +            MessageQueue messageQueue = idToQueueMap.get(uniqueQueueId);
    +            return messageQueue;
    +        }
    +    }
    +
    +    private boolean queueChange(List<MessageQueue> mqs) {
    +        if (mqs.size() != this.idToQueueMap.size()) {
    +            return true;
    +        }
    +
    +        for (MessageQueue queue : mqs) {
    +            String id = queue.getTopic() + "_" + queue.getBrokerName() + "_" + queue.getQueueId();
    +            if (!this.idToQueueMap.containsKey(id)) {
    +                return true;
    +            }
    +        }
    +
    +        return false;
    +    }
    +
    +    private String getMsgQueueIdBy(String arg) {
    +        int hash = getHash(arg);
    +        SortedMap<Integer, String> subMap = virtualNodes.tailMap(hash);
    +
    +        Integer i;
    +        String virtualNode;
    +
    +        if (subMap.size() == 0) {
    +            i = virtualNodes.firstKey();
    +            virtualNode = virtualNodes.get(i);
    +        } else {
    +            i = subMap.firstKey();
    +            virtualNode = subMap.get(i);
    +        }
    +
    +        String result = virtualNode.substring(0, virtualNode.indexOf("&&"));
    +        return result;
    +    }
    +
    +    private int getHash(String str) {
    --- End diff --
    
    Why not just use str.hashCode?


---

[GitHub] incubator-rocketmq issue #138: [ROCKETMQ-138]add implement SelectMessageQueu...

Posted by lyy4j <gi...@git.apache.org>.
Github user lyy4j commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    I have look at the class which has a good abstract of Consistent Hash ,but I think the class SelectMessageQueueByConsistentHash  is more simple and have the unified style with the original code


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    `wanting to send a message with...`
    yes, this should be the top comment of your class
    
    ```java
    /**
    wanting to send a message with ...
    */
    public class SelectMessageQueueByConsistentHash implements MessageQueueSelector {
    ...
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by lyy4j <gi...@git.apache.org>.
Github user lyy4j commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    I hava  add the  comment of this class  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    
    [![Coverage Status](https://coveralls.io/builds/12590054/badge)](https://coveralls.io/builds/12590054)
    
    Coverage decreased (-0.7%) to 38.413% when pulling **52d7ff19b6229f97edd0eb35653c2116a3fc99b0 on lyy4j:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    
    [![Coverage Status](https://coveralls.io/builds/12605086/badge)](https://coveralls.io/builds/12605086)
    
    Coverage decreased (-0.6%) to 38.54% when pulling **47bd29bdf88d68403dcf45407cd6c32233474309 on lyy4j:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    Hi, @Jaskey could you please have a look at this PR?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    Please see this pull request description https://github.com/apache/incubator-rocketmq/pull/130, for example.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: [ROCKETMQ-138]add implement SelectMessageQueu...

Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    @lyy4j  
    
    1. I will still suggest resuing ConsistentHashRouter since it has a good enough abstraction which will make your selector much cleanner and easier as well as extensible(say specified virtual node nums, hash funtions)
    
    2. I will suggest we expose a more friendly interface for user to use, for example , just let user to implement a shardingKey(MessageQueue q) method, the `select` method should be hidden since it is not friendly enough in this case IMO


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #138: add implement SelectMessageQueueByConsistentH...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/138
  
    
    [![Coverage Status](https://coveralls.io/builds/12605086/badge)](https://coveralls.io/builds/12605086)
    
    Coverage decreased (-0.6%) to 38.54% when pulling **47bd29bdf88d68403dcf45407cd6c32233474309 on lyy4j:master** into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #138: [ROCKETMQ-138]add implement SelectMess...

Posted by dongeforever <gi...@git.apache.org>.
Github user dongeforever commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/138#discussion_r140401564
  
    --- Diff: client/src/main/java/org/apache/rocketmq/client/producer/selector/SelectMessageQueueByConsistentHash.java ---
    @@ -0,0 +1,132 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.rocketmq.client.producer.selector;
    +
    +import org.apache.rocketmq.client.producer.MessageQueueSelector;
    +import org.apache.rocketmq.common.message.Message;
    +import org.apache.rocketmq.common.message.MessageQueue;
    +
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.SortedMap;
    +import java.util.TreeMap;
    +
    +/**
    +* wanting to send a message with the same key combine with orderly consumer, user can 
    +* use the implements of the MessageQueueSelector, which has one optimization compared with SelectMessageQueueByHash
    +* when a broker crash , it can reduce the message Arriving out of the order
    +*/
    +public class SelectMessageQueueByConsistentHash implements MessageQueueSelector {
    +
    +    private volatile SortedMap<Integer, String> virtualNodes =
    --- End diff --
    
    how about  just using <Integer, MessageQueue>?



---