Conversation
Signed-off-by: Won Jun Jang <wjang@uber.com>
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
============================================
+ Coverage 84.77% 84.96% +0.19%
- Complexity 608 618 +10
============================================
Files 94 94
Lines 2410 2428 +18
Branches 271 271
============================================
+ Hits 2043 2063 +20
+ Misses 276 275 -1
+ Partials 91 90 -1
Continue to review full report at Codecov.
|
|
|
||
| @Override | ||
| public boolean equals(Object other) { | ||
| public synchronized boolean equals(Object other) { |
There was a problem hiding this comment.
Why does this need to be synchronized?
There was a problem hiding this comment.
If this function and the update function are called at the same time, there could(?) be a race condition on the variable maxTracesPerSecond. Might be overkill
| } | ||
|
|
||
| private double getMaxBalance(double maxTracesPerSecond) { | ||
| return maxTracesPerSecond < 1.0 ? 1.0 : maxTracesPerSecond; |
There was a problem hiding this comment.
Why 1.0? Could you refactor the magic number into a constant?
I'm not a fan of hardcoding this number here because we'll have to update the client when we need to change this number.
|
|
||
| public RateLimiter(double creditsPerSecond, double maxBalance) { | ||
| this(creditsPerSecond, maxBalance, new SystemClock()); | ||
| this(creditsPerSecond, maxBalance, new SystemClock(), maxBalance * new Random().nextDouble()); |
There was a problem hiding this comment.
Could we cache the Random()?
2 participants
Signed-off-by: Won Jun Jang wjang@uber.com
Which problem is this PR solving?
cf. jaegertracing/jaeger-client-go#320