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 2015/02/09 16:36:39 UTC

[20/22] incubator-brooklyn git commit: partial rebind - more code review, esp much better javadoc

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 <al...@cloudsoftcorp.com>
Authored: Mon Feb 9 14:23:18 2015 +0000
Committer: Alex Heneveld <al...@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;
     }