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
ZOOKEEPER-1990 - fix Random instances #617
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Make sense to me.
See the jira for more info. Basically we have multiple ways of creating Random instances in ZooKeeper. Since java 1.7, the default constructor is good enough even in multi-threaded environment, we get a good seed. But in some places, we just create a random instance, where System.nanotime is the seed, which is not a good practice in multi-threaded environments. I only replaced those, and I also left the tests as is, because in some cases it is intentional in them. I created the PR to bring more attention to the ticket, please feel free to share your ideas on the topic! Author: Norbert Kalmar <nkalmar@yahoo.com> Reviewers: fangmin@apache.org, andor@apache.org Closes #617 from nkalmar/ZOOKEEPER-1990 (cherry picked from commit 181de25) Signed-off-by: Andor Molnar <andor@apache.org>
Committed to branch-3.5 and master. |
See the jira for more info. Basically we have multiple ways of creating Random instances in ZooKeeper. Since java 1.7, the default constructor is good enough even in multi-threaded environment, we get a good seed. But in some places, we just create a random instance, where System.nanotime is the seed, which is not a good practice in multi-threaded environments. I only replaced those, and I also left the tests as is, because in some cases it is intentional in them. I created the PR to bring more attention to the ticket, please feel free to share your ideas on the topic! Author: Norbert Kalmar <nkalmar@yahoo.com> Reviewers: fangmin@apache.org, andor@apache.org Closes apache#617 from nkalmar/ZOOKEEPER-1990
See the jira for more info.
Basically we have multiple ways of creating Random instances in ZooKeeper. Since java 1.7, the default constructor is good enough even in multi-threaded environment, we get a good seed.
But in some places, we just create a random instance, where System.nanotime is the seed, which is not a good practice in multi-threaded environments.
I only replaced those, and I also left the tests as is, because in some cases it is intentional in them.
I created the PR to bring more attention to the ticket, please feel free to share your ideas on the topic!