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

Handle hash collision in KeyShared subscription mode #8396

Merged
merged 1 commit into from
Oct 31, 2020

Conversation

ltamber
Copy link
Contributor

@ltamber ltamber commented Oct 28, 2020

Motivation

Currently, in ConsistentHashingStickyKeyConsumerSelector key consumer selector,if multi key have the same hash code, the consumer in the hashRing will be replaced by the newer consumer, so the behavior of the message dispatch will not consistent if the consumer subscribed in a different order.

Modifications

handle the hash collision with a list.

Verifying this change

unit test ConsistentHashingStickyKeyConsumerSelectorTest was passed.

@ltamber
Copy link
Contributor Author

ltamber commented Oct 28, 2020

@jiazhai @codelipenghui PTAL

@ltamber
Copy link
Contributor Author

ltamber commented Oct 28, 2020

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented Oct 28, 2020

/pulsarbot run-failure-checks

@jiazhai jiazhai added type/bug The PR fixed a bug or issue reported a bug area/broker labels Oct 28, 2020
@codelipenghui codelipenghui added this to the 2.7.0 milestone Oct 28, 2020
@codelipenghui
Copy link
Contributor

@ltamber Could you please add some unit test for the new change?

@ltamber
Copy link
Contributor Author

ltamber commented Oct 29, 2020

@codelipenghui It seem that hard to simulate(but it does exist) hash collision when use murmur_3, so I just run the ConsistentHashingStickyKeyConsumerSelectorTest unit test

@ltamber
Copy link
Contributor Author

ltamber commented Oct 29, 2020

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 014f99a into apache:master Oct 31, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Nov 13, 2020
…pache#8396)

### Motivation
Currently, in `ConsistentHashingStickyKeyConsumerSelector` key consumer selector,if multi key have the same hash code, the consumer in the `hashRing` will be replaced by the newer consumer,  so the behavior of the message dispatch will not consistent if the consumer subscribed in a different order.

### Modifications

handle the hash collision with a list.

### Verifying this change

unit test `ConsistentHashingStickyKeyConsumerSelectorTest` was passed.
flowchartsman pushed a commit to flowchartsman/pulsar that referenced this pull request Nov 17, 2020
…pache#8396)

### Motivation
Currently, in `ConsistentHashingStickyKeyConsumerSelector` key consumer selector,if multi key have the same hash code, the consumer in the `hashRing` will be replaced by the newer consumer,  so the behavior of the message dispatch will not consistent if the consumer subscribed in a different order.

### Modifications

handle the hash collision with a list.

### Verifying this change

unit test `ConsistentHashingStickyKeyConsumerSelectorTest` was passed.
}

return consumerList.get(hash % consumerList.size());
Copy link
Member

Choose a reason for hiding this comment

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

@ltamber I have a question about this line of code in #23315 (comment) . Are you available to answer that question? Thanks /cc @codelipenghui

Copy link
Member

Choose a reason for hiding this comment

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

I made a change already, #23327

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants