brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [02/13] brooklyn-library git commit: notes and logging for intermittent failure BROOKLYN-206
Date Mon, 07 Mar 2016 09:32:32 GMT
notes and logging for intermittent failure BROOKLYN-206


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-library/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-library/commit/dddc28e6
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-library/tree/dddc28e6
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-library/diff/dddc28e6

Branch: refs/heads/master
Commit: dddc28e6ee8c55f2d201bc2be005399d743db46a
Parents: 6fe1f88
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Wed Feb 24 14:13:07 2016 -0800
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Wed Feb 24 14:13:07 2016 -0800

----------------------------------------------------------------------
 .../entity/proxy/AbstractControllerImpl.java    | 33 +++++++++++---------
 .../AbstractNonProvisionedControllerImpl.java   | 10 +++---
 .../entity/proxy/AbstractControllerTest.java    | 31 +++++++++++++-----
 .../proxy/TrackingAbstractControllerImpl.java   |  3 +-
 4 files changed, 49 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/dddc28e6/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
index 204ec54..5c4c443 100644
--- a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
+++ b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
@@ -44,13 +44,13 @@ import org.apache.brooklyn.core.location.access.BrooklynAccessUtils;
 import org.apache.brooklyn.entity.group.AbstractMembershipTrackingPolicy;
 import org.apache.brooklyn.entity.group.Cluster;
 import org.apache.brooklyn.entity.software.base.SoftwareProcessImpl;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.text.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Objects;
 import com.google.common.base.Predicates;
@@ -142,20 +142,25 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl
impleme
 
         LOG.info("Added policy {} to {}", serverPoolMemberTrackerPolicy, this);
         
-        // Initialize ourselves immediately with the latest set of members; don't wait for
-        // listener notifications because then will be out-of-date for short period (causing

-        // problems for rebind)
-        Map<Entity,String> serverPoolTargets = Maps.newLinkedHashMap();
-        for (Entity member : getServerPool().getMembers()) {
-            if (belongsInServerPool(member)) {
-                if (LOG.isTraceEnabled()) LOG.trace("Done {} checkEntity {}", this, member);
-                String address = getAddressOfEntity(member);
-                serverPoolTargets.put(member, address);
+        synchronized (serverPoolAddresses) {
+            // Initialize ourselves immediately with the latest set of members; don't wait
for
+            // listener notifications because then will be out-of-date for short period (causing

+            // problems for rebind)
+            // if invoked on start, we'll have isActive=false at this point so other policies
wont' run anyway,
+            // but synch in case invoked at other times; and note if !isActive during start
means we miss some after this,
+            // we will update again on postStart after setting isActive=true
+            Map<Entity,String> serverPoolTargets = Maps.newLinkedHashMap();
+            for (Entity member : getServerPool().getMembers()) {
+                if (belongsInServerPool(member)) {
+                    if (LOG.isTraceEnabled()) LOG.trace("Done {} checkEntity {}", this, member);
+                    String address = getAddressOfEntity(member);
+                    serverPoolTargets.put(member, address);
+                }
             }
-        }
 
-        LOG.info("Resetting {}, server pool targets {}", new Object[] {this, serverPoolTargets});
-        sensors().set(SERVER_POOL_TARGETS, serverPoolTargets);
+            LOG.info("Resetting {}, server pool targets {}", new Object[] {this, serverPoolTargets});
+            sensors().set(SERVER_POOL_TARGETS, serverPoolTargets);
+        }
     }
     
     protected void removeServerPoolMemberTrackingPolicy() {

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/dddc28e6/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractNonProvisionedControllerImpl.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractNonProvisionedControllerImpl.java
b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractNonProvisionedControllerImpl.java
index 7ad111a..137be36 100644
--- a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractNonProvisionedControllerImpl.java
+++ b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractNonProvisionedControllerImpl.java
@@ -57,19 +57,19 @@ public abstract class AbstractNonProvisionedControllerImpl extends AbstractEntit
     public AbstractNonProvisionedControllerImpl() {
         this(MutableMap.of(), null, null);
     }
-    public AbstractNonProvisionedControllerImpl(Map properties) {
+    public AbstractNonProvisionedControllerImpl(Map<?,?> properties) {
         this(properties, null, null);
     }
     public AbstractNonProvisionedControllerImpl(Entity parent) {
         this(MutableMap.of(), parent, null);
     }
-    public AbstractNonProvisionedControllerImpl(Map properties, Entity parent) {
+    public AbstractNonProvisionedControllerImpl(Map<?,?> properties, Entity parent)
{
         this(properties, parent, null);
     }
     public AbstractNonProvisionedControllerImpl(Entity parent, Cluster cluster) {
         this(MutableMap.of(), parent, cluster);
     }
-    public AbstractNonProvisionedControllerImpl(Map properties, Entity parent, Cluster cluster)
{
+    public AbstractNonProvisionedControllerImpl(Map<?,?> properties, Entity parent,
Cluster cluster) {
     }
 
     public static class MemberTrackingPolicy extends AbstractMembershipTrackingPolicy {
@@ -106,8 +106,8 @@ public abstract class AbstractNonProvisionedControllerImpl extends AbstractEntit
     
     protected void preStart() {
         AttributeSensor<?> hostAndPortSensor = getConfig(HOST_AND_PORT_SENSOR);
-        Maybe<Object> hostnameSensor = getConfigRaw(HOSTNAME_SENSOR, true);
-        Maybe<Object> portSensor = getConfigRaw(PORT_NUMBER_SENSOR, true);
+        Maybe<Object> hostnameSensor = config().getRaw(HOSTNAME_SENSOR);
+        Maybe<Object> portSensor = config().getRaw(PORT_NUMBER_SENSOR);
         if (hostAndPortSensor != null) {
             checkState(!hostnameSensor.isPresent() && !portSensor.isPresent(), 
                     "Must not set %s and either of %s or %s", HOST_AND_PORT_SENSOR, HOSTNAME_SENSOR,
PORT_NUMBER_SENSOR);

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/dddc28e6/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
index 7e98d41..6daae33 100644
--- a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
+++ b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
@@ -39,7 +39,6 @@ import org.apache.brooklyn.api.location.MachineProvisioningLocation;
 import org.apache.brooklyn.api.location.NoMachinesAvailableException;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.core.entity.Attributes;
-import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.factory.EntityFactory;
 import org.apache.brooklyn.core.entity.trait.Startable;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
@@ -47,6 +46,8 @@ import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.core.test.entity.TestEntityImpl;
 import org.apache.brooklyn.entity.group.Cluster;
 import org.apache.brooklyn.entity.group.DynamicCluster;
+import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -56,8 +57,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
-import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
-import org.apache.brooklyn.location.ssh.SshMachineLocation;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -103,29 +102,47 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
     // values change.
     @Test
     public void testUpdateCalledWhenChildHostnameAndPortChanges() throws Exception {
+        log.info("adding child (no effect until up)");
         TestEntity child = cluster.addChild(EntitySpec.create(TestEntity.class));
         cluster.addMember(child);
 
         List<Collection<String>> u = Lists.newArrayList(controller.getUpdates());
         assertTrue(u.isEmpty(), "expected no updates, but got "+u);
         
+        log.info("setting child service_up");
         child.sensors().set(Startable.SERVICE_UP, true);
+        // above may trigger error logged about no hostname, but should update again with
the settings below
         
-        // TODO Ugly sleep to allow AbstractController to detect node having been added
-        Thread.sleep(100);
-        
+        log.info("setting mymachine:1234");
         child.sensors().set(ClusteredEntity.HOSTNAME, "mymachine");
         child.sensors().set(Attributes.SUBNET_HOSTNAME, "mymachine");
         child.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
         assertEventuallyExplicitAddressesMatch(ImmutableList.of("mymachine:1234"));
         
+        /* a race failure has been observed, https://issues.apache.org/jira/browse/BROOKLYN-206
+         * but now (two months later) i (alex) can't see how it could happen. 
+         * probably optimistic but maybe it is fixed. if not we'll need the debug logs to
see what is happening.
+         * i've confirmed:
+         * * the policy is attached and active during setup, before start completes
+         * * the child is added as a member synchronously
+         * * the policy which is "subscribed to members" is in fact subscribed to everything
+         *   then filtered for members, not ideal, but there shouldn't be a race in the policy
getting notices
+         * * the handling of those events are both processed in order and look up the current
values
+         *   rather than relying on the published values; either should be sufficient to
cause the addresses to change
+         * there was a sleep(100) marked "Ugly sleep to allow AbstractController to detect
node having been added"
+         * from the test's addition by aled in early 2014, but can't see why that would be
necessary
+         */
+        
+        log.info("setting mymachine2:1234");
         child.sensors().set(ClusteredEntity.HOSTNAME, "mymachine2");
         child.sensors().set(Attributes.SUBNET_HOSTNAME, "mymachine2");
         assertEventuallyExplicitAddressesMatch(ImmutableList.of("mymachine2:1234"));
         
+        log.info("setting mymachine2:1235");
         child.sensors().set(ClusteredEntity.HTTP_PORT, 1235);
         assertEventuallyExplicitAddressesMatch(ImmutableList.of("mymachine2:1235"));
         
+        log.info("clearing");
         child.sensors().set(ClusteredEntity.HOSTNAME, null);
         child.sensors().set(Attributes.SUBNET_HOSTNAME, null);
         assertEventuallyExplicitAddressesMatch(ImmutableList.<String>of());
@@ -305,7 +322,7 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
         List<Collection<String>> u = Lists.newArrayList(controller.getUpdates());
         Collection<String> last = Iterables.getLast(u, null);
         log.debug("test "+u.size()+" updates, expecting "+expectedAddresses+"; actual "+last);
-        assertTrue(u.size() > 0);
+        assertTrue(!u.isEmpty(), "no updates; expecting "+expectedAddresses);
         assertEquals(ImmutableSet.copyOf(last), ImmutableSet.copyOf(expectedAddresses), "actual="+last+"
expected="+expectedAddresses);
         assertEquals(last.size(), expectedAddresses.size(), "actual="+last+" expected="+expectedAddresses);
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/dddc28e6/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/TrackingAbstractControllerImpl.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/TrackingAbstractControllerImpl.java
b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/TrackingAbstractControllerImpl.java
index f0b165d..9dfd66d 100644
--- a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/TrackingAbstractControllerImpl.java
+++ b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/TrackingAbstractControllerImpl.java
@@ -22,7 +22,6 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 
-import org.apache.brooklyn.entity.proxy.AbstractControllerImpl;
 import org.apache.brooklyn.entity.software.base.test.driver.MockSshDriver;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -56,7 +55,7 @@ public class TrackingAbstractControllerImpl extends AbstractControllerImpl
imple
     }
 
     @Override
-    public Class getDriverInterface() {
+    public Class<?> getDriverInterface() {
         return MockSshDriver.class;
     }
     


Mime
View raw message