Github user arunmahadevan commented on the pull request:
https://github.com/apache/storm/pull/925#issuecomment-162242973
@HeartSaVioR thanks for providing the patch. I had overlooked the edge
cases where the TimeTrigger might not fire or fires twice causing tests to fail.
The patch you have provided addresses the issue. Only concern is that the
existing TimeEvictionPolicy uses System.currentTimeMillis() to evict the events
so it relies on the passage of time to expire events and if you manually invoke
the trigger, the expected events may not expire.
However I verified that the tests pass with your changes so it can be
merged to fix the random test failures.
In https://github.com/apache/storm/pull/900 an api is added to set the
reference timestamp for the eviction policy instead of it relying on system
timestamp. With that PR I will fix the unit tests by setting the future
timestamp to simulate passage of time and invoke the trigger manually.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
