brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Fix enrichers too much rebinded
Date Thu, 31 Jul 2014 08:27:58 GMT
GitHub user ahgittin opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/101

    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

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/incubator-brooklyn fix-enrichers-too-much-rebinded

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/101.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #101
    
----
commit e6b7af4c3489a6be616937d4a0d54db6e653e9a9
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T05:23:35Z

    add some more tests for enricher rebind (but none of these are the culprit for the multiple-enrichers-being-added)

commit e58b744f0274d0c2e9587c78e8eeb786507067cd
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T05:25:20Z

    fix vanilla-java integration tests, and clarify the API and fix paths used in ArchiveBuilder
which caused the problems (jars can't use "./" prefix, and these jars had "./test-classes/"
!);
    also clean up warnings and javadoc

commit 424bda6845aeb67a42e4da2038598a34929493ce
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T06:13:52Z

    increase JMX poll frequency slightly (to 3s rather than 500ms) as it's a bit expensive

commit d135df402c3b93f69772418babec004678cebd94
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T06:14:27Z

    add failing test for enrichers being added multiple times on rebind

commit cc64c47935fcbeab694fae6a659f5766e7da8ee2
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T06:54:48Z

    add a fix for enrichers/policies being added multiple times on rebind, by declining to
add an enricher which looks too much like one already present

commit 51145520aced1b20a482f78bde4b43cfa4697b8d
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T06:55:35Z

    explicitly fix two of the instances where enrichers and policies were being added again
on rebind

commit 6416d446196bdfb0e0723414e524fc0b673d8fa3
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date:   2014-07-31T07:38:34Z

    fix more of the explicit places where enrichers could be re-added on rebind

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message