Refactoring

[vmorarian]

• simplified logic
• removed Future wrap (always can do that in action)
• more functional style
• code style corrections

[meniku]

Thank you!
I merged your changes. Unhappily there were many overlaps with the changes from #22 so it was a huge amount of work :( Maybe you want to review the changes I made in topic/pull-26 / a27ef82 ?

[vmorarian]

As I understand, caching is needed for large clusters.
Your changes looks good. I only have few minor comments/suggestions:

• cache set is heavy operation, and on cache miss it will block caller
• am I right, that caching is also need to be added for Table.all ?

Another option is to create actor which will update once per N mins list of all regions. I made branch with code template https://github.com/vmorarian/hannibal/tree/refactoring-like-cache
And I think that great improvement would be in cleaning json which is sent to client. Maybe I'm mistaken, but in some cases /api generates redundant json fields (they are not consumed on UI side).

[meniku]

Thanks for the input.

• We could remove the commit d8269bd thus having the Regions cached in a property of the Region object instead of EHCache. However I realized that it would be more convenient to use Play's EH-cache for caching. A cache miss should happen only when opening the web-interface right after Hannibal launch. The cache will be updated by the UpdateMetricsActor when region metrics are collected (every 60 seconds by default). Maybe we should add a custom ehcache.xml as the default invalidates the items after 120 seconds, this will be problematic if someone increases the default 60 seconds for the UpdateMetricsActor to over 120 seconds.
• I don't know if it is really necessary to cache tables. Most setups shouldn't have much tables, or am I wrong here?
• Your branch looks interesting, however I don't really like having a seperate actor which I have to ask to lookup regions instead of asking the model object itself. I would rather put the hBaseActor.ask call inside the implementation of Region.all, so the caller doesn't have to know about how the result is obtained.
• You're right about the Json optimization: there is very much potencial. Also, it's also questionable if Json is the right format for some things like for example the metrics. There is an incredible amount of timestamp -> value pairs transfered as Json objects like {ts: 23423424234, v: 323.2134}, which all have to be parsed. A binary format could remove much of the overhead here. And last but not least, way to much logic is done in the view layer, as noted in Does not work for large clusters #22.

However I think performance discussion does not belong here. I would suggest we agree upon the cache (EHCache vs hBaseActor), complete the pull-request and continue performance related discussion in #22.

[meniku]

merged it into 'next'

[vmorarian]

Your comments are reasonable. Lets go in this way.

I'm going to work in other PR on upgrade to Playframework v2.2.