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

[ROCKETMQ-67] Consistent Hash allocate strategy #67

Closed
wants to merge 1 commit into from
Closed

[ROCKETMQ-67] Consistent Hash allocate strategy #67

wants to merge 1 commit into from

Conversation

Jaskey
Copy link
Contributor

@Jaskey Jaskey commented Feb 20, 2017

JIRA: https://issues.apache.org/jira/browse/ROCKETMQ-67

Consitent hash alogrithm to allocate message queue to prevent "thundering herd".

Core implementation:

Define a ConsitentHashRouter to hash clientId to a hash ring. And try to find the nearest clientId for every message queue.

High scalability considered, there are several new class to support as many scenarios as possible when user needs to use consitent hash

  1. Any types of object can be routed as long as they implement Node interface.
  2. Custom hash function can be specified
  3. Numbers of virtual nodes can be specifed.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 31.603% when pulling a56df6a on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 31.603% when pulling a56df6a on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 31.603% when pulling a56df6a on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

package org.apache.rocketmq.common.consistenthash;

/**
* Created by jaskeylin on 2017/2/20.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, please remove the author info.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry , i didn'it notice that. Just a minute

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 31.673% when pulling fa0d936 on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 31.673% when pulling fa0d936 on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 31.673% when pulling fa0d936 on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 31.571% when pulling d96a23f on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 31.571% when pulling d96a23f on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 31.571% when pulling d96a23f on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 31.677% when pulling 17379ca on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 31.677% when pulling 17379ca on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 31.677% when pulling 17379ca on Jaskey:ROCKETMQ-67-Consistent-Hash into 573b22c on apache:master.

@Jaskey
Copy link
Contributor Author

Jaskey commented Mar 9, 2017

@zhouxinyu @lizhanhui @shroman @vongosling
any advice on this pr?

Since I think consistent hash can be also applied to order message sharding, the classes in this pr can be reused. I will submit a new pr for that sharding message queue selector after this pr is merged : https://issues.apache.org/jira/browse/ROCKETMQ-136?jql=project%20%3D%20ROCKETMQ

@shroman
Copy link
Contributor

shroman commented Mar 10, 2017

@Jaskey Can you explain more about this feature? Having a better description on JIRA would help a lot review this PR -- especially for new functionalities. Since the description is very general, I have the following questions.

What is the advantage of introducing this strategy?
Consistent hashing could be useful if you have to move data between nodes on broker addition/removal. But since AllocateMessageQueueStrategy is used by consumers just to read from queues (if I get it right), on broker topology change a simple rehashing works well.
Of course, consistent hashing will glue consumers to the queues they are already consuming from as much as possible, even if topology is changed. Is this your concern?

@Jaskey
Copy link
Contributor Author

Jaskey commented Mar 10, 2017

@shroman

This is feature to give choice to users who care more about latency stabilization and messages duplication.

As you know, the default AllocateMessageQueueStrategy is averaging strategy which allocate queue to consumer as evenly as possible. But as you state, whenever queues numbers or consumer numbers changed, say a new consumer starts or an old consumer shutdowns, a rehashing will be triggered then almost all consumer suffered from this that they will rebalance to drop old queues and get new queues.

And that will cause

  1. message latency from producer to consumer increases at the moment when consumer/queue numbers change, even when they scale up.

  2. messages will be duplicated significantly since the offset may not be persisted to broker and that queue is assigned to another consumer to pull messages from.

This is especially significant when they have tens of consumer instances and scale-up or deployment is often.

Consistent Hash strategy to allocate queue is a good choice for these users.

@shroman
Copy link
Contributor

shroman commented Mar 10, 2017

@Jaskey This is a very good description, and now reviewers will understand well your intentions :) If you clearly state your intentions in JIRA, or discuss in the ml, it saves much time, and good for the record.

Sorry, it's not a code review yet, just wanted understand your intentions.

@Jaskey
Copy link
Contributor Author

Jaskey commented Mar 10, 2017

@shroman
The detail descriptions has been updated to JIRA. I will try described as detail as possible when I create an issue in JIRA in the future.

@Jaskey
Copy link
Contributor Author

Jaskey commented Apr 6, 2017

@shroman @zhouxinyu @lizhanhui @vongosling

I heard that consistent hash strategy will be accomplished in 4.1.x , while this pr has been open for more than a month, can we accelerate it, would you please help review the implementations?

for (int i = 0; i < 4; i++) {
h <<= 8;
h |= ((int) digest[i]) & 0xFF;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

digest is 128 bits, but the generated h only use 32 bits, other 96 bits are ignored, will it be better use ^ method

dongeforever pushed a commit to dongeforever/apache-rocketmq that referenced this pull request May 27, 2017
@asfgit asfgit closed this in 787d128 Jun 6, 2017
lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019
lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019
lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019
lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019
lizhanhui pushed a commit to lizhanhui/rocketmq that referenced this pull request Jun 25, 2019
JiaMingLiu93 pushed a commit to JiaMingLiu93/rocketmq that referenced this pull request May 28, 2020
JiaMingLiu93 pushed a commit to JiaMingLiu93/rocketmq that referenced this pull request May 28, 2020
pingww pushed a commit that referenced this pull request Aug 26, 2022
* codeCov of protocol.mqtt and protocol.rpc for #22

* remove redundant 'TestConnectHandler'

Co-authored-by: AhaThinking <ahathinking@AhaThinkingdeMacBook-Pro.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants