You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/07/03 11:42:13 UTC
[3/6] git commit: BROOKLYN-14: rebind policies in separate phase
BROOKLYN-14: rebind policies in separate phase
- Only add policies+enrichers to entities after all entities have had
their state set, and all relationships (parent-child,
group membership, locations) have been set up
- Also corrects spelling of “doReconsruct” in BasicPolicyRebindSupport
etc
- Improves comments in RebindManagerImpl on rebind’s phases
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/59ce9e8b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/59ce9e8b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/59ce9e8b
Branch: refs/heads/master
Commit: 59ce9e8b04b4d226436edab96c9e8b4ec2539154
Parents: b8f1a04
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jul 1 10:28:33 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Jul 1 10:28:33 2014 +0100
----------------------------------------------------------------------
.../brooklyn/entity/rebind/RebindSupport.java | 8 ++-
.../rebind/BasicEnricherRebindSupport.java | 13 ++++-
.../entity/rebind/BasicEntityRebindSupport.java | 52 +++++++++---------
.../rebind/BasicLocationRebindSupport.java | 14 ++++-
.../entity/rebind/BasicPolicyRebindSupport.java | 14 ++++-
.../entity/rebind/RebindManagerImpl.java | 49 +++++++++++++++--
.../entity/rebind/RebindLocationTest.java | 4 +-
.../entity/rebind/RebindPolicyTest.java | 55 ++++++++++++++++++++
8 files changed, 168 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/api/src/main/java/brooklyn/entity/rebind/RebindSupport.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/entity/rebind/RebindSupport.java b/api/src/main/java/brooklyn/entity/rebind/RebindSupport.java
index 9d3aaf4..6aa0cca 100644
--- a/api/src/main/java/brooklyn/entity/rebind/RebindSupport.java
+++ b/api/src/main/java/brooklyn/entity/rebind/RebindSupport.java
@@ -18,7 +18,7 @@ public interface RebindSupport<T extends Memento> {
/**
* Creates a memento representing this entity's current state. This is useful for when restarting brooklyn.
*/
- public T getMemento();
+ T getMemento();
/**
* Reconstructs this entity, given a memento of its state. Sets the internal state
@@ -29,5 +29,9 @@ public interface RebindSupport<T extends Memento> {
*
* Called before rebind.
*/
- public void reconstruct(RebindContext rebindContext, T memento);
+ void reconstruct(RebindContext rebindContext, T memento);
+
+ void addPolicies(RebindContext rebindContext, T Memento);
+
+ void addEnrichers(RebindContext rebindContext, T Memento);
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/main/java/brooklyn/entity/rebind/BasicEnricherRebindSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/BasicEnricherRebindSupport.java b/core/src/main/java/brooklyn/entity/rebind/BasicEnricherRebindSupport.java
index 4a5747c..37aa249 100644
--- a/core/src/main/java/brooklyn/entity/rebind/BasicEnricherRebindSupport.java
+++ b/core/src/main/java/brooklyn/entity/rebind/BasicEnricherRebindSupport.java
@@ -40,15 +40,24 @@ public class BasicEnricherRebindSupport implements RebindSupport<EnricherMemento
FlagUtils.setFieldsFromFlags(enricher, configBag);
FlagUtils.setAllConfigKeys(enricher, configBag, false);
- doReconsruct(rebindContext, memento);
+ doReconstruct(rebindContext, memento);
((AbstractEnricher)enricher).rebind();
}
+ @Override
+ public void addPolicies(RebindContext rebindContext, EnricherMemento Memento) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void addEnrichers(RebindContext rebindContext, EnricherMemento Memento) {
+ throw new UnsupportedOperationException();
+ }
/**
* For overriding, to give custom reconsruct behaviour.
*/
- protected void doReconsruct(RebindContext rebindContext, EnricherMemento memento) {
+ protected void doReconstruct(RebindContext rebindContext, EnricherMemento memento) {
// default is no-op
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java b/core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java
index 9ab4a8b..50fc396 100644
--- a/core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java
+++ b/core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java
@@ -88,8 +88,6 @@ public class BasicEntityRebindSupport implements RebindSupport<EntityMemento> {
setParent(rebindContext, memento);
addChildren(rebindContext, memento);
- addPolicies(rebindContext, memento);
- addEnrichers(rebindContext, memento);
addMembers(rebindContext, memento);
addTags(rebindContext, memento);
addLocations(rebindContext, memento);
@@ -98,6 +96,32 @@ public class BasicEntityRebindSupport implements RebindSupport<EntityMemento> {
((AbstractEntity)entity).rebind();
}
+ @Override
+ public void addPolicies(RebindContext rebindContext, EntityMemento memento) {
+ for (String policyId : memento.getPolicies()) {
+ AbstractPolicy policy = (AbstractPolicy) rebindContext.getPolicy(policyId);
+ if (policy != null) {
+ entity.addPolicy(policy);
+ } else {
+ LOG.warn("Policy not found; discarding policy {} of entity {}({})",
+ new Object[] {policyId, memento.getType(), memento.getId()});
+ }
+ }
+ }
+
+ @Override
+ public void addEnrichers(RebindContext rebindContext, EntityMemento memento) {
+ for (String enricherId : memento.getEnrichers()) {
+ AbstractEnricher enricher = (AbstractEnricher) rebindContext.getEnricher(enricherId);
+ if (enricher != null) {
+ entity.addEnricher(enricher);
+ } else {
+ LOG.warn("Enricher not found; discarding enricher {} of entity {}({})",
+ new Object[] {enricherId, memento.getType(), memento.getId()});
+ }
+ }
+ }
+
/**
* For overriding, to reconstruct other fields.
*/
@@ -162,28 +186,4 @@ public class BasicEntityRebindSupport implements RebindSupport<EntityMemento> {
}
}
}
-
- protected void addPolicies(RebindContext rebindContext, EntityMemento memento) {
- for (String policyId : memento.getPolicies()) {
- AbstractPolicy policy = (AbstractPolicy) rebindContext.getPolicy(policyId);
- if (policy != null) {
- entity.addPolicy(policy);
- } else {
- LOG.warn("Policy not found; discarding policy {} of entity {}({})",
- new Object[] {policyId, memento.getType(), memento.getId()});
- }
- }
- }
-
- protected void addEnrichers(RebindContext rebindContext, EntityMemento memento) {
- for (String enricherId : memento.getEnrichers()) {
- AbstractEnricher enricher = (AbstractEnricher) rebindContext.getEnricher(enricherId);
- if (enricher != null) {
- entity.addEnricher(enricher);
- } else {
- LOG.warn("Enricher not found; discarding enricher {} of entity {}({})",
- new Object[] {enricherId, memento.getType(), memento.getId()});
- }
- }
- }
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/main/java/brooklyn/entity/rebind/BasicLocationRebindSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/BasicLocationRebindSupport.java b/core/src/main/java/brooklyn/entity/rebind/BasicLocationRebindSupport.java
index 5e5c6e6..07d840f 100644
--- a/core/src/main/java/brooklyn/entity/rebind/BasicLocationRebindSupport.java
+++ b/core/src/main/java/brooklyn/entity/rebind/BasicLocationRebindSupport.java
@@ -89,7 +89,17 @@ public class BasicLocationRebindSupport implements RebindSupport<LocationMemento
location.init(); // TODO deprecated calling init; will be deleted
location.rebind();
- doReconsruct(rebindContext, memento);
+ doReconstruct(rebindContext, memento);
+ }
+
+ @Override
+ public void addPolicies(RebindContext rebindContext, LocationMemento Memento) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void addEnrichers(RebindContext rebindContext, LocationMemento Memento) {
+ throw new UnsupportedOperationException();
}
protected void addChildren(RebindContext rebindContext, LocationMemento memento) {
@@ -115,7 +125,7 @@ public class BasicLocationRebindSupport implements RebindSupport<LocationMemento
/**
* For overriding, to give custom reconsruct behaviour.
*/
- protected void doReconsruct(RebindContext rebindContext, LocationMemento memento) {
+ protected void doReconstruct(RebindContext rebindContext, LocationMemento memento) {
// default is no-op
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/main/java/brooklyn/entity/rebind/BasicPolicyRebindSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/BasicPolicyRebindSupport.java b/core/src/main/java/brooklyn/entity/rebind/BasicPolicyRebindSupport.java
index 39202cb..c952f83 100644
--- a/core/src/main/java/brooklyn/entity/rebind/BasicPolicyRebindSupport.java
+++ b/core/src/main/java/brooklyn/entity/rebind/BasicPolicyRebindSupport.java
@@ -40,14 +40,24 @@ public class BasicPolicyRebindSupport implements RebindSupport<PolicyMemento> {
FlagUtils.setFieldsFromFlags(policy, configBag);
FlagUtils.setAllConfigKeys(policy, configBag, false);
- doReconsruct(rebindContext, memento);
+ doReconstruct(rebindContext, memento);
((AbstractPolicy)policy).rebind();
}
+ @Override
+ public void addPolicies(RebindContext rebindContext, PolicyMemento Memento) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void addEnrichers(RebindContext rebindContext, PolicyMemento Memento) {
+ throw new UnsupportedOperationException();
+ }
+
/**
* For overriding, to give custom reconsruct behaviour.
*/
- protected void doReconsruct(RebindContext rebindContext, PolicyMemento memento) {
+ protected void doReconstruct(RebindContext rebindContext, PolicyMemento memento) {
// default is no-op
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
index b1c5465..d3928b7 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -243,16 +243,23 @@ public class RebindManagerImpl implements RebindManager {
final RebindContextImpl rebindContext = new RebindContextImpl(classLoader);
LookupContext realLookupContext = new RebindContextLookupContext(managementContext, rebindContext, exceptionHandler);
- // Two-phase deserialization.
- // First we deserialize just the "manifest" to find all instances (and their types).
- // Then we deserialize so that inter-entity references can be set.
- //
+ // Four-phase deserialization.
+ // 1. deserialize just the "manifest" to find all instances (and their types).
+ // 2. deserialize so that inter-entity references can be set (and entity config/state is set).
+ // 3. add policies+enrichers to all the entities.
+ // 4. manage the entities
+
// TODO if underlying data-store is changed between first and second phase (e.g. to add an
// entity), then second phase might try to reconstitute an entity that has not been put in
// the rebindContext. This should not affect normal production usage, because rebind is run
// against a data-store that is not being written to by other brooklyn instance(s).
- BrooklynMementoManifest mementoManifest = persister.loadMementoManifest(exceptionHandler);
+ //
+ // PHASE ONE
+ //
+
+ BrooklynMementoManifest mementoManifest = persister.loadMementoManifest(exceptionHandler);
+
// Instantiate locations
LOG.debug("RebindManager instantiating locations: {}", mementoManifest.getLocationIdToType().keySet());
for (Map.Entry<String, String> entry : mementoManifest.getLocationIdToType().entrySet()) {
@@ -319,6 +326,10 @@ public class RebindManagerImpl implements RebindManager {
LOG.debug("Not rebinding enrichers; feature disabled: {}", memento.getEnricherIds());
}
+ //
+ // PHASE TWO
+ //
+
// Reconstruct locations
LOG.debug("RebindManager reconstructing locations");
for (LocationMemento locMemento : sortParentFirst(memento.getLocationMementos()).values()) {
@@ -385,7 +396,35 @@ public class RebindManagerImpl implements RebindManager {
}
}
}
+
+ //
+ // PHASE THREE
+ //
+
+ // Associate policies+enrichers with entities
+ LOG.debug("RebindManager reconstructing entities");
+ for (EntityMemento entityMemento : sortParentFirst(memento.getEntityMementos()).values()) {
+ Entity entity = rebindContext.getEntity(entityMemento.getId());
+ if (LOG.isDebugEnabled()) LOG.debug("RebindManager reconstructing entity {}", entityMemento);
+
+ if (entity == null) {
+ // usually because of creation-failure, when not using fail-fast
+ exceptionHandler.onEntityNotFound(entityMemento.getId());
+ } else {
+ try {
+ entityMemento.injectTypeClass(entity.getClass());
+ ((EntityInternal)entity).getRebindSupport().addPolicies(rebindContext, entityMemento);
+ ((EntityInternal)entity).getRebindSupport().addEnrichers(rebindContext, entityMemento);
+ } catch (Exception e) {
+ exceptionHandler.onRebindEntityFailed(entity, e);
+ }
+ }
+ }
+ //
+ // PHASE FOUR
+ //
+
LOG.debug("RebindManager managing locations");
for (Location location: locations.values()) {
if (location.getParent()==null) {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/test/java/brooklyn/entity/rebind/RebindLocationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/RebindLocationTest.java b/core/src/test/java/brooklyn/entity/rebind/RebindLocationTest.java
index 23e6de2..741e522 100644
--- a/core/src/test/java/brooklyn/entity/rebind/RebindLocationTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/RebindLocationTest.java
@@ -344,8 +344,8 @@ public class RebindLocationTest extends RebindTestFixtureWithApp {
return getMementoWithProperties(MutableMap.<String,Object>of("myfield", myfield));
}
@Override
- protected void doReconsruct(RebindContext rebindContext, LocationMemento memento) {
- super.doReconsruct(rebindContext, memento);
+ protected void doReconstruct(RebindContext rebindContext, LocationMemento memento) {
+ super.doReconstruct(rebindContext, memento);
myfield = (String) memento.getCustomField("myfield");
rebound = true;
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/59ce9e8b/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java b/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
index b136665..4b7767a 100644
--- a/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
@@ -10,8 +10,12 @@ import java.util.Map;
import org.testng.annotations.Test;
import brooklyn.config.ConfigKey;
+import brooklyn.enricher.basic.AbstractEnricher;
+import brooklyn.entity.Group;
+import brooklyn.entity.basic.BasicGroup;
import brooklyn.entity.basic.ConfigKeys;
import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityLocal;
import brooklyn.entity.proxying.EntitySpec;
import brooklyn.entity.rebind.RebindEnricherTest.MyEnricher;
import brooklyn.location.Location;
@@ -25,6 +29,7 @@ import brooklyn.test.entity.TestEntity;
import brooklyn.util.collections.MutableMap;
import brooklyn.util.flags.SetFromFlag;
+import com.google.common.base.Predicates;
import com.google.common.collect.Iterables;
public class RebindPolicyTest extends RebindTestFixtureWithApp {
@@ -163,6 +168,56 @@ public class RebindPolicyTest extends RebindTestFixtureWithApp {
assertFalse(newPolicy.isRebinding());
}
+ // Previously, policy+enricher was added to entity as part of entity.reconstitute, so other entities might not
+ // have been initialised and the relationships not set. If a policy immediately looked at entity's children or
+ // at another entity, then it might find those entities' state uninitialised.
+ //
+ // Longer term, we may want to force policies+enrichers to be inactive until the entity really is managed.
+ // However, currently some policies inject an onEvent message during their `setEntity` method (because
+ // their subscription does not give them the current value - only changed values. Doing that sudo-onEvent is
+ // a bad idea because even on normal startup the entity might still be in its init method, so we shouldn't be
+ // kicking off actions at that point.
+ @Test
+ public void testPolicyAddedWhenEntityRelationshipsSet() throws Exception {
+ BasicGroup origGroup = origApp.createAndManageChild(EntitySpec.create(BasicGroup.class));
+ TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class));
+ origGroup.addMember(origEntity);
+
+ EnricherChecksEntityHierarchy origEnricher = origApp.addEnricher(EnricherSpec.create(EnricherChecksEntityHierarchy.class));
+ PolicyChecksEntityHierarchy origPolicy = origApp.addPolicy(PolicySpec.create(PolicyChecksEntityHierarchy.class));
+ assertTrue(origEnricher.success);
+ assertTrue(origPolicy.success);
+
+ newApp = (TestApplication) rebind();
+ EnricherChecksEntityHierarchy newEnricher = (EnricherChecksEntityHierarchy) Iterables.getOnlyElement(newApp.getEnrichers());
+ PolicyChecksEntityHierarchy newPolicy = (PolicyChecksEntityHierarchy) Iterables.getOnlyElement(newApp.getPolicies());
+
+ assertTrue(newEnricher.success);
+ assertTrue(newPolicy.success);
+ }
+ public static class PolicyChecksEntityHierarchy extends AbstractPolicy {
+ transient volatile boolean success;
+ @Override
+ public void setEntity(EntityLocal entity) {
+ super.setEntity(entity);
+ assertTrue(entity instanceof TestApplication);
+ assertEquals(entity.getChildren().size(), 2);
+ assertEquals(((Group)Iterables.find(entity.getChildren(), Predicates.instanceOf(Group.class))).getMembers().size(), 1);
+ success = true;
+ }
+ }
+ public static class EnricherChecksEntityHierarchy extends AbstractEnricher {
+ transient volatile boolean success;
+ @Override
+ public void setEntity(EntityLocal entity) {
+ super.setEntity(entity);
+ assertTrue(entity instanceof TestApplication);
+ assertEquals(entity.getChildren().size(), 2);
+ assertEquals(((Group)Iterables.find(entity.getChildren(), Predicates.instanceOf(Group.class))).getMembers().size(), 1);
+ success = true;
+ }
+ }
+
public static class PolicyChecksIsRebinding extends AbstractPolicy {
boolean isRebindingValWhenRebinding;