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 2016/02/01 18:46:14 UTC
[11/50] brooklyn-library git commit: AbstractController server-pool:
incorporate comments
AbstractController server-pool: incorporate comments
- incorporate Peter's review comments from
https://github.com/brooklyncentral/brooklyn/pull/325
- and some other logging cleanup
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-library/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-library/commit/cb90d4d8
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-library/tree/cb90d4d8
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-library/diff/cb90d4d8
Branch: refs/heads/0.4.0
Commit: cb90d4d891cbfdab02926a1b12da46168346f325
Parents: 2b096d6
Author: Aled Sage <al...@gmail.com>
Authored: Thu Sep 27 13:15:08 2012 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Sep 27 13:15:08 2012 +0100
----------------------------------------------------------------------
.../entity/proxy/AbstractController.java | 45 ++++++++++++--------
1 file changed, 28 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/cb90d4d8/software/webapp/src/main/java/brooklyn/entity/proxy/AbstractController.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/brooklyn/entity/proxy/AbstractController.java b/software/webapp/src/main/java/brooklyn/entity/proxy/AbstractController.java
index c73a224..9477460 100644
--- a/software/webapp/src/main/java/brooklyn/entity/proxy/AbstractController.java
+++ b/software/webapp/src/main/java/brooklyn/entity/proxy/AbstractController.java
@@ -36,6 +36,13 @@ import com.google.common.collect.Sets;
* Represents a controller mechanism for a {@link Cluster}.
*/
public abstract class AbstractController extends SoftwareProcessEntity implements LoadBalancer {
+
+ // TODO Should review synchronization model. Currently, all changes to the serverPoolTargets
+ // (and checking for potential changes) is done while synchronized on this. That means it
+ // will also call update/reload while holding the lock. This is "conservative", but means
+ // sub-classes need to be extremely careful about any additional synchronization and of
+ // their implementations of update/reconfigureService/reload.
+
protected static final Logger LOG = LoggerFactory.getLogger(AbstractController.class);
/** sensor for port to forward to on target entities */
@@ -63,7 +70,7 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
public static final BasicAttributeSensor<String> ROOT_URL = WebAppService.ROOT_URL;
public static final BasicAttributeSensor<Set<String>> SERVER_POOL_TARGETS = new BasicAttributeSensor(
- Set.class, "proxy.targets", "Main set of downstream targets");
+ Set.class, "proxy.serverpool.targets", "The downstream targets in the server pool");
/**
* @deprecated Use SERVER_POOL_TARGETS
@@ -72,8 +79,8 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
public static final MethodEffector<Void> RELOAD = new MethodEffector(AbstractController.class, "reload");
- protected boolean isActive;
- protected boolean updateNeeded = true;
+ protected volatile boolean isActive;
+ protected volatile boolean updateNeeded = true;
protected AbstractMembershipTrackingPolicy serverPoolMemberTrackerPolicy;
protected Set<String> serverPoolAddresses = Sets.newLinkedHashSet();
@@ -210,14 +217,14 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
}
/**
- * Implementations should update the configuration so that 'addresses' are targeted.
+ * Implementations should update the configuration so that 'serverPoolAddresses' are targeted.
* The caller will subsequently call reload if reconfigureService returned true.
*
* @return True if the configuration has been modified (i.e. required reload); false otherwise.
*/
protected abstract boolean reconfigureService();
- public void update() {
+ public synchronized void update() {
if (!isActive()) updateNeeded = true;
else {
updateNeeded = false;
@@ -260,8 +267,9 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
setAttribute(SERVER_POOL_TARGETS, serverPoolAddresses);
}
- protected void onServerPoolMemberChanged(Entity member) {
- if (LOG.isTraceEnabled()) LOG.trace("Start {} checkEntity {}", this, member);
+ protected synchronized void onServerPoolMemberChanged(Entity member) {
+ if (LOG.isTraceEnabled()) LOG.trace("For {}, considering membership of {} which is in locations {}",
+ new Object[] {this, member, member.getLocations()});
if (belongsInServerPool(member)) {
addServerPoolMember(member);
} else {
@@ -272,21 +280,22 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
protected boolean belongsInServerPool(Entity member) {
if (!groovyTruth(member.getAttribute(Startable.SERVICE_UP))) {
- LOG.debug("Members of {}, checking {}, eliminating because not up", getDisplayName(), member.getDisplayName());
+ if (LOG.isTraceEnabled()) LOG.trace("Members of {}, checking {}, eliminating because not up", getDisplayName(), member.getDisplayName());
return false;
}
if (!getServerPool().getMembers().contains(member)) {
- LOG.debug("Members of {}, checking {}, eliminating because not member", getDisplayName(), member.getDisplayName());
+ if (LOG.isTraceEnabled()) LOG.trace("Members of {}, checking {}, eliminating because not member", getDisplayName(), member.getDisplayName());
return false;
}
- LOG.debug("Members of {}, checking {}, approving", getDisplayName(), member.getDisplayName());
+ if (LOG.isTraceEnabled()) LOG.trace("Members of {}, checking {}, approving", getDisplayName(), member.getDisplayName());
return true;
}
protected synchronized void addServerPoolMember(Entity member) {
- if (LOG.isTraceEnabled()) LOG.trace("Considering to add to {}, new member {} in locations {} - "+
- "waiting for service to be up", new Object[] {this, member, member.getLocations()});
- if (serverPoolTargets.contains(member)) return;
+ if (serverPoolTargets.contains(member)) {
+ if (LOG.isTraceEnabled()) LOG.trace("For {}, not adding as already have member {}", new Object[] {this, member});
+ return;
+ }
String address = getAddressOfEntity(member);
if (address != null) {
@@ -300,9 +309,10 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
}
protected synchronized void removeServerPoolMember(Entity member) {
- if (LOG.isTraceEnabled()) LOG.trace("Considering to remove from {}, member {} in locations {} - "+
- "waiting for service to be up", new Object[] {this, member, member.getLocations()});
- if (!serverPoolTargets.contains(member)) return;
+ if (!serverPoolTargets.contains(member)) {
+ if (LOG.isTraceEnabled()) LOG.trace("For {}, not removing as don't have member {}", new Object[] {this, member});
+ return;
+ }
String address = getAddressOfEntity(member);
if (address != null) {
@@ -321,7 +331,8 @@ public abstract class AbstractController extends SoftwareProcessEntity implement
if (ip!=null && port!=null) {
return ip+":"+port;
}
- LOG.error("Unable to construct hostname:port representation for "+member+"; skipping in "+this);
+ LOG.error("Unable to construct hostname:port representation for {} ({}:{}); skipping in {}",
+ new Object[] {member, ip, port, this});
return null;
}
}