Skip to content

Use fnv32a hash for user_subring#2887

Merged
pracucci merged 2 commits intocortexproject:masterfrom
harry671003:master
Jul 16, 2020
Merged

Use fnv32a hash for user_subring#2887
pracucci merged 2 commits intocortexproject:masterfrom
harry671003:master

Conversation

@harry671003
Copy link
Contributor

What this PR does:

  • At the moment the hash function FNV-1 which is used for computing the ingester subring doesn't provide enough avalanche effect. The result of this is that, if similar user names are used in Cortex, they'll end up being assigned to a very few ingesters.
  • The hash function FNV-1a has better avalanche characteristics than FNV-1. This PR changes subring hash to FNV-1a.

Which issue(s) this PR fixes:
Fixes #2757

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: 🌲 Harry 🌊 John 🏔 <johrry@amazon.com>
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Thank you.

(As a side-note, I would be happy to see removal of this fnv.go copy entirely, and just using stdlib hashing functions.)

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Impeccable PR. Well done and thanks! 👏 ❤️

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci merged commit d099a1f into cortexproject:master Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Block store: S3 uploads doesn’t scale linearly with number of tenants

3 participants