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

cmd/tempo-vulture: share rand.Rand instance across multiple searches #1763

Merged
merged 3 commits into from Oct 5, 2022

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented Sep 27, 2022

#1760 noted that creating a new rand.Rand every time a random number is generated causes a poor distrubution of the search space, where numbers appear 7x more frequently than they do when using a shared rand.Rand instance.

This is demonstrated using the following Go playground link: https://go.dev/play/p/EkhINRfOO5p

This commit changes Vulture to use a shared rand.Rand instance instead of creating a new one each time a search or read is performed.

Closes #1760.

grafana#1760 noted that creating a new rand.Rand every time a
random number is generated causes a poor distrubution of the search
space, where numbers appear 7x more frequently than they do when using a
shared rand.Rand instance.

This is demonstrated using the following Go playground link: https://go.dev/play/p/EkhINRfOO5p

This commit changes Vulture to use a shared rand.Rand instance instead
of creating a new one each time a search or read is performed.
@CLAassistant
Copy link

CLAassistant commented Sep 27, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

thanks!

@joe-elliott joe-elliott merged commit 364fed5 into grafana:main Oct 5, 2022
@rfratto rfratto deleted the vulture-randomization-fix branch October 5, 2022 12:58
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.

Vulture does not evenly distribute search space
3 participants