[Bug] ConsistentHashingStickyKeyConsumerSelector's getConsumerKeyHashRanges doesn't return correct results - Githubissues

apache / pulsar

Apache Pulsar - distributed pub-sub messaging system

https://pulsar.apache.org/

Apache License 2.0

14.26k stars 3.59k forks source link

[Bug] ConsistentHashingStickyKeyConsumerSelector's getConsumerKeyHashRanges doesn't return correct results #23321

Closed lhotari closed 1 month ago

lhotari commented 2 months ago

Search before asking

[X] I searched in the issues and found nothing similar.

Read release policy

[X] I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker.

Version

all versions

Minimal reproduce step

After the last entry in the hash ring, it wraps around to the first entry: https://github.com/apache/pulsar/blob/7e73967f9daf368562fa0318d4ecb62272d72174/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConsistentHashingStickyKeyConsumerSelector.java#L123-L125

This is ignored in the getConsumerKeyHashRanges method: https://github.com/apache/pulsar/blob/7e73967f9daf368562fa0318d4ecb62272d72174/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConsistentHashingStickyKeyConsumerSelector.java#L138-L145

Another problem is that the rule to pick one consumer when there are hash collisions is also ignored: https://github.com/apache/pulsar/blob/7e73967f9daf368562fa0318d4ecb62272d72174/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConsistentHashingStickyKeyConsumerSelector.java#L127

What did you expect to see?

the results returned by the getConsumerKeyHashRanges should be accurate and correct

What did you see instead?

the getConsumerKeyHashRanges logic

ignores the "hash ring" wrap around after the last entry
ignores the logic for choosing a single entry when there's a hash collision
- this is a conflicting goal currently since for hash collisions, the target consumer is selected based on the input hash
- for hash ranges with multiple consumers, it would be necessary to scan the complete range to find out the actual hash ranges for a consumer

Anything else?

No response

Are you willing to submit a PR?

[ ] I'm willing to submit a PR!

lhotari commented 1 month ago

This will be included in #23327 since the implementation was needed for tests in that PR.

lhotari commented 1 month ago

Issue #16654 is related.