Partial rebind - address code review comments
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/491ca49e Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/491ca49e Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/491ca49e Branch: refs/heads/master Commit: 491ca49efdfe4f8a7bb2bf5cbba5cd592bf4798a Parents: 3c97a57 Author: Alex Heneveld <[email protected]> Authored: Mon Feb 9 12:32:29 2015 +0000 Committer: Alex Heneveld <[email protected]> Committed: Mon Feb 9 12:32:29 2015 +0000 ---------------------------------------------------------------------- .../src/main/java/brooklyn/BrooklynVersion.java | 4 +- .../brooklyn/catalog/internal/CatalogUtils.java | 10 ----- .../rebind/ActivePartialRebindIteration.java | 35 ++++++++------- .../rebind/InitialFullRebindIteration.java | 12 ++--- .../brooklyn/entity/rebind/RebindIteration.java | 14 +++++- .../entity/rebind/RebindManagerImpl.java | 2 +- .../test/java/brooklyn/BrooklynVersionTest.java | 2 +- .../catalog/internal/CatalogTestUtils.java | 46 ++++++++++++++++++++ .../entity/group/DynamicClusterTest.java | 14 ++++++ .../entity/rebind/ActivePartialRebindTest.java | 2 +- .../rebind/ActivePartialRebindVersionTest.java | 4 +- .../persister/XmlMementoSerializerTest.java | 5 +-- .../osgi/OsgiVersionMoreEntityTest.java | 3 +- 13 files changed, 104 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/main/java/brooklyn/BrooklynVersion.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/BrooklynVersion.java b/core/src/main/java/brooklyn/BrooklynVersion.java index 0cdf375..3496d9e 100644 --- a/core/src/main/java/brooklyn/BrooklynVersion.java +++ b/core/src/main/java/brooklyn/BrooklynVersion.java @@ -67,7 +67,7 @@ public class BrooklynVersion { // we read the maven pom metadata and osgi metadata and make sure it's sensible // everything is put into a single map for now (good enough, but should be cleaned up) readPropertiesFromMavenResource(BrooklynVersion.class.getClassLoader()); - readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), "org.apache.brooklyn.core"); + readPropertiesFromOsgiResource(BrooklynVersion.class.getClassLoader(), BROOKLYN_CORE_SYMBOLIC_NAME); // TODO there is also build-metadata.properties used in ServerResource /v1/server/version endpoint // see comments on that about folding it into this class instead @@ -227,7 +227,7 @@ public class BrooklynVersion { } public static String get() { - return getVersionFromStatic(); + return INSTANCE.getVersion(); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/main/java/brooklyn/catalog/internal/CatalogUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogUtils.java b/core/src/main/java/brooklyn/catalog/internal/CatalogUtils.java index 9741672..ed4d06d 100644 --- a/core/src/main/java/brooklyn/catalog/internal/CatalogUtils.java +++ b/core/src/main/java/brooklyn/catalog/internal/CatalogUtils.java @@ -34,7 +34,6 @@ import brooklyn.catalog.internal.BasicBrooklynCatalog.BrooklynLoaderTracker; import brooklyn.config.BrooklynLogging; import brooklyn.entity.Entity; import brooklyn.entity.basic.EntityInternal; -import brooklyn.entity.proxying.EntitySpec; import brooklyn.entity.rebind.RebindManagerImpl.RebindTracker; import brooklyn.management.ManagementContext; import brooklyn.management.classloading.BrooklynClassLoadingContext; @@ -228,13 +227,4 @@ public class CatalogUtils { } } - @Beta - public static EntitySpec<?> createEntitySpec(ManagementContext mgmt, CatalogItem<?, ?> catalogItem) { - BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(mgmt, catalogItem); - @SuppressWarnings({ "unchecked", "rawtypes" }) - EntitySpec<?> spec = EntitySpec.create( (Class)loader.loadClass(catalogItem.getJavaType()) ); - spec.catalogItemId(catalogItem.getId()); - return spec; - } - } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/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 d3c4f5f..ff315ca 100644 --- a/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/ActivePartialRebindIteration.java @@ -74,18 +74,23 @@ public class ActivePartialRebindIteration extends RebindIteration { } public void applyTransformer(CompoundTransformer transformer) { - if (transformer!=null) - transformers.add(transformer); + transformers.add(Preconditions.checkNotNull(transformer, "transformer")); } @Override protected void doRun() throws Exception { - checkEnteringPhase(1); - Preconditions.checkState(readOnlyRebindCount.get()==Integer.MIN_VALUE, "Partial rebind in read-only mode not supported"); + Preconditions.checkState(rebindManager.getRebindMode()==ManagementNodeState.MASTER, "Partial rebind only supported in master mode, not "+rebindManager.getRebindMode()); + Preconditions.checkState(readOnlyRebindCount.get()==Integer.MIN_VALUE, "Rebind count should be MIN when running in master mode"); Preconditions.checkNotNull(objectsToRebindInitial, "Objects to rebind must be set"); LOG.debug("Partial rebind Rebinding ("+mode+") from "+rebindManager.getPersister().getBackingStoreDescription()+"..."); + super.doRun(); + } + + @Override + protected void loadManifestFiles() throws Exception { + checkEnteringPhase(1); Builder mementoRawBuilder = BrooklynMementoRawData.builder(); /* @@ -116,21 +121,8 @@ public class ActivePartialRebindIteration extends RebindIteration { mementoRawData = mementoRawBuilder.build(); preprocessManifestFiles(); - - // skip this phase, as catalog is not being changed - // (and we don't want to unload things) -// rebuildCatalog(); - phase++; - - instantiateLocationsAndEntities(); - instantiateMementos(); - instantiateAdjuncts(instantiator); - reconstructEverything(); - associateAdjunctsWithEntities(); - manageTheObjects(); - finishingUp(); } - + @Override protected void preprocessManifestFiles() throws Exception { for (CompoundTransformer transformer: transformers) { @@ -141,6 +133,13 @@ public class ActivePartialRebindIteration extends RebindIteration { } @Override + protected void rebuildCatalog() { + checkEnteringPhase(2); + + // skip; old catalog items should be re-used + } + + @Override protected Collection<String> getMementoRootEntities() { // all entities are roots here, because we are not recursing return memento.getEntityIds(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java index 44ef922..a84d34b 100644 --- a/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java @@ -60,22 +60,16 @@ public class InitialFullRebindIteration extends RebindIteration { return false; } + @Override protected void doRun() throws Exception { LOG.debug("Rebinding ("+mode+ (readOnlyRebindCount.get()>Integer.MIN_VALUE ? ", iteration "+readOnlyRebindCount : "")+ ") from "+rebindManager.getPersister().getBackingStoreDescription()+"..."); - loadManifestFiles(); - rebuildCatalog(); - instantiateLocationsAndEntities(); - instantiateMementos(); - instantiateAdjuncts(instantiator); - reconstructEverything(); - associateAdjunctsWithEntities(); - manageTheObjects(); - finishingUp(); + super.doRun(); } + @Override protected void loadManifestFiles() throws Exception { checkEnteringPhase(1); Preconditions.checkState(mementoRawData==null, "Memento raw data should not yet be set when calling this"); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/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 3582030..3d22d0b 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java @@ -232,7 +232,19 @@ public abstract class RebindIteration { return rebindContext; } - protected abstract void doRun() throws Exception; + protected void doRun() throws Exception { + loadManifestFiles(); + rebuildCatalog(); + instantiateLocationsAndEntities(); + instantiateMementos(); + instantiateAdjuncts(instantiator); + reconstructEverything(); + associateAdjunctsWithEntities(); + manageTheObjects(); + finishingUp(); + } + + protected abstract void loadManifestFiles() throws Exception; public void run() { if (iterationStarted.getAndSet(true)) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/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 e6f4aea..ca85a03 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java @@ -382,7 +382,7 @@ public class RebindManagerImpl implements RebindManager { rebindActive, readOnlyRebindCount, rebindMetrics, persistenceStoreAccess); iteration.setObjectIterator(objectsToRebind); - iteration.applyTransformer(transformer); + if (transformer!=null) iteration.applyTransformer(transformer); iteration.run(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/BrooklynVersionTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/BrooklynVersionTest.java b/core/src/test/java/brooklyn/BrooklynVersionTest.java index 887d8e4..9683600 100644 --- a/core/src/test/java/brooklyn/BrooklynVersionTest.java +++ b/core/src/test/java/brooklyn/BrooklynVersionTest.java @@ -63,7 +63,7 @@ public class BrooklynVersionTest { log.info("sha1: "+sha1); if (Strings.isNonBlank(sha1) || BrooklynVersion.isDevelopmentEnvironment()) return; - // we might not have a SHA1 if it's a standalone source build; just log warn in that case + // we might not have a SHA1 if it's a standalone (non-git) source build; just log warn in that case log.warn("This build does not have git SHA1 information."); // (can't assert anything, except that sha1 lookup doesn't NPE) } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/catalog/internal/CatalogTestUtils.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/catalog/internal/CatalogTestUtils.java b/core/src/test/java/brooklyn/catalog/internal/CatalogTestUtils.java new file mode 100644 index 0000000..14005a5 --- /dev/null +++ b/core/src/test/java/brooklyn/catalog/internal/CatalogTestUtils.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package brooklyn.catalog.internal; + +import brooklyn.catalog.CatalogItem; +import brooklyn.entity.proxying.EntitySpec; +import brooklyn.management.ManagementContext; +import brooklyn.management.classloading.BrooklynClassLoadingContext; + +import com.google.common.annotations.Beta; + +public class CatalogTestUtils { + + /** creates entity spec with the java type of a catalog item; + * would be nice to have this in {@link CatalogUtils}, + * but the logic for parsing the yaml is buried in camp code, + * so it's a little bit hard to make this a first class method. + * <p> + * (this impl ignores many things, including config and location.) + */ + @Beta + public static EntitySpec<?> createEssentialEntitySpec(ManagementContext mgmt, CatalogItem<?, ?> catalogItem) { + BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(mgmt, catalogItem); + @SuppressWarnings({ "unchecked", "rawtypes" }) + EntitySpec<?> spec = EntitySpec.create( (Class)loader.loadClass(catalogItem.getJavaType()) ); + spec.catalogItemId(catalogItem.getId()); + return spec; + } + +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java index ad5394a..8f953a4 100644 --- a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java +++ b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java @@ -46,6 +46,7 @@ import org.testng.annotations.Test; import brooklyn.entity.BrooklynAppUnitTestSupport; import brooklyn.entity.Entity; import brooklyn.entity.basic.Attributes; +import brooklyn.entity.basic.BasicEntity; import brooklyn.entity.basic.BrooklynTaskTags; import brooklyn.entity.basic.Entities; import brooklyn.entity.basic.EntityFactory; @@ -64,6 +65,7 @@ import brooklyn.test.EntityTestUtils; import brooklyn.test.entity.TestEntity; import brooklyn.test.entity.TestEntityImpl; import brooklyn.util.collections.MutableMap; +import brooklyn.util.collections.QuorumCheck.QuorumChecks; import brooklyn.util.exceptions.Exceptions; import brooklyn.util.time.Time; @@ -875,6 +877,18 @@ public class DynamicClusterTest extends BrooklynAppUnitTestSupport { assertEquals(cluster.getMembers().size(), 1); } + @Test + public void testWithNonStartableEntity() throws Exception { + DynamicCluster cluster = app.createAndManageChild(EntitySpec.create(DynamicCluster.class) + .configure(DynamicCluster.MEMBER_SPEC, EntitySpec.create(BasicEntity.class)) + .configure(DynamicCluster.UP_QUORUM_CHECK, QuorumChecks.alwaysTrue()) + .configure(DynamicCluster.INITIAL_SIZE, 2)); + cluster.start(ImmutableList.of(loc)); + + EntityTestUtils.assertAttributeEqualsEventually(cluster, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING); + assertTrue(cluster.getAttribute(Attributes.SERVICE_UP)); + } + private Throwable unwrapException(Throwable e) { if (e instanceof ExecutionException) { return unwrapException(e.getCause()); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java b/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java index 414ca25..f43cbe5 100644 --- a/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java +++ b/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java @@ -87,7 +87,7 @@ public class ActivePartialRebindTest extends RebindTestFixtureWithApp { gcAndLog("before"); long used0 = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); - for (int i=0; i<500; i++) { + for (int i=0; i<200; i++) { doPartialRebindOfIds(c1.getId()); origManagementContext.getGarbageCollector().gcIteration(); gcAndLog("iteration "+i); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindVersionTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindVersionTest.java b/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindVersionTest.java index 9eec02b..e9594b9 100644 --- a/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindVersionTest.java +++ b/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindVersionTest.java @@ -24,7 +24,7 @@ import org.testng.Assert; import org.testng.annotations.Test; import brooklyn.catalog.CatalogItem; -import brooklyn.catalog.internal.CatalogUtils; +import brooklyn.catalog.internal.CatalogTestUtils; import brooklyn.entity.Entity; import brooklyn.entity.group.DynamicCluster; import brooklyn.entity.proxying.EntitySpec; @@ -94,7 +94,7 @@ public class ActivePartialRebindVersionTest extends RebindTestFixtureWithApp { // + " entitySpec: { type: "+catV1.getId()+" }\n", true); DynamicCluster cluster = origApp.createAndManageChild(EntitySpec.create(DynamicCluster.class) .configure(DynamicCluster.INITIAL_SIZE, 1) - .configure(DynamicCluster.MEMBER_SPEC, CatalogUtils.createEntitySpec(origManagementContext, catV1)) + .configure(DynamicCluster.MEMBER_SPEC, CatalogTestUtils.createEssentialEntitySpec(origManagementContext, catV1)) ); cluster.start(MutableList.of(origApp.newSimulatedLocation())); Entity childV1 = MutableList.copyOf(cluster.getChildren()).get(1); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java b/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java index 6083134..a57c9eb 100644 --- a/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java +++ b/core/src/test/java/brooklyn/entity/rebind/persister/XmlMementoSerializerTest.java @@ -37,7 +37,7 @@ import brooklyn.basic.BrooklynObject; import brooklyn.catalog.CatalogItem; import brooklyn.catalog.internal.CatalogItemBuilder; import brooklyn.catalog.internal.CatalogItemDtoAbstract; -import brooklyn.catalog.internal.CatalogUtils; +import brooklyn.catalog.internal.CatalogTestUtils; import brooklyn.entity.Entity; import brooklyn.entity.Feed; import brooklyn.entity.basic.Entities; @@ -47,7 +47,6 @@ import brooklyn.entity.rebind.BrooklynObjectType; import brooklyn.location.Location; import brooklyn.location.LocationSpec; import brooklyn.management.ManagementContext; -import brooklyn.management.internal.LocalManagementContext; import brooklyn.management.osgi.OsgiVersionMoreEntityTest; import brooklyn.mementos.BrooklynMementoPersister.LookupContext; import brooklyn.policy.Enricher; @@ -203,7 +202,7 @@ public class XmlMementoSerializerTest { EntitySpec<DynamicCluster> spec = EntitySpec.create(DynamicCluster.class) .configure(DynamicCluster.INITIAL_SIZE, 1) - .configure(DynamicCluster.MEMBER_SPEC, CatalogUtils.createEntitySpec(mgmt, ci)); + .configure(DynamicCluster.MEMBER_SPEC, CatalogTestUtils.createEssentialEntitySpec(mgmt, ci)); serializer.setLookupContext(new LookupContextImpl(mgmt, ImmutableList.<Entity>of(), ImmutableList.<Location>of(), ImmutableList.<Policy>of(), http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/491ca49e/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java b/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java index 4e1bb94..967bb6d 100644 --- a/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java +++ b/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java @@ -36,6 +36,7 @@ import brooklyn.catalog.CatalogItem; import brooklyn.catalog.internal.CatalogEntityItemDto; import brooklyn.catalog.internal.CatalogItemBuilder; import brooklyn.catalog.internal.CatalogItemDtoAbstract; +import brooklyn.catalog.internal.CatalogTestUtils; import brooklyn.catalog.internal.CatalogUtils; import brooklyn.entity.Entity; import brooklyn.entity.basic.Entities; @@ -162,7 +163,7 @@ public class OsgiVersionMoreEntityTest { } public static Entity addItemFromCatalog(ManagementContext mgmt, TestApplication parent, CatalogItem<?, ?> c2) { - return parent.createAndManageChild( CatalogUtils.createEntitySpec(mgmt, c2) ); + return parent.createAndManageChild( CatalogTestUtils.createEssentialEntitySpec(mgmt, c2) ); } public static void assertV1MethodCall(Entity me) throws IllegalAccessException, InvocationTargetException, NoSuchMethodException {
