[
https://issues.apache.org/jira/browse/STORM-1078?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14940375#comment-14940375
]
ASF GitHub Bot commented on STORM-1078:
---------------------------------------
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/776#discussion_r40963272
--- Diff: storm-core/test/jvm/backtype/storm/utils/RateTrackerTest.java ---
@@ -27,36 +27,63 @@
public class RateTrackerTest extends TestCase {
@Test
+ public void testExactRate() {
+ final long interval = 1000l;
+ long time = 0l;
+ RateTracker rt = new RateTracker(10000, 10, time);
+ double [] expected = new double[] {10.0, 10.0, 10.0, 10.0, 10.0,
10.0, 10.0, 10.0, 10.0, 10.0};
+ for (int i = 0; i < expected.length; i++) {
+ double exp = expected[i];
+ rt.notify(10);
+ time += interval;
+ double actual = rt.reportRate(time);
+ rt.forceRotate(1, interval);
+ assertEquals("Expected rate on iteration "+i+" is wrong.",
exp, actual, 0.00001);
+ }
+ expected = new double[] {11.0, 12.0, 13.0, 14.0, 15.0, 16.0, 17.0,
18.0, 19.0, 20.0};
--- End diff --
It would be better to comment that it is continuous test after previous
loop, so for each loop earliest bucket (which stores 10) is eclipsed and
replaced to 20.
I was misunderstood L43-51 to a new test, so I spent some times to find out
why it is not 20.0.
> RateTracker.java is not thread safe
> -----------------------------------
>
> Key: STORM-1078
> URL: https://issues.apache.org/jira/browse/STORM-1078
> Project: Apache Storm
> Issue Type: Bug
> Components: storm-core
> Affects Versions: 0.11.0
> Reporter: Robert Joseph Evans
> Assignee: Robert Joseph Evans
>
> The RateTracker class is not thread safe at all. It may not be that big of a
> deal, but the rates will be off if we notify from multiple threads, like we
> do with disruptor. It also has the potential to be way off if notify is
> being called at the same time as updateSlides. This would result in the new
> bucket not being set to 0, but getting the old value that was there
> previously.
> We want to be very careful that what we do does not impact the performance
> too much. So ideally no big locks but use AtomicLongs instead.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
