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

Vulture does not evenly distribute search space #1760

Closed
rfratto opened this issue Sep 26, 2022 · 0 comments · Fixed by #1763
Closed

Vulture does not evenly distribute search space #1760

rfratto opened this issue Sep 26, 2022 · 0 comments · Fixed by #1763
Labels
good first issue Good for newcomers

Comments

@rfratto
Copy link
Member

rfratto commented Sep 26, 2022

Vulture currently creates a new random number generator every time it performs a search. The random number generator is then used to generate a random number between two values (a min timestamp and a max timestamp).

However, until Vulture has been alive for two weeks (specified by the --tempo-retention-duration field), the seed used for the random number generator will only change once. The seed will initially be set to the process start time, and then will change to the process start time rounded to the nearest --tempo-read-backoff-duration. After this point, it will stay on that value until the retention duration period has elapsed.

Using the same seed leads to search distribution issues. This Go playground link demonstrates that always creating a new RNG with the same seed will repeat numbers seven times more often over sharing a single RNG for all searches.

@joe-elliott joe-elliott added the good first issue Good for newcomers label Sep 26, 2022
rfratto added a commit to rfratto/tempo that referenced this issue Sep 27, 2022
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.
joe-elliott pushed a commit that referenced this issue Oct 5, 2022
…1763)

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

#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.

* update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants