Skip to content
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

SAMOA-71: fixes concurrency issues in HorizontalAMRulesRegressor #68

Closed
wants to merge 2 commits into from

Conversation

mgrzenda
Copy link
Contributor

I suggest a change in ruleSet implementation from LinkedList (providing fail-fast iterators, which causes ConcurrencyException when list content changes during iterating over it) to CopyOnWriteArrayList (which is a thread-safe variant eliminating concurrency exceptions). In the analysed case, the modifications to ruleSet are expected to be much less frequent than reading the rules. When the number of read operations is relatively large and update operations are far less frequent, a possible choice is CopyOnWriteArrayList.

I have compared the performance on 35k instance streams (with a higher than 35k number concurrency exception got thrown) and got the same <1 second processing time. Hence, the possible negative impact on the performance can be considered negligible, if any. Still, suggestions and possible other solution ideas from designers of AMRules are welcome.

@mgrzenda
Copy link
Contributor Author

Recent automated builds ten to be slow and exceed timeouts. This commit increases the values limiting the time allocated to Storm tests. This is to prevent the failure of these tests and entire Travis build in turn.

@gdfm
Copy link
Contributor

gdfm commented Sep 28, 2017

+1, looks good to me.
Thanks for the fix!

@abifet
Copy link
Contributor

abifet commented Oct 4, 2017

+1
Thanks! That's a nice improvement.

@asfgit asfgit closed this in 6e6bb18 Oct 5, 2017
@kination
Copy link
Contributor

kination commented Oct 6, 2017

Now test goes well. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants