Repository: incubator-brooklyn Updated Branches: refs/heads/master b0eff1ac5 -> 83b27b320
fixes for HA hot backup and notes on better management for subsequent rebinds for child nodes, we apply management to the latest pre-registered instance, not a cached proxy; and lots of notes on manageAll(...) as a better approach than the current manageRebindedRoot Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/cda8069e Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/cda8069e Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/cda8069e Branch: refs/heads/master Commit: cda8069e01e9eeb0d0cd1792ae0d67713a0c465d Parents: b0eff1a Author: Alex Heneveld <[email protected]> Authored: Fri Mar 20 12:31:58 2015 +0000 Committer: Alex Heneveld <[email protected]> Committed: Fri Mar 20 12:35:23 2015 +0000 ---------------------------------------------------------------------- .../java/brooklyn/management/EntityManager.java | 2 +- .../ha/HighAvailabilityManagerImpl.java | 6 +- .../internal/EntityManagementSupport.java | 2 +- .../management/internal/LocalEntityManager.java | 65 ++++++++++++++++---- .../internal/LocalLocationManager.java | 25 ++++---- 5 files changed, 74 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cda8069e/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 2780c1a..67e229d 100644 --- a/api/src/main/java/brooklyn/management/EntityManager.java +++ b/api/src/main/java/brooklyn/management/EntityManager.java @@ -122,5 +122,5 @@ public interface EntityManager { * (for instance because the entity is no longer relevant) */ void unmanage(Entity e); - + } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cda8069e/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java index b2af52a..ec844a3 100644 --- a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java +++ b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java @@ -885,12 +885,14 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager { /** clears all managed items from the management context; same items destroyed as in the course of a rebind cycle */ protected void clearManagedItems(ManagementTransitionMode mode) { + // start with the root applications for (Application app: managementContext.getApplications()) { if (((EntityInternal)app).getManagementSupport().isDeployed()) { - ((EntityInternal)app).getManagementContext().getEntityManager().unmanage(app); + ((LocalEntityManager)((EntityInternal)app).getManagementContext().getEntityManager()).unmanage(app, mode); } } - // for normal management, call above will remove; for read-only, etc, let's do what's below: + // for active management, call above will remove recursively at present, + // but for read-only, and if we stop recursively, go through them all for (Entity entity: managementContext.getEntityManager().getEntities()) { ((LocalEntityManager)managementContext.getEntityManager()).unmanage(entity, mode); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cda8069e/core/src/main/java/brooklyn/management/internal/EntityManagementSupport.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/internal/EntityManagementSupport.java b/core/src/main/java/brooklyn/management/internal/EntityManagementSupport.java index ef9496c..f0f8074 100644 --- a/core/src/main/java/brooklyn/management/internal/EntityManagementSupport.java +++ b/core/src/main/java/brooklyn/management/internal/EntityManagementSupport.java @@ -162,7 +162,7 @@ public class EntityManagementSupport { boolean alreadyManaging = isDeployed(); if (alreadyManaging) { - log.warn("Already managed: "+entity+" ("+nonDeploymentManagementContext+"); onManagementStarted is no-op"); + log.warn("Already managed: "+entity+" ("+nonDeploymentManagementContext+"); onManagementStarting is no-op"); } else if (nonDeploymentManagementContext == null || !nonDeploymentManagementContext.getMode().isPreManaged()) { throw new IllegalStateException("Not in expected pre-managed state: "+entity+" ("+nonDeploymentManagementContext+")"); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cda8069e/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java b/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java index cd66e5b..08ce0de 100644 --- a/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java +++ b/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java @@ -278,11 +278,34 @@ public class LocalEntityManager implements EntityManagerInternal { manageRecursive(item, mode); } - protected void manageRecursive(Entity e, final ManagementTransitionMode initialMode) { - AccessController.Response access = managementContext.getAccessController().canManageEntity(e); + protected void checkManagementAllowed(Entity item) { + AccessController.Response access = managementContext.getAccessController().canManageEntity(item); if (!access.isAllowed()) { - throw new IllegalStateException("Access controller forbids management of "+e+": "+access.getMsg()); - } + throw new IllegalStateException("Access controller forbids management of "+item+": "+access.getMsg()); + } + } + + /* TODO we sloppily use "recursive" to ensure ordering of parent-first in many places + * (which may not be necessary but seems like a good idea), + * and also to collect many entities when doing a big rebind, + * ensuring all have #manageNonRecursive called before calling #onManagementStarted. + * + * it would be better to have a manageAll(Map<Entity,ManagementTransitionMode> items) + * method which did that in two phases, allowing us to selectively rebind, + * esp when we come to want supporting different modes and different brooklyn nodes. + * + * the impl of manageAll could sort them with parents before children, + * (and manageRecursive could simply populate a map and delegate to manageAll). + * + * manageRebindRoot would then go, and the (few) callers would construct the map. + * + * similarly we might want an unmanageAll(), + * although possibly all unmanagement should be recursive, if we assume an entity's ancestors are always at least proxied + * (and the non-recursive RO path here could maybe be dropped) + */ + + protected void manageRecursive(Entity e, final ManagementTransitionMode initialMode) { + checkManagementAllowed(e); final List<EntityInternal> allEntities = Lists.newArrayList(); Predicate<EntityInternal> manageEntity = new Predicate<EntityInternal>() { public boolean apply(EntityInternal it) { @@ -311,8 +334,10 @@ public class LocalEntityManager implements EntityManagerInternal { } else { if (mode.wasPrimary() && mode.isPrimary()) { // active partial rebind; continue + } else if (mode.wasReadOnly() && mode.isReadOnly()) { + // reload in RO mode } else { - // on rebind, should not have any deployed instances + // on initial non-RO rebind, should not have any deployed instances log.warn("Already deployed "+it+" when managing "+mode+"/"+initialMode+"; ignoring this and all descendants"); return false; } @@ -331,6 +356,10 @@ public class LocalEntityManager implements EntityManagerInternal { return manageNonRecursive(it, mode); } }; if (initialMode.wasPrimary() && initialMode.isPrimary()) { + // already managed, so this shouldn't be recursive + // (in ActivePartialRebind we cheat calling in to this method; + // the TODO above removing manageRebindRoot would allow us to avoid this cheat!) + log.debug("Managing "+e+" but skipping recursion, as mode is "+initialMode); manageEntity.apply( (EntityInternal)e ); } else { recursively(e, manageEntity); @@ -378,8 +407,8 @@ public class LocalEntityManager implements EntityManagerInternal { // do not remove from maps below, bail out now return; - } else if (mode.wasReadOnly() && mode.isDestroying()) { - // we are unmanaging an instance (secondary) for which the primary has been destroyed elsewhere + } else if (mode.wasReadOnly() && mode.isNoLongerLoaded()) { + // we are unmanaging an instance (secondary); either stopping here or primary destroyed elsewhere ((EntityInternal)e).getManagementSupport().onManagementStopping(info); unmanageNonRecursive(e); stopTasks(e); @@ -387,10 +416,16 @@ public class LocalEntityManager implements EntityManagerInternal { managementContext.getRebindManager().getChangeListener().onUnmanaged(e); if (managementContext.getGarbageCollector() != null) managementContext.getGarbageCollector().onUnmanaged(e); - } else if (mode.wasPrimary() && mode.isDestroying()) { - // we are unmanaging an instance either because it is being destroyed (primary), - // or due to an explicit call (shutting down all things, read-only and primary); - // in either case, should be recursive + } else if (mode.wasPrimary() && mode.isNoLongerLoaded()) { + // unmanaging a primary; currently this is done recursively + + /* TODO tidy up when it is recursive and when it isn't; if something is being unloaded or destroyed, + * that probably *is* recursive, but the old mode might be different if in some cases things are read-only. + * or maybe nothing needs to be recursive, we just make sure the callers (e.g. HighAvailabilityModeImpl.clearManagedItems) + * call in a good order + * + * see notes above about recursive/manage/All/unmanageAll + */ // Need to store all child entities as onManagementStopping removes a child from the parent entity final List<EntityInternal> allEntities = Lists.newArrayList(); @@ -523,6 +558,14 @@ public class LocalEntityManager implements EntityManagerInternal { } private void recursively(Entity e, Predicate<EntityInternal> action) { + Entity otherPreregistered = preRegisteredEntitiesById.get(e.getId()); + if (otherPreregistered!=null) { + // if something has been pre-registered, prefer it + // (e.g. if we recursing through children, we might have a proxy from previous iteration; + // the most recent will have been pre-registered) + e = otherPreregistered; + } + boolean success = action.apply( (EntityInternal)e ); if (!success) { return; // Don't manage children if action false/unnecessary for parent http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cda8069e/core/src/main/java/brooklyn/management/internal/LocalLocationManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/management/internal/LocalLocationManager.java b/core/src/main/java/brooklyn/management/internal/LocalLocationManager.java index 1fc5096..2f75120 100644 --- a/core/src/main/java/brooklyn/management/internal/LocalLocationManager.java +++ b/core/src/main/java/brooklyn/management/internal/LocalLocationManager.java @@ -203,8 +203,17 @@ public class LocalLocationManager implements LocationManagerInternal { Preconditions.checkNotNull(mode, "Mode not set for rebinding %s", item); manageRecursive(item, mode); } - + + protected void checkManagementAllowed(Location item) { + AccessController.Response access = managementContext.getAccessController().canManageLocation(item); + if (!access.isAllowed()) { + throw new IllegalStateException("Access controller forbids management of "+item+": "+access.getMsg()); + } + } + protected Location manageRecursive(Location loc, final ManagementTransitionMode initialMode) { + // TODO see comments in LocalEntityManager about recursive management / manageRebindRoot v manageAll + AccessController.Response access = managementContext.getAccessController().canManageLocation(loc); if (!access.isAllowed()) { throw new IllegalStateException("Access controller forbids management of "+loc+": "+access.getMsg()); @@ -287,7 +296,7 @@ public class LocalLocationManager implements LocationManagerInternal { // do not remove from maps below, bail out now return; - } else if ((mode.isReadOnly() && mode.wasPrimary()) || (mode.isDestroying() && mode.wasReadOnly())) { + } else if ((mode.wasPrimary() && mode.isReadOnly()) || (mode.wasReadOnly() && mode.isNoLongerLoaded())) { if (mode.isReadOnly() && mode.wasPrimary()) { // TODO shouldn't this fall into "hasBeenReplaced" above? log.debug("Unmanaging on demotion: "+loc+" ("+mode+")"); @@ -297,15 +306,11 @@ public class LocalLocationManager implements LocationManagerInternal { managementContext.getRebindManager().getChangeListener().onUnmanaged(loc); if (managementContext.gc != null) managementContext.gc.onUnmanaged(loc); unmanageNonRecursiveClearItsFields(loc, mode); - } else if (mode.isDestroying()) { - - // TODO isUnloading??? - - // we are unmanaging an instance either because it is being destroyed (primary), - // or due to an explicit call (shutting down all things, read-only and primary); - // in either case, should be recursive + } else if (mode.isNoLongerLoaded()) { // Need to store all child entities as onManagementStopping removes a child from the parent entity + + // As above, see TODO in LocalEntityManager about recursive management / unmanagement v manageAll/unmanageAll recursively(loc, new Predicate<AbstractLocation>() { public boolean apply(AbstractLocation it) { if (shouldSkipUnmanagement(it)) return false; boolean result = unmanageNonRecursiveRemoveFromRecords(it, mode); @@ -326,8 +331,6 @@ public class LocalLocationManager implements LocationManagerInternal { } }); } else { - // what about to unmanaged_persisted? - log.warn("Invalid mode for unmanage: "+mode+" on "+loc+" (ignoring)"); }
