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

Remove and forbid use of j.u.c.ThreadLocalRandom #15862

Merged
merged 1 commit into from
Jan 8, 2016
Merged

Remove and forbid use of j.u.c.ThreadLocalRandom #15862

merged 1 commit into from
Jan 8, 2016

Conversation

jasontedor
Copy link
Member

This commit removes and now forbids all uses of
java.util.concurrent.ThreadLocalRandom across the codebase. The
underlying issue with ThreadLocalRandom is that it can not be
seeded. This means that if ThreadLocalRandom is used in production code,
then tests that cover any code path containing ThreadLocalRandom will be
prevented from being reproducible by use of ThreadLocalRandom. Instead,
using org.elasticsearch.common.random.Randomness#get will give
reproducible sources of random when running under tests and otherwise
still give an instance of ThreadLocalRandom when running as production
code.

Closes #15294

This commit removes and now forbids all uses of
java.util.concurrent.ThreadLocalRandom across the codebase. The
underlying issue with ThreadLocalRandom is that it can not be
seeded. This means that if ThreadLocalRandom is used in production code,
then tests that cover any code path containing ThreadLocalRandom will be
prevented from being reproducible by use of ThreadLocalRandom. Instead,
using org.elasticsearch.common.random.Randomness#get will give
reproducible sources of random when running under tests and otherwise
still give an instance of ThreadLocalRandom when running as production
code.
@jasontedor jasontedor added >enhancement >test Issues or PRs that are addressing/adding tests review v5.0.0-alpha1 labels Jan 8, 2016
@nik9000
Copy link
Member

nik9000 commented Jan 8, 2016

LGTM. Its convenient that this directly resolves back to ThreadLocalRandom outside of tests. It makes it simpler to review.

jasontedor added a commit that referenced this pull request Jan 8, 2016
Remove and forbid use of j.u.c.ThreadLocalRandom

Closes #15294
@jasontedor jasontedor merged commit 0b1a7fc into elastic:master Jan 8, 2016
@jasontedor jasontedor deleted the thread-local-random-be-gone branch January 8, 2016 18:00
@jasontedor
Copy link
Member Author

Thanks @nik9000.

@jasontedor jasontedor removed the review label Jan 8, 2016
@nik9000
Copy link
Member

nik9000 commented Jan 8, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement >test Issues or PRs that are addressing/adding tests v5.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants