Hi Alex,
Good suggestions. A couple of questions.
Do we definitely need both SERVICE_NOT_UP_INDICATORS and
SERVICE_PROBLEMS, or could be have just one?
The benefit I see of both is that SERVICE_PROBLEMS could be applicable
when starting/stopping, whereas SERVICE_NOT_UP_INDICATORS only matter
when we are supposed to be running.
If we can get away with just one, it will keep things simpler.
---
The issue addressed in PR #101 is that some entities are written such
that enrichers are getting duplicated when brooklyn restarts: the
"rebind" re-registers the enricher, but the entity also registers an
enricher again when its rebind() method calls its connectSensors() method.
I agree with your points Alex in #101, especially with us adding support
for persisting sensor feeds and subscriptions (though these often use
anonymous inner classes in their impl which makes them hard to persist!).
The medium-to-long term solution is to fix our entity implementations.
---
> Finally for tracking enrichers, sensor feeds, subscriptions, and
policies, I suggest
> we add an optional "uniqueName", the presence of which blocks the
addition of
> something of the same kind with the same uniqueName. This will
better solve the
> problem described in #101, and it gives us a way to allow code to
find and/or
> remove some of the enrichers above if they need to customize logic.
Do you mean that addEnricher() would be overloaded to take an optional
name (or perhaps part of EnricherSpec) - so addEnricher would be a no-op
if there was already an enricher with that name? And similarly for
addSubscription etc?
What would addEnricher return instead? By "same kind", do you mean
another enricher rather than a subscription etc, rather than meaning
another enricher of the same Java type?
For #101, I lean towards logging a warning instead. When the entity
calls `connectSensors()` during rebind, we know it's still rebinding so
could do an extra check if there is an enricher that looks the same. If
there is, then warn.
Fundamentally, addEnricher should add the enricher. And we should update
all our entity implementations to the new pattern of doing more in the
init of the entity. If that pattern isn't right, we should figure out
what is.
Aled
On 31/07/2014 16:18, Alex Heneveld wrote:
Hi Svet, All,
Good time to raise this. I've been wondering about a similar thing,
and mentioned this at #101. i'd like to see a way that new
requirements for service_up and service_state can easily be added by
third parties.
Currently we explicitly attach a "computeServiceUp" computation at a
few entities (e.g. to say service is up iff service_state = running
AND REST to /foo returns 200). But it this is ad hoc, and it does not
easily third party updates (in particular clearing problems cleanly,
ie having independent problem detectors and clearers). A related
ambiguity is in SERVICE_STATE which is a combination of expected state
together with being on_fire for some problems.
I'd like to suggest:
1) We add a SERVICE_NOT_UP_INDICATORS *map* sensor
2) We attach an enricher which sets SERVICE_UP based on
SERVICE_NOT_UP_INDICATORS.isEmpty()
Then up-ness is controlled by effectors and policies which add and
remove SERVICE_NOT_UP_INDICATORS, keyed by an identifier unique to
them. For instance all clusters and fabrics would simply subscribe to
children's UP events and add such an indicator object the under
"cluster.size" key if there are not a sufficient number of UP
children. (Incidentally this would solve an issue where cluster
health is not always cleared appropriately when nodes come back
online.) The existing isRunning checks for SoftwareProcess entities
would also add such an indicator if it is detected as not running.
And we do something similar for SERVICE_STATE:
Introduce a SERVICE_PROBLEMS *map* attribute and an enricher which
sets SERVICE_STATE based on the problems being empty and the value of
new sensor SERVICE_STATE_EXPECTED. SERVICE_STATE_EXPECTED is set by
the lifecycle tasks, and then: if a service is expected starting or
stopping that is shown as SERVICE_STATE, otherwise if
!SERVICE_PROBLEMS.isEmpty() it is set as ON_FIRE, otherwise it is set
based on SERVICE_STATE_EXPECTED and SERVICE_UP. Also we could have an
enricher which puts a SERVICE_PROBLEM if
`(SERVICE_STATE_EXPECTED==RUNNING && SERVICE_UP==false)`.
This is a touch more complicated than SERVICE_UP but I think it would
be clearer and could simplify some of the "isRunning" logic checks
during post-start. Where we want to wait on multiple things to
determine up-ness, we can insert a SERVICE_NOT_UP_INDICATOR manually,
then wait for the appropriate enricher/feed to clear it. And it could
handle the case where a subscription should be responsible for the
final transition to EXPECTED=RUNNING (there are a few cases where
start will set RUNNING early, and a subscription comes along later and
finishes the job, after sensors have been emitted). And of course it
would support Svet's use case where the "abc-compliance" policy would
simply add an entry { abc-compliance: "Replication violation" } to the
SERVICE_PROBLEMS, and clear it if it becomes okay -- and service state
is automatically updated to be ON_FIRE when there is a compliance
problem.
Finally for tracking enrichers, sensor feeds, subscriptions, and
policies, I suggest we add an optional "uniqueName", the presence of
which blocks the addition of something of the same kind with the same
uniqueName. This will better solve the problem described in #101, and
it gives us a way to allow code to find and/or remove some of the
enrichers above if they need to customize logic.
Best
Alex
On 31/07/2014 05:34, Svetoslav Neykov wrote:
Hi,
It seems that there is no way to unset an ON_FIRE state previously
set by my
code. First it is not clear what the new state should be and second some
other code could've set the state as well meanwhile.
Here is some background. I am developing sample policies which monitor
machines for compliance with certain rules. If the rule is broken the
machine should be set ON_FIRE. So far so good. The problem is that
once the
machines are back in compliant state I need to clear the error.
The ON_FIRE state in Lifecycle seems orthogonal to the rest of the
states.
Logically we can have ON_FIRE while RUNNING or STARTING. It could be a
temporary error, not a final state in the state machine.
Just as an observation, we could have an entity ON_FIRE and
SERVICE_UP at
the same time.
Possible solutions to the ON_FIRE issue could be:
* Forbidding manual setting of ON_FIRE state, instead creating a
mechanism to register functions returning the state. By default it
would be
SERVICE_STATE == RUNNING. The cons is that it is a poll-based approach.
* Reference counting the setting of ON_FIRE. The cons is that
it is
requires tedious housekeeping, leading to bugs.
Perhaps a combination of both approaches would be best - use the
first one
with a long poll, with the ability to trigger the check manually.
Any thoughts?
Best,
Svet.
