partial rebind - more code review, esp much better javadoc
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/b13cb63a Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/b13cb63a Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/b13cb63a Branch: refs/heads/master Commit: b13cb63a4a23e847a3595a79a6fa2aab2609008a Parents: 491ca49 Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Mon Feb 9 14:23:18 2015 +0000 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Mon Feb 9 14:23:18 2015 +0000 ---------------------------------------------------------------------- .../java/brooklyn/management/EntityManager.java | 2 ++ .../rebind/ActivePartialRebindIteration.java | 8 +++++++- .../brooklyn/entity/rebind/RebindContextImpl.java | 13 +++++++------ .../entity/rebind/RebindContextLookupContext.java | 9 +-------- .../brooklyn/entity/rebind/RebindIteration.java | 9 ++------- .../brooklyn/entity/rebind/RebindManagerImpl.java | 17 +++++++++++++++-- .../rebind/persister/XmlMementoSerializer.java | 1 + .../internal/BrooklynObjectManagementMode.java | 1 + .../internal/ManagementTransitionInfo.java | 2 ++ .../internal/ManagementTransitionMode.java | 6 ++++++ .../BrooklynMementoPersisterTestFixture.java | 6 ++---- 11 files changed, 46 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/api/src/main/java/brooklyn/management/EntityManager.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/brooklyn/management/EntityManager.java b/api/src/main/java/brooklyn/management/EntityManager.java index 5a709fa..2780c1a 100644 --- a/api/src/main/java/brooklyn/management/EntityManager.java +++ b/api/src/main/java/brooklyn/management/EntityManager.java @@ -113,6 +113,8 @@ public interface EntityManager { * this might push it out to one or more remote management nodes. * Manage an entity. */ + // TODO manage and unmanage without arguments should be changed to take an explicit ManagementTransitionMode + // (but that class is not currently in the API project) void manage(Entity e); /** http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java index ff315ca..76e600a 100644 --- a/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java @@ -46,7 +46,8 @@ import brooklyn.util.collections.MutableSet; import com.google.common.base.Preconditions; /** - * Pauses a set of existing entities, writes their state, applies a transformation, then reads them back. + * Replaces a set of existing entities (and their adjunts) and locations: + * writes their state, applies a transformation, then reads the state back. */ public class ActivePartialRebindIteration extends RebindIteration { @@ -88,6 +89,11 @@ public class ActivePartialRebindIteration extends RebindIteration { super.doRun(); } + /** Rather than loading from the remote persistence store (as {@link InitialFullRebindIteration} does), + * this constructs the memento data by serializing the objects we are replacing. + * TODO: Currently this does not do any pausing or unmanagement or guarding write access, + * so there is a short window for data loss between this write and the subsequent read. + */ @Override protected void loadManifestFiles() throws Exception { checkEnteringPhase(1); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java index a581c7d..b7eba1a 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java @@ -28,6 +28,7 @@ import brooklyn.catalog.CatalogItem; import brooklyn.entity.Entity; import brooklyn.entity.Feed; import brooklyn.location.Location; +import brooklyn.management.ManagementContext; import brooklyn.mementos.BrooklynMementoPersister.LookupContext; import brooklyn.policy.Enricher; import brooklyn.policy.Policy; @@ -45,14 +46,18 @@ public class RebindContextImpl implements RebindContext { private final Map<String, CatalogItem<?, ?>> catalogItems = Maps.newLinkedHashMap(); private final ClassLoader classLoader; + @SuppressWarnings("unused") + private final ManagementContext mgmt; private final RebindExceptionHandler exceptionHandler; - private LookupContext lookupContext; + private final LookupContext lookupContext; private boolean allAreReadOnly = false; - public RebindContextImpl(RebindExceptionHandler exceptionHandler, ClassLoader classLoader) { + public RebindContextImpl(ManagementContext mgmt, RebindExceptionHandler exceptionHandler, ClassLoader classLoader) { + this.mgmt = checkNotNull(mgmt, "mgmt"); this.exceptionHandler = checkNotNull(exceptionHandler, "exceptionHandler"); this.classLoader = checkNotNull(classLoader, "classLoader"); + this.lookupContext = new RebindContextLookupContext(mgmt, this, exceptionHandler); } public void registerEntity(String id, Entity entity) { @@ -171,10 +176,6 @@ public class RebindContextImpl implements RebindContext { return allAreReadOnly; } - public void setLookupContext(LookupContext lookupContext) { - this.lookupContext = lookupContext; - } - @Override public LookupContext lookup() { return lookupContext; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/entity/rebind/RebindContextLookupContext.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindContextLookupContext.java b/core/src/main/java/brooklyn/entity/rebind/RebindContextLookupContext.java index 05730d7..7024bfe 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindContextLookupContext.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindContextLookupContext.java @@ -34,6 +34,7 @@ import brooklyn.mementos.BrooklynMementoPersister.LookupContext; import brooklyn.policy.Enricher; import brooklyn.policy.Policy; +/** Looks in {@link RebindContext} <i>and</i> {@link ManagementContext} to find entities, locations, etc. */ public class RebindContextLookupContext implements LookupContext { @SuppressWarnings("unused") @@ -44,19 +45,11 @@ public class RebindContextLookupContext implements LookupContext { protected final RebindContextImpl rebindContext; protected final RebindExceptionHandler exceptionHandler; - protected final boolean lookInManagementContext; - public RebindContextLookupContext(RebindContextImpl rebindContext, RebindExceptionHandler exceptionHandler) { - this(null, rebindContext, exceptionHandler); - } public RebindContextLookupContext(ManagementContext managementContext, RebindContextImpl rebindContext, RebindExceptionHandler exceptionHandler) { - this(managementContext, rebindContext, exceptionHandler, false); - } - public RebindContextLookupContext(ManagementContext managementContext, RebindContextImpl rebindContext, RebindExceptionHandler exceptionHandler, boolean lookInManagementContext) { this.managementContext = managementContext; this.rebindContext = rebindContext; this.exceptionHandler = exceptionHandler; - this.lookInManagementContext = lookInManagementContext; } @Override public ManagementContext lookupManagementContext() { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java index 3d22d0b..de956ae 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java @@ -73,7 +73,6 @@ import brooklyn.mementos.BrooklynMemento; import brooklyn.mementos.BrooklynMementoManifest; import brooklyn.mementos.BrooklynMementoManifest.EntityMementoManifest; import brooklyn.mementos.BrooklynMementoPersister; -import brooklyn.mementos.BrooklynMementoPersister.LookupContext; import brooklyn.mementos.BrooklynMementoRawData; import brooklyn.mementos.CatalogItemMemento; import brooklyn.mementos.EnricherMemento; @@ -163,7 +162,6 @@ public abstract class RebindIteration { protected final AtomicBoolean iterationStarted = new AtomicBoolean(); protected final RebindContextImpl rebindContext; protected final Reflections reflections; - protected final LookupContext lookupContext; protected final BrooklynObjectInstantiator instantiator; // populated in the course of a run @@ -210,11 +208,8 @@ public abstract class RebindIteration { this.persistenceStoreAccess = persistenceStoreAccess; managementContext = rebindManager.getManagementContext(); - rebindContext = new RebindContextImpl(exceptionHandler, classLoader); + rebindContext = new RebindContextImpl(managementContext, exceptionHandler, classLoader); reflections = new Reflections(classLoader); - lookupContext = new RebindContextLookupContext(managementContext, rebindContext, exceptionHandler); - // TODO there seems to be a lot of redundancy between RebindContext and LookupContext; do we need both? - rebindContext.setLookupContext(lookupContext); instantiator = new BrooklynObjectInstantiator(classLoader, rebindContext, reflections); if (mode==ManagementNodeState.HOT_STANDBY || mode==ManagementNodeState.HOT_BACKUP) { @@ -422,7 +417,7 @@ public abstract class RebindIteration { checkEnteringPhase(4); - memento = persistenceStoreAccess.loadMemento(mementoRawData, lookupContext, exceptionHandler); + memento = persistenceStoreAccess.loadMemento(mementoRawData, rebindContext.lookup(), exceptionHandler); } protected void instantiateAdjuncts(BrooklynObjectInstantiator instantiator) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/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 ca85a03..5d155cb 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java @@ -68,6 +68,8 @@ import brooklyn.util.time.Duration; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; +import com.google.common.collect.Iterators; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -368,6 +370,10 @@ public class RebindManagerImpl implements RebindManager { public void rebindPartialActive(CompoundTransformer transformer, Iterator<BrooklynObject> objectsToRebind) { final ClassLoader classLoader = managementContext.getCatalog().getRootClassLoader(); + // TODO we might want different exception handling for partials; + // failure at various points should leave proxies in a sensible state, + // either pointing at old or at new, though this is relatively untested, + // and some things e.g. policies might not be properly started final RebindExceptionHandler exceptionHandler = RebindExceptionHandlerImpl.builder() .danglingRefFailureMode(danglingRefFailureMode) @@ -381,7 +387,15 @@ public class RebindManagerImpl implements RebindManager { ActivePartialRebindIteration iteration = new ActivePartialRebindIteration(this, mode, classLoader, exceptionHandler, rebindActive, readOnlyRebindCount, rebindMetrics, persistenceStoreAccess); - iteration.setObjectIterator(objectsToRebind); + iteration.setObjectIterator(Iterators.transform(objectsToRebind, + new Function<BrooklynObject,BrooklynObject>() { + @Override + public BrooklynObject apply(BrooklynObject obj) { + // entities must be deproxied + if (obj instanceof Entity) obj = Entities.deproxy((Entity)obj); + return obj; + } + })); if (transformer!=null) iteration.applyTransformer(transformer); iteration.run(); } @@ -390,7 +404,6 @@ public class RebindManagerImpl implements RebindManager { List<BrooklynObject> objectsToRebind = MutableList.of(); for (String objectId: objectsToRebindIds) { BrooklynObject obj = managementContext.lookup(objectId); - if (obj instanceof Entity) obj = Entities.deproxy((Entity)obj); objectsToRebind.add(obj); } rebindPartialActive(transformer, objectsToRebind.iterator()); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java b/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java index 78644b3..0d88bea 100644 --- a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java +++ b/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java @@ -496,6 +496,7 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T> implements Memento Thread oldOwner = xstreamLockOwner.getAndSet(null); throw new IllegalStateException("xstream was locked by "+oldOwner+" but unlock attempt by "+Thread.currentThread()); } + xstreamLockOwner.notifyAll(); } } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/management/internal/BrooklynObjectManagementMode.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/internal/BrooklynObjectManagementMode.java b/core/src/main/java/brooklyn/management/internal/BrooklynObjectManagementMode.java index a005c2d..dd9d8bc 100644 --- a/core/src/main/java/brooklyn/management/internal/BrooklynObjectManagementMode.java +++ b/core/src/main/java/brooklyn/management/internal/BrooklynObjectManagementMode.java @@ -18,6 +18,7 @@ */ package brooklyn.management.internal; +/** Indicates how an entity/location/adjunct is treated at a given {@link ManagementContext} */ public enum BrooklynObjectManagementMode { /** item does not exist, not in memory, nor persisted (e.g. creating for first time, or finally destroying) */ NONEXISTENT, http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/management/internal/ManagementTransitionInfo.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/internal/ManagementTransitionInfo.java b/core/src/main/java/brooklyn/management/internal/ManagementTransitionInfo.java index 550c977..8b3c43d 100644 --- a/core/src/main/java/brooklyn/management/internal/ManagementTransitionInfo.java +++ b/core/src/main/java/brooklyn/management/internal/ManagementTransitionInfo.java @@ -20,6 +20,8 @@ package brooklyn.management.internal; import brooklyn.management.ManagementContext; +/** Stores a management transition mode, and the management context. */ +// TODO does this class really pull its weight? public class ManagementTransitionInfo { final ManagementContext mgmtContext; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/main/java/brooklyn/management/internal/ManagementTransitionMode.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/internal/ManagementTransitionMode.java b/core/src/main/java/brooklyn/management/internal/ManagementTransitionMode.java index 3c630e1..1d25902 100644 --- a/core/src/main/java/brooklyn/management/internal/ManagementTransitionMode.java +++ b/core/src/main/java/brooklyn/management/internal/ManagementTransitionMode.java @@ -23,6 +23,12 @@ import org.slf4j.LoggerFactory; import com.google.common.base.Preconditions; +/** + * Records details of a management transition, specifically the {@link BrooklynObjectManagementMode} before and after, + * and allows easy checking of various aspects of that. + * <p> + * This helps make code readable and keep correct logic if we expand/change the management modes. + */ public class ManagementTransitionMode { private static final Logger log = LoggerFactory.getLogger(ManagementTransitionMode.class); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b13cb63a/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterTestFixture.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterTestFixture.java b/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterTestFixture.java index 3306a32..9c608f2 100644 --- a/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterTestFixture.java +++ b/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterTestFixture.java @@ -36,7 +36,6 @@ import brooklyn.entity.proxying.EntitySpec; import brooklyn.entity.rebind.PersistenceExceptionHandler; import brooklyn.entity.rebind.PersistenceExceptionHandlerImpl; import brooklyn.entity.rebind.RebindContextImpl; -import brooklyn.entity.rebind.RebindContextLookupContext; import brooklyn.entity.rebind.RebindManager.RebindFailureMode; import brooklyn.entity.rebind.RebindTestUtils; import brooklyn.entity.rebind.RecordingRebindExceptionHandler; @@ -104,14 +103,13 @@ public abstract class BrooklynMementoPersisterTestFixture { RebindTestUtils.waitForPersisted(localManagementContext); RecordingRebindExceptionHandler failFast = new RecordingRebindExceptionHandler(RebindFailureMode.FAIL_FAST, RebindFailureMode.FAIL_FAST); - RebindContextImpl rebindContext = new RebindContextImpl(failFast, classLoader); - RebindContextLookupContext lookupContext = new RebindContextLookupContext(localManagementContext, rebindContext, failFast); + RebindContextImpl rebindContext = new RebindContextImpl(localManagementContext, failFast, classLoader); // here we force these two to be reegistered in order to resolve the enricher and policy // (normally rebind will do that after loading the manifests, but in this test we are just looking at persistence/manifest) rebindContext.registerEntity(app.getId(), app); rebindContext.registerEntity(entity.getId(), entity); - BrooklynMemento reloadedMemento = persister.loadMemento(null, lookupContext, failFast); + BrooklynMemento reloadedMemento = persister.loadMemento(null, rebindContext.lookup(), failFast); return reloadedMemento; }
