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

Improves reservoir to avoid a race condition and to be more fair #47

Merged
merged 2 commits into from
Dec 12, 2018
Merged

Improves reservoir to avoid a race condition and to be more fair #47

merged 2 commits into from
Dec 12, 2018

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Dec 10, 2018

This re-uses the implementation we just made in Brave based on feedback
on yours. Enjoy!

See openzipkin/brave#819

This re-uses the implementation we just made in Brave based on feedback
on yours. Enjoy!

See openzipkin/brave#819
@codefromthecrypt
Copy link
Contributor Author

sorry had a bug. should be ok now cc @abhiksingh

@codefromthecrypt
Copy link
Contributor Author

build fail looks like a flake

@abhiksingh
Copy link

@adriancole thanks for the fix and a more fair reservoir implementation. The team is reviewing the code and we will look to merge this in at the earliest.

@haotianw465
Copy link
Contributor

Hi @adriancole, I have created an issue for the flaky unit test. It is not related to this PR. The re-run build passed and I'm merging this in. Thank you for your contribution we really appreciate it.

@haotianw465 haotianw465 merged commit 7855a7a into aws:master Dec 12, 2018
@codefromthecrypt codefromthecrypt deleted the fancy-reservoir branch December 13, 2018 00:23
@codefromthecrypt
Copy link
Contributor Author

Hats off to those who made the code I copied here: @devinsba spiked the implementation and @anuraaga had the design for more fair sampling large numbers of requests. It was also reviewed by @huydx and @zeagord

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.

3 participants