apache/incubator-brooklyn — pull request #101 by ahgittin (2014-07-31 08:27:56+00:00)
Title: Fix enrichers too much rebinded

There is a nasty issue that enrichers (and policies) often get duplicated on a rebind. The reason for this is that many times, the places where enrichers are added is near where sensors are added, and because we don't persist sensor feeds we count on those being added on rebind (eg in `SoftwareProcessImpl.callRebindHooks()` invoking `connectSensors()`).

Normally you'd think it's fine if an enricher gets added again. But you'd be wrong, here's why it's nasty: say we have an enricher on a JBoss (e.g. to take a time average), and then another at a web-cluster parent (to average across nodes), and another at the controlled-dwac parent (to propagate). After rebind, we have two of each of these. And after another we have three of each. Now when the original sensor is published, we get the enriched value published 3 times from JBoss. And each of these 3 values is handled now by 3 enrichers at the web-cluster parent, so the enriched sensor at web-cluster is published 9 times. And you guessed it, the simple propagator has 3 instances each running 9 times so is publishing 27 times for initial event. Persist a few more times, and you're now publishing N^3, queueing lots and lots of tasks and bringing down the server.

There are a lot of things we can do to improve it.

This PR fixes several of the places where enrichers might be added multiple times (moving them to `init()` or similar as that's generally safe for enrichers).

This PR also does a somewhat messy check in `AbstractEntity.addEnricher(...)` and `addPolicy(...)` to see whether there is already an enricher/policy which seems similar, and refuses to add it and warns if encountered. This should help flag other places this becomes an issue (and will clean up servers which have too many enrichers already).

These two techniques seem pretty good for now. Ideally long-term however we can get rid of the second, having cleaned these things up so it isn't a problem. As a point-in-time brain dump, I'd like to see:

* feeds which can ignore duplicate values do so
* feeds which can drop intermediate values do so under duress (or we know which tasks to drop under heavy load)
* **we have a cleaner model for feeds** where we can tell if the populator of a sensor is registered, recognising sensor feeds and enrichers are often quite similar, and both are general cases of subscriptions (maybe that's what we need to be persisting?) along with policy
* although some of the software-process sensor feed definitions should support multiple implementations (e.g. configure the built-in brooklyn collector or an external collectd)
* allows sensor feeds to be loaded at init time also, and do this in general, with a `whenServiceUp` or `whenServiceExpectedStateIsRunning` guard (cleaning up `SERVICE_STATE` to be `EXPECTED_STATE` and `ACTUAL_STATE`)
* allowing a caller to set multiple inputs for a sensor, or even dynamically add inputs which should be used (esp to compute service up, someone could easily add a new requirement for service_up)
* persist sensor feed info and subscriptions

/cc @aledsage @neykov