DB-7022 avoid coordinated pauses from multiple threads #1857
Conversation
| long jitter = (long)(normalPause * RANDOM.nextFloat() * 0.01f); // 1% possible jitter | ||
| return normalPause + jitter; | ||
| // DB-7022 avoid coordinated pauses from multiple threads | ||
| return RANDOM.nextLong() % normalPause; |
There was a problem hiding this comment.
The Random.nextLong() may return a negative value, so need to use the absolute value or something. See the following for a potential problem with getting the absolute value of the smallest possible long:
https://help.semmle.com/wiki/display/JAVA/Incorrect+absolute+value+of+random+number
There was a problem hiding this comment.
Good point. Will provide a better fix.
|
I'm going to test the latest version in cluster and backport it to 2.5/2.7. |
|
Hi guys! I see you've caught a potential coding problem by hand and referred to our documentation for more info. That's awesome! You may be interested in LGTM.com, where our analysis of such problems is freely available for open source projects like Splice Machine. One of the most important features is automatic code review for GitHub Pull Requests. LGTM will automatically comment on PRs that introduce new alerts — ranging from potentially critical security issues (such as this one in AMPHTML), to resource leaks (such as this one in a NASA project), to unsafe/incorrect random numbers. The coding problem you manually spotted here will have been automatically detected if PR integration was enabled for your project. LGTM.com is already analysing >70K open source repos; here are the results for splicemachine/spliceengine: https://lgtm.com/projects/g/splicemachine/spliceengine/alerts/. You can enable PR integration here: https://lgtm.com/projects/g/splicemachine/spliceengine/ci/ My co-worker used your build instructions to set up a custom build command so that LGTM can successfully analyse splicemachine; I will open a PR here with those settings so you can tweak them in the future. (Full disclosure: I work at Semmle, on the team responsible for LGTM.com) |
|
@yxia92 I updated DB-7022 with my latest analysis. I find the proposed fix beneficial and would like to proceed with merging it. |
|
Thanks @OlegMazurov I've changed my vote to approval. |
No description provided.