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

fix hash function bug #1267

Merged
merged 13 commits into from
Jun 20, 2021
Merged

fix hash function bug #1267

merged 13 commits into from
Jun 20, 2021

Conversation

tangweihua163
Copy link
Contributor

What this PR does:

fix bug

Which issue(s) this PR fixes:

Fixes #

hash function shifts a byte by 24/16/8 bits, always get zero.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@AlexStocks
Copy link
Contributor

why do u change the first charactor of word to its low case?

@tangweihua163
Copy link
Contributor Author

why do u change the first charactor of word to its low case?

like all other three implements of LoadBalance interface, least_active, round_robin and random, consistent_hash should not export its detail out. this change is not necessary, but reasonable.

the focal point is the bug in hash method of type consistentHashSelector, left shifting a byte by more than 8 bits always got zero.

@AlexStocks AlexStocks changed the base branch from master to 3.0 June 20, 2021 02:25
@AlexStocks
Copy link
Contributor

I have changed the target branch from master to 3.0.

@tangweihua163 tangweihua163 reopened this Jun 20, 2021
@LaurenceLiZhixin LaurenceLiZhixin added this to the v3.0.0 milestone Jun 20, 2021
Copy link
Contributor

@zhaoyunxing92 zhaoyunxing92 left a comment

Choose a reason for hiding this comment

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

approved these changes

@AlexStocks AlexStocks merged commit dbc6132 into apache:3.0 Jun 20, 2021
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

6 participants