Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 8224E200BEF for ; Tue, 29 Nov 2016 14:40:18 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 813AF160B05; Tue, 29 Nov 2016 13:40:18 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id DC9AF160B2B for ; Tue, 29 Nov 2016 14:40:16 +0100 (CET) Received: (qmail 55458 invoked by uid 500); 29 Nov 2016 13:40:16 -0000 Mailing-List: contact commits-help@brooklyn.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.apache.org Delivered-To: mailing list commits@brooklyn.apache.org Received: (qmail 55358 invoked by uid 99); 29 Nov 2016 13:40:16 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 29 Nov 2016 13:40:16 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id BDB65F16A9; Tue, 29 Nov 2016 13:40:15 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: heneveld@apache.org To: commits@brooklyn.apache.org Date: Tue, 29 Nov 2016 13:40:23 -0000 Message-Id: <4ea7627aa6c84a1abbd1f1653b9bfb95@git.apache.org> In-Reply-To: <3d625b9ed94c42a281a87e57c40f842b@git.apache.org> References: <3d625b9ed94c42a281a87e57c40f842b@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [09/13] brooklyn-server git commit: BROOKLYN-345 address review comments archived-at: Tue, 29 Nov 2016 13:40:18 -0000 BROOKLYN-345 address review comments - fix test of catalogYamlWithDslReferenceParentDefault - ServiceFailureDetector.lastPublished - use a real configKey instead of `@SetFromFlag` - Entity memento: persist entire configKey if not same as a key statically defined on the entity class. Don’t just use name-equivalence, as then couldn’t change defaultVal in yawl’s `brooklyn.parameters`. Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a9ec838b Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a9ec838b Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a9ec838b Branch: refs/heads/master Commit: a9ec838b1957c17a75692f9e46d538cd75a93b34 Parents: dba7494 Author: Aled Sage Authored: Wed Nov 16 21:50:00 2016 +0000 Committer: Aled Sage Committed: Mon Nov 28 21:11:48 2016 +0000 ---------------------------------------------------------------------- .../camp/brooklyn/ConfigParametersYamlTest.java | 5 ++++ .../ServiceFailureDetectorYamlRebindTest.java | 11 ++++---- .../ServiceFailureDetectorYamlTest.java | 27 ++++++++++++------ .../mgmt/rebind/dto/BasicEntityMemento.java | 28 ++++++++++--------- .../policy/ha/ServiceFailureDetector.java | 29 ++++++++++++++------ 5 files changed, 65 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a9ec838b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java index 79ab2a4..78219c6 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java @@ -133,6 +133,11 @@ public class ConfigParametersYamlTest extends AbstractYamlRebindTest { // Check config key is listed assertKeyEquals(entity, TestEntity.CONF_NAME.getName(), "myDescription", String.class, "myDefaultYamlVal", "myDefaultYamlVal"); + + // Rebind, and then check again that the config key is listed + Entity newApp = rebind(); + TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(newApp.getChildren()); + assertKeyEquals(newEntity, TestEntity.CONF_NAME.getName(), "myDescription", String.class, "myDefaultYamlVal", "myDefaultYamlVal"); } @Test http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a9ec838b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlRebindTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlRebindTest.java index 377a2a5..d10be45 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlRebindTest.java @@ -28,6 +28,7 @@ import static org.apache.brooklyn.camp.brooklyn.ServiceFailureDetectorYamlTest.s import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.rebind.RebindManager.RebindFailureMode; +import org.apache.brooklyn.core.entity.EntityPredicates; import org.apache.brooklyn.core.entity.RecordingSensorEventListener; import org.apache.brooklyn.core.entity.StartableApplication; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ServiceNotUpLogic; @@ -57,7 +58,7 @@ public class ServiceFailureDetectorYamlRebindTest extends AbstractYamlRebindTest public void testRebindWhenHealthyWithDslConfig() throws Exception { runRebindWhenHealthy(catalogYamlWithDsl, appVersionedId); - TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(app().getChildren()); + TestEntity newEntity = (TestEntity) Iterables.find(app().getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); ServiceFailureDetector newEnricher = assertHasEnricher(newEntity, ServiceFailureDetector.class); assertEnricherConfigMatchesDsl(newEnricher); } @@ -66,7 +67,7 @@ public class ServiceFailureDetectorYamlRebindTest extends AbstractYamlRebindTest public void testRebindWhenHealthyWithDslConfigReferenceParentDefault() throws Exception { runRebindWhenHealthy(catalogYamlWithDslReferenceParentDefault, appVersionedId); - TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(app().getChildren()); + TestEntity newEntity = (TestEntity) Iterables.find(app().getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); ServiceFailureDetector newEnricher = assertHasEnricher(newEntity, ServiceFailureDetector.class); assertEnricherConfigMatchesDsl(newEnricher); } @@ -98,7 +99,7 @@ public class ServiceFailureDetectorYamlRebindTest extends AbstractYamlRebindTest // Rebind StartableApplication newApp = rebind(); - TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(newApp.getChildren()); + TestEntity newEntity = (TestEntity) Iterables.find(newApp.getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); assertHasEnricher(newEntity, ServiceFailureDetector.class); // Confirm ServiceFailureDetector still functions @@ -118,14 +119,14 @@ public class ServiceFailureDetectorYamlRebindTest extends AbstractYamlRebindTest Entity app = createStartWaitAndLogApplication(appYaml); // Make entity go on-fire - TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + TestEntity entity = (TestEntity) Iterables.find(app.getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); RecordingSensorEventListener listener = subscribeToHaSensors(entity); ServiceNotUpLogic.updateNotUpIndicator(entity, INDICATOR_KEY_1, "Simulating a problem"); listener.assertHasEventEventually(SensorEventPredicates.sensorEqualTo(HASensors.ENTITY_FAILED)); // Rebind StartableApplication newApp = rebind(); - TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(newApp.getChildren()); + TestEntity newEntity = (TestEntity) Iterables.find(newApp.getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); assertHasEnricher(newEntity, ServiceFailureDetector.class); // Confirm ServiceFailureDetector still functions http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a9ec838b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlTest.java index 4374c2b..b7b4ffb 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ServiceFailureDetectorYamlTest.java @@ -26,6 +26,7 @@ import java.util.Map; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.entity.EntityPredicates; import org.apache.brooklyn.core.entity.RecordingSensorEventListener; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ServiceNotUpLogic; import org.apache.brooklyn.core.sensor.SensorEventPredicates; @@ -61,6 +62,7 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { " itemType: entity", " item:", " type: " + TestEntity.class.getName(), + " name: targetEntity", " brooklyn.enrichers:", " - type: " + ServiceFailureDetector.class.getName()); @@ -72,6 +74,7 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { " item:", " services:", " - type: " + TestEntity.class.getName(),//FailingEntity.class.getName(), + " name: targetEntity", " brooklyn.parameters:", " - name: custom.stabilizationDelay", " type: " + Duration.class.getName(), @@ -87,6 +90,11 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { " entityRecovered.stabilizationDelay: $brooklyn:config(\"custom.stabilizationDelay\")", " entityFailed.republishTime: $brooklyn:config(\"custom.republishTime\")"); + /* + * TODO Currently have to use `scopeRoot`. The brooklyn.parameter defined on the parent entity + * isn't inherited, so when the child does `$brooklyn:config("custom.stabilizationDelay")` it + * doesn't find a default value - we get null. + */ static final String catalogYamlWithDslReferenceParentDefault = Joiner.on("\n").join( "brooklyn.catalog:", " id: my-app", @@ -101,14 +109,17 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { " type: " + Duration.class.getName(), " default: 1m", " services:", - " - type: " + TestEntity.class.getName(),//FailingEntity.class.getName(), + " - type: " + TestEntity.class.getName(), + " name: ignored", + " - type: " + TestEntity.class.getName(), + " name: targetEntity", " brooklyn.enrichers:", " - type: " + ServiceFailureDetector.class.getName(), " brooklyn.config:", - " serviceOnFire.stabilizationDelay: $brooklyn:config(\"custom.stabilizationDelay\")", - " entityFailed.stabilizationDelay: $brooklyn:config(\"custom.stabilizationDelay\")", - " entityRecovered.stabilizationDelay: $brooklyn:config(\"custom.stabilizationDelay\")", - " entityFailed.republishTime: $brooklyn:config(\"custom.republishTime\")"); + " serviceOnFire.stabilizationDelay: $brooklyn:scopeRoot().config(\"custom.stabilizationDelay\")", + " entityFailed.stabilizationDelay: $brooklyn:scopeRoot().config(\"custom.stabilizationDelay\")", + " entityRecovered.stabilizationDelay: $brooklyn:scopeRoot().config(\"custom.stabilizationDelay\")", + " entityFailed.republishTime: $brooklyn:scopeRoot().config(\"custom.republishTime\")"); @Test public void testDefaults() throws Exception { @@ -119,7 +130,7 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { public void testWithDslConfig() throws Exception { Entity app = runTest(catalogYamlWithDsl, appVersionedId); - TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + TestEntity newEntity = (TestEntity) Iterables.find(app.getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); ServiceFailureDetector newEnricher = assertHasEnricher(newEntity, ServiceFailureDetector.class); assertEnricherConfigMatchesDsl(newEnricher); } @@ -128,7 +139,7 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { public void testWithDslConfigReferenceParentDefault() throws Exception { Entity app = runTest(catalogYamlWithDslReferenceParentDefault, appVersionedId); - TestEntity newEntity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + TestEntity newEntity = (TestEntity) Iterables.find(app.getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); ServiceFailureDetector newEnricher = assertHasEnricher(newEntity, ServiceFailureDetector.class); assertEnricherConfigMatchesDsl(newEnricher); } @@ -140,7 +151,7 @@ public class ServiceFailureDetectorYamlTest extends AbstractYamlTest { "services:", "- type: " + appId); Entity app = createStartWaitAndLogApplication(appYaml); - TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + TestEntity entity = (TestEntity) Iterables.find(app.getChildren(), EntityPredicates.displayNameEqualTo("targetEntity")); assertHasEnricher(entity, ServiceFailureDetector.class); // Confirm ServiceFailureDetector triggers event http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a9ec838b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java index a150fd5..9966c95 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java @@ -150,29 +150,28 @@ public class BasicEntityMemento extends AbstractTreeNodeMemento implements Entit staticConfigKeys = getStaticConfigKeys(); staticSensorKeys = getStaticSensorKeys(); - if (configByKey!=null) { - configKeys = Maps.newLinkedHashMap(); - config = Maps.newLinkedHashMap(); - + configKeys = Maps.newLinkedHashMap(); + config = Maps.newLinkedHashMap(); + + if (allConfigKeys != null) { for (ConfigKey key : allConfigKeys) { - if (!key.equals(staticConfigKeys.get(key.getName()))) { + if (key != staticConfigKeys.get(key.getName())) { configKeys.put(key.getName(), key); } } + } + if (configByKey != null) { for (Map.Entry, Object> entry : configByKey.entrySet()) { ConfigKey key = entry.getKey(); - if (!configKeys.containsKey(key) && !key.equals(staticConfigKeys.get(key.getName()))) { + if (!configKeys.containsKey(key) && key != staticConfigKeys.get(key.getName())) { configKeys.put(key.getName(), key); } config.put(key.getName(), entry.getValue()); } - configKeys = toPersistedMap(configKeys); - config = toPersistedMap(config); } - if (configUnmatched!=null) { - if (config == null) config = Maps.newLinkedHashMap(); + + if (configUnmatched != null) { config.putAll(configUnmatched); - config = toPersistedMap(config); } if (attributesByKey!=null) { attributeKeys = Maps.newLinkedHashMap(); @@ -183,9 +182,12 @@ public class BasicEntityMemento extends AbstractTreeNodeMemento implements Entit attributeKeys.put(key.getName(), key); attributes.put(key.getName(), entry.getValue()); } - attributeKeys = toPersistedMap(attributeKeys); - attributes = toPersistedMap(attributes); } + + configKeys = toPersistedMap(configKeys); + config = toPersistedMap(config); + attributeKeys = toPersistedMap(attributeKeys); + attributes = toPersistedMap(attributes); } protected synchronized Map> getStaticConfigKeys() { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a9ec838b/policy/src/main/java/org/apache/brooklyn/policy/ha/ServiceFailureDetector.java ---------------------------------------------------------------------- diff --git a/policy/src/main/java/org/apache/brooklyn/policy/ha/ServiceFailureDetector.java b/policy/src/main/java/org/apache/brooklyn/policy/ha/ServiceFailureDetector.java index cba2abd..5f8ae3d 100644 --- a/policy/src/main/java/org/apache/brooklyn/policy/ha/ServiceFailureDetector.java +++ b/policy/src/main/java/org/apache/brooklyn/policy/ha/ServiceFailureDetector.java @@ -35,6 +35,7 @@ import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ComputeServic import org.apache.brooklyn.core.sensor.BasicNotificationSensor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.brooklyn.policy.autoscaling.AutoScalerPolicy; import org.apache.brooklyn.policy.ha.HASensors.FailureDescriptor; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -114,8 +115,11 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat * (we'll only publish again if we are in a different state from before Brooklyn was last * shutdown). */ - @SetFromFlag - protected LastPublished lastPublished = LastPublished.NONE; + public static final ConfigKey LAST_PUBLISHED = ConfigKeys.newConfigKey( + LastPublished.class, + "lastPublished", + "Indicates the last published event (entity 'failed', 'recovered', or none); used like an attribute (i.e. expect to be set on-the-fly)", + LastPublished.NONE); protected Long firstUpTime; @@ -139,6 +143,15 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat */ private final Object mutex = new Object(); + @Override + protected void doReconfigureConfig(ConfigKey key, T val) { + if (key.equals(LAST_PUBLISHED)) { + // find to modify this on-the-fly; no additional work required + } else { + super.doReconfigureConfig(key, val); + } + } + public ServiceFailureDetector() { this(new ConfigBag()); } @@ -172,7 +185,7 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat synchronized (mutex) { if (state.orNull() == Lifecycle.ON_FIRE) { - if (lastPublished == LastPublished.FAILED) { + if (config().get(LAST_PUBLISHED) == LastPublished.FAILED) { if (currentRecoveryStartTime != null) { if (LOG.isDebugEnabled()) LOG.debug("{} health-check for {}, component was recovering, now failing: {}", new Object[] {this, entity, getExplanation(state)}); currentRecoveryStartTime = null; @@ -198,7 +211,7 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat publishEntityRecoveredTime = null; } else if (state.orNull() == Lifecycle.RUNNING) { - if (lastPublished == LastPublished.FAILED) { + if (config().get(LAST_PUBLISHED) == LastPublished.FAILED) { if (currentRecoveryStartTime == null) { if (LOG.isDebugEnabled()) LOG.debug("{} health-check for {}, component now recovering: {}", new Object[] {this, entity, getExplanation(state)}); currentRecoveryStartTime = now; @@ -235,9 +248,8 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat publishEntityFailedTime = now + republishDelay.toMilliseconds(); recomputeIn = Math.min(recomputeIn, republishDelay.toMilliseconds()); } - lastPublished = LastPublished.FAILED; entity.sensors().emit(HASensors.ENTITY_FAILED, new HASensors.FailureDescriptor(entity, getFailureDescription(now))); - requestPersist(); + config().set(LAST_PUBLISHED, LastPublished.FAILED); } else { recomputeIn = Math.min(recomputeIn, delayBeforeCheck); } @@ -247,9 +259,8 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat if (LOG.isDebugEnabled()) LOG.debug("{} publishing recovered (state={}; currentRecoveryStartTime={}; now={}", new Object[] {this, state, Time.makeDateString(currentRecoveryStartTime), Time.makeDateString(now)}); publishEntityRecoveredTime = null; - lastPublished = LastPublished.RECOVERED; entity.sensors().emit(HASensors.ENTITY_RECOVERED, new HASensors.FailureDescriptor(entity, null)); - requestPersist(); + config().set(LAST_PUBLISHED, LastPublished.RECOVERED); } else { recomputeIn = Math.min(recomputeIn, delayBeforeCheck); } @@ -283,7 +294,7 @@ public class ServiceFailureDetector extends ServiceStateLogic.ComputeServiceStat "currentFailurePeriod=%s; currentRecoveryPeriod=%s", entity.getLocations(), (state.orNull() != null ? state : ""), - lastPublished, + config().get(LAST_PUBLISHED), Time.makeDateString(System.currentTimeMillis()), (currentFailureStartTime != null ? getTimeStringSince(currentFailureStartTime) : "") + " (stabilization "+Time.makeTimeStringRounded(serviceFailedStabilizationDelay) + ")", (currentRecoveryStartTime != null ? getTimeStringSince(currentRecoveryStartTime) : "") + " (stabilization "+Time.makeTimeStringRounded(serviceRecoveredStabilizationDelay) + ")");