BROOKLYN-325 Rebind entity starting #356
Conversation
…ting Add tests to SoftwareProcessRebindNotRunningEntityTest
| //FIXME does not set on fire root application | ||
| if (expectedState.getState() == Lifecycle.STARTING || expectedState.getState() == Lifecycle.STOPPING) { | ||
| setAttribute(SERVICE_STATE_EXPECTED, new Lifecycle.Transition(Lifecycle.ON_FIRE, new Date())); | ||
| setAttribute(SERVICE_STATE_ACTUAL, Lifecycle.ON_FIRE); |
There was a problem hiding this comment.
Use ServiceStateLogic.setExpectedState instead of setting the two attributes yourself.
There was a problem hiding this comment.
This is fixed in a separate commit, so the comment will not show as outdated.
| LOG.warn("On rebind of {}, not calling software process rebind hooks because expected state is {}", this, expectedState); | ||
| return; | ||
| } | ||
|
|
||
| Lifecycle actualState = getAttribute(SERVICE_STATE_ACTUAL); | ||
| if (actualState == null || actualState != Lifecycle.RUNNING) { | ||
| if (expectedState.getState() == Lifecycle.STARTING || expectedState.getState() == Lifecycle.STOPPING) { | ||
| setAttribute(SERVICE_STATE_ACTUAL, Lifecycle.ON_FIRE); | ||
| } |
There was a problem hiding this comment.
Not sure about this one, wonder what conditions would cause it (excluding setting it on fire from above). The message below won't be correct any more if you change the state, can you update it (or log a separate msg for this case).
There was a problem hiding this comment.
This is fixed in a separate commit, so the comment will not show as outdated.
…ting/stopping, not only child entities
| return expected.getState(); | ||
| } | ||
|
|
||
| public static void setActualState(Entity entity, Lifecycle state) { |
There was a problem hiding this comment.
Let's delete setActualState: it's not used. The pattern used is to set the expected state, and then for the actual state to be inferred by an enricher.
| oldEntities.remove(entity.getId()); | ||
| } | ||
|
|
||
| if (ServiceStateLogic.getExpectedState(entity) == Lifecycle.STARTING || ServiceStateLogic.getExpectedState(entity) == Lifecycle.STOPPING) { |
There was a problem hiding this comment.
I think this call needs to be guarded so it's not called by a HOT_STANDBY Brooklyn server (i.e. a read-only view) etc. We only want to set this if this entity is in BrooklynObjectManagementMode.MANAGED_PRIMARY.
(Note that is not the oldMode; it is the new mode that we care about).
I therefore wonder if there is a better place for this code. I'll dig around and see.
There was a problem hiding this comment.
See ivanayov#5 for an alternative (and for additional tests, including for hotStandby).
…ting-2 Set on-fire in BasicEntityRebindSupport
ivanayov
changed the title
ivanayov
force-pushed
the
rebind-entity-starting
branch
from
8d5089e
to
da6aedd
Compare
| protected void instanceRebind(AbstractBrooklynObject instance) { | ||
| Preconditions.checkState(instance == entity, "Expected %s and %s to match, but different objects", instance, entity); | ||
| Lifecycle expectedState = ServiceStateLogic.getExpectedState(entity); | ||
| if (expectedState == Lifecycle.STARTING || expectedState == Lifecycle.STOPPING) { | ||
| LOG.warn("Entity "+entity); | ||
| LOG.warn("Entity {} goes on-fire because it was in state {} on rebind", | ||
| entity, ServiceStateLogic.getExpectedState(entity)); |
There was a problem hiding this comment.
Just use expectedState rather than calling ServiceStateLogic.getExecptedState(entity) again.
There was a problem hiding this comment.
@aledsage For some reason this is not showing outdated.
| ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE); | ||
|
|
||
| if (!(entity instanceof BasicApplication) && entity.getLocations().isEmpty()) { | ||
| LOG.warn("Rebind happened during provisioning for entity {} with parent {}" + |
There was a problem hiding this comment.
Unfortunately this is too general. There are some entity types that will not necessarily have locations (e.g. that are not software processes, but instead bind to an API etc. For example, the GistGenerator docs example in http://docs.cloudsoft.io/blueprints/java/defining-and-deploying.html.
Let's take this out, and come back to it in a separate PR.
|
@iyovcheva I've added a couple more minor comments. Can you please address those, and then I think this is good to merge! |
|
@neykov any comments you have are much appreciated as well. |
ivanayov
force-pushed
the
rebind-entity-starting
branch
from
da6aedd
to
9b88108
Compare
|
Thanks @iyovcheva - LGTM; merging. We can look at the other TODOs in SoftwareProcessRebindNotRunningEntityTest.java (which were removed in the final commit?!). |
This fixes BROOKLYN-325 - when server restarts while entity is starting or stopping, on rebind it will now go on fire instead of reporting the entity's sensors as starting/stopping.