brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [08/15] git commit: support programmatic API on HighAvailabilityManager to changeMode and setPriority; priority persisted and used to elect master (also prefers non-snapshot and more recent Brooklyn versions by default, unless explicit priority overrides
Date Wed, 01 Oct 2014 18:03:08 GMT
support programmatic API on HighAvailabilityManager to changeMode and setPriority;
priority persisted and used to elect master (also prefers non-snapshot and more recent Brooklyn versions by default, unless explicit priority overrides).
good test for changing mode in HotStandbyTest.testChangeMode.


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

Branch: refs/heads/master
Commit: 32c926b95c8e89538778cd85010fa2e9a20ecf74
Parents: c3aac5e
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Wed Oct 1 11:32:26 2014 +0100
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Wed Oct 1 16:40:16 2014 +0100

----------------------------------------------------------------------
 .../management/ha/HighAvailabilityManager.java  |  13 ++
 .../management/ha/ManagementNodeSyncRecord.java |   2 +
 .../ha/ManagementPlaneSyncRecordPersister.java  |   1 +
 .../rebind/PeriodicDeltaChangeListener.java     |   5 +-
 .../entity/rebind/RebindManagerImpl.java        |  12 +-
 .../dto/BasicManagementNodeSyncRecord.java      |  22 ++-
 .../management/ha/BasicMasterChooser.java       |  64 ++++++--
 .../ha/HighAvailabilityManagerImpl.java         | 148 +++++++++++++++----
 .../ha/ManagementPlaneSyncRecordDeltaImpl.java  |  11 +-
 ...ntPlaneSyncRecordPersisterToObjectStore.java |  15 +-
 .../NonDeploymentManagementContext.java         |  13 +-
 .../HighAvailabilityManagerSplitBrainTest.java  |   8 +-
 .../brooklyn/management/ha/HotStandbyTest.java  |  78 ++++++++++
 .../management/ha/MasterChooserTest.java        |  36 ++++-
 ...lynLauncherRebindToCloudObjectStoreTest.java |   4 -
 .../util/text/NaturalOrderComparator.java       |   3 +-
 16 files changed, 373 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java b/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java
index d9888f9..72bb70f 100644
--- a/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java
+++ b/api/src/main/java/brooklyn/management/ha/HighAvailabilityManager.java
@@ -61,6 +61,7 @@ public interface HighAvailabilityManager {
      * instead of {@link #start(HighAvailabilityMode)}. It may be an error if
      * this is called after this HA Manager is started.
      */
+    @Beta
     void disabled();
 
     /** Whether HA mode is operational */
@@ -83,6 +84,18 @@ public interface HighAvailabilityManager {
      */
     void stop();
 
+    /** changes the mode that this HA server is running in
+     * <p>
+     * note it will be an error to {@link #changeMode(HighAvailabilityMode)} to {@link ManagementNodeState#MASTER} 
+     * when there is already a master; to promote a node explicitly set its priority higher than
+     * the others and invoke {@link #changeMode(HighAvailabilityMode)} to a standby mode on the existing master */
+    void changeMode(HighAvailabilityMode mode);
+
+    /** sets the priority, and publishes it synchronously so it is canonical */
+    void setPriority(long priority);
+    
+    long getPriority();
+    
     /**
      * Returns a snapshot of the management-plane's current / most-recently-known status.
      * <p>

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/api/src/main/java/brooklyn/management/ha/ManagementNodeSyncRecord.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/management/ha/ManagementNodeSyncRecord.java b/api/src/main/java/brooklyn/management/ha/ManagementNodeSyncRecord.java
index bbd8665..38150bf 100644
--- a/api/src/main/java/brooklyn/management/ha/ManagementNodeSyncRecord.java
+++ b/api/src/main/java/brooklyn/management/ha/ManagementNodeSyncRecord.java
@@ -46,6 +46,8 @@ public interface ManagementNodeSyncRecord {
     
     ManagementNodeState getStatus();
 
+    Long getPriority();
+    
     /** timestamp set by the originating management machine */
     long getLocalTimestamp();
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/api/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersister.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersister.java b/api/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersister.java
index ebacd7a..f2d42fc 100644
--- a/api/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersister.java
+++ b/api/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersister.java
@@ -63,5 +63,6 @@ public interface ManagementPlaneSyncRecordPersister {
         Collection<String> getRemovedNodeIds();
         MasterChange getMasterChange();
         String getNewMasterOrNull();
+        String getExpectedMasterToClear();
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
index acf9de7..48da586 100644
--- a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
+++ b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
@@ -144,6 +144,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
             LOG.warn("Request to start "+this+" when already running - "+scheduledTask+"; ignoring");
             return;
         }
+        stopped = false;
         running = true;
         
         Callable<Task<?>> taskFactory = new Callable<Task<?>>() {
@@ -298,7 +299,9 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     
     @VisibleForTesting
     public void persistNow() {
-        if (!isActive()) return;
+        if (!isActive()) {
+            return;
+        }
         try {
             persistingMutex.acquire();
             if (!isActive()) return;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/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 2f85db6..745b7cb 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -206,6 +206,14 @@ public class RebindManagerImpl implements RebindManager {
         setPeriodicPersistPeriod(Duration.of(periodMillis, TimeUnit.MILLISECONDS));
     }
 
+    public boolean isPersistenceRunning() {
+        return persistenceRunning;
+    }
+    
+    public boolean isReadOnlyRunning() {
+        return readOnlyRunning;
+    }
+    
     @Override
     public void setPersister(BrooklynMementoPersister val) {
         PersistenceExceptionHandler exceptionHandler = PersistenceExceptionHandlerImpl.builder()
@@ -905,9 +913,9 @@ public class RebindManagerImpl implements RebindManager {
             if (wasReadOnly && isNowReadOnly)
                 return ManagementTransitionMode.REBINDING_READONLY;
             else if (wasReadOnly)
-                return ManagementTransitionMode.REBINDING_NO_LONGER_PRIMARY;
-            else if (isNowReadOnly)
                 return ManagementTransitionMode.REBINDING_BECOMING_PRIMARY;
+            else if (isNowReadOnly)
+                return ManagementTransitionMode.REBINDING_NO_LONGER_PRIMARY;
             else
                 throw new IllegalStateException("Rebinding master not supported: "+item);
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/main/java/brooklyn/entity/rebind/plane/dto/BasicManagementNodeSyncRecord.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/plane/dto/BasicManagementNodeSyncRecord.java b/core/src/main/java/brooklyn/entity/rebind/plane/dto/BasicManagementNodeSyncRecord.java
index c7aa008..bc1e137 100644
--- a/core/src/main/java/brooklyn/entity/rebind/plane/dto/BasicManagementNodeSyncRecord.java
+++ b/core/src/main/java/brooklyn/entity/rebind/plane/dto/BasicManagementNodeSyncRecord.java
@@ -32,7 +32,8 @@ import brooklyn.util.time.Time;
 import com.google.common.base.Objects;
 
 /**
- * Represents the state of a management node within the Brooklyn management plane.
+ * Represents the state of a management node within the Brooklyn management plane
+ * (DTO class).
  * 
  * @author aled
  */
@@ -50,6 +51,7 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
         protected String nodeId;
         protected URI uri;
         protected ManagementNodeState status;
+        protected Long priority;
         protected long localTimestamp;
         protected Long remoteTimestamp;
 
@@ -68,6 +70,9 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
         public Builder status(ManagementNodeState val) {
             status = val; return self();
         }
+        public Builder priority(Long val) {
+            priority = val; return self();
+        }
         public Builder localTimestamp(long val) {
             localTimestamp = val; return self();
         }
@@ -79,12 +84,13 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
         }
         public Builder from(ManagementNodeSyncRecord other, boolean ignoreNulls) {
             if (ignoreNulls && other==null) return this;
-            if (other.getNodeId()!=null) nodeId = other.getNodeId();
             if (other.getBrooklynVersion()!=null) brooklynVersion = other.getBrooklynVersion();
-            if (other.getLocalTimestamp()>0) localTimestamp = other.getLocalTimestamp();
-            if (other.getRemoteTimestamp()!=null) remoteTimestamp = other.getRemoteTimestamp();
+            if (other.getNodeId()!=null) nodeId = other.getNodeId();
             if (other.getUri()!=null) uri = other.getUri();
             if (other.getStatus()!=null) status = other.getStatus();
+            if (other.getPriority()!=null) priority = other.getPriority();
+            if (other.getLocalTimestamp()>0) localTimestamp = other.getLocalTimestamp();
+            if (other.getRemoteTimestamp()!=null) remoteTimestamp = other.getRemoteTimestamp();
             return this;
         }
         public ManagementNodeSyncRecord build() {
@@ -96,6 +102,7 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
     private String nodeId;
     private URI uri;
     private ManagementNodeState status;
+    private Long priority;
     private Long localTimestamp;
     private Long remoteTimestamp;
     
@@ -116,6 +123,7 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
         nodeId = builder.nodeId;
         uri = builder.uri;
         status = builder.status;
+        priority = builder.priority;
         localTimestamp = builder.localTimestamp;
         remoteTimestamp = builder.remoteTimestamp;
     }
@@ -141,6 +149,11 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
     }
     
     @Override
+    public Long getPriority() {
+        return priority;
+    }
+    
+    @Override
     public long getLocalTimestamp() {
         if (localTimestamp!=null) return localTimestamp;
         if (timestampUtc!=null) return timestampUtc;
@@ -167,6 +180,7 @@ public class BasicManagementNodeSyncRecord implements ManagementNodeSyncRecord,
                 .add("nodeId", getNodeId())
                 .add("uri", getUri())
                 .add("status", getStatus())
+                .add("priority", getPriority())
                 .add("localTimestamp", getLocalTimestamp()+"="+Time.makeDateString(getLocalTimestamp()))
                 .add("remoteTimestamp", getRemoteTimestamp()+(getRemoteTimestamp()==null ? "" : 
                     "="+Time.makeDateString(getRemoteTimestamp())))

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/main/java/brooklyn/management/ha/BasicMasterChooser.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/BasicMasterChooser.java b/core/src/main/java/brooklyn/management/ha/BasicMasterChooser.java
index 78c110a..80f40d0 100644
--- a/core/src/main/java/brooklyn/management/ha/BasicMasterChooser.java
+++ b/core/src/main/java/brooklyn/management/ha/BasicMasterChooser.java
@@ -27,10 +27,13 @@ import org.slf4j.LoggerFactory;
 
 import brooklyn.entity.trait.Identifiable;
 import brooklyn.util.collections.MutableList;
+import brooklyn.util.text.NaturalOrderComparator;
 import brooklyn.util.time.Duration;
 
 import com.google.common.annotations.Beta;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.Ordering;
 
 /**
  * @since 0.7.0
@@ -92,19 +95,58 @@ public abstract class BasicMasterChooser implements MasterChooser {
         return min.record;
     }
 
+    public static class AlphabeticChooserScore implements Comparable<AlphabeticChooserScore> {
+        long priority;
+        int versionBias;
+        String brooklynVersion;
+        int statePriority;
+        String nodeId;
+        
+        @Override
+        public int compareTo(AlphabeticChooserScore o) {
+            if (o==null) return -1;
+            return ComparisonChain.start()
+                // invert the order where we prefer higher values
+                .compare(o.priority, this.priority)
+                .compare(o.versionBias, this.versionBias)
+                .compare(o.brooklynVersion, this.brooklynVersion, 
+                    Ordering.from(NaturalOrderComparator.INSTANCE).nullsFirst())
+                .compare(o.statePriority, this.statePriority)
+                .compare(this.nodeId, o.nodeId, Ordering.usingToString().nullsLast())
+                .result();
+        }
+    }
+    
+    /** comparator which prefers, in order:
+     * <li> higher explicit priority
+     * <li> non-snapshot Brooklyn version, then any Brooklyn version, and lastly null version 
+     *      (using {@link NaturalOrderComparator} so e.g. v10 > v3.20 > v3.9 )
+     * <li> higher version (null last)
+     * <li> node which reports it's master, hot standby, then standby 
+     * <li> finally (common case): lower (alphabetically) node id
+     */
     public static class AlphabeticMasterChooser extends BasicMasterChooser {
-        final boolean preferHot;
-        public AlphabeticMasterChooser(boolean preferHot) { this.preferHot = preferHot; }
-        public AlphabeticMasterChooser() { this.preferHot = true; }
+        final boolean preferHotStandby;
+        public AlphabeticMasterChooser(boolean preferHotStandby) { this.preferHotStandby = preferHotStandby; }
+        public AlphabeticMasterChooser() { this.preferHotStandby = true; }
         @Override
-        protected String score(ManagementNodeSyncRecord contender) {
-            if (!preferHot)
-                return contender.getNodeId();
-            // simple prefix with the rating
-            String state = (contender.getStatus()==ManagementNodeState.MASTER ? "1" :
-                contender.getStatus()==ManagementNodeState.HOT_STANDBY ? "2" :
-                contender.getStatus()==ManagementNodeState.STANDBY ? "3" : "9");
-            return state + ":" + contender.getNodeId();
+        protected AlphabeticChooserScore score(ManagementNodeSyncRecord contender) {
+            AlphabeticChooserScore score = new AlphabeticChooserScore();
+            score.priority = contender.getPriority()!=null ? contender.getPriority() : 0;
+            score.brooklynVersion = contender.getBrooklynVersion();
+            score.versionBias = contender.getBrooklynVersion()==null ? -2 :
+                contender.getBrooklynVersion().toLowerCase().indexOf("snapshot")>=0 ? -1 :
+                0;
+            if (preferHotStandby) {
+                // other master should be preferred before we get invoked, but including for good measure
+                score.statePriority = contender.getStatus()==ManagementNodeState.MASTER ? 3 :
+                    contender.getStatus()==ManagementNodeState.HOT_STANDBY ? 2 :
+                        contender.getStatus()==ManagementNodeState.STANDBY ? 1 : -1;
+            } else {
+                score.statePriority = 0;
+            }
+            score.nodeId = contender.getNodeId();
+            return score;
         }
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/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 bdd6eba..200a902 100644
--- a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
+++ b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
@@ -50,6 +50,7 @@ import brooklyn.util.collections.MutableSet;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.task.BasicTask;
 import brooklyn.util.task.ScheduledTask;
+import brooklyn.util.text.Strings;
 import brooklyn.util.time.Duration;
 
 import com.google.api.client.repackaged.com.google.common.base.Preconditions;
@@ -128,6 +129,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
     private volatile boolean running;
     private volatile ManagementNodeState nodeState = ManagementNodeState.INITIALIZING;
     private volatile boolean nodeStateTransitionComplete = false;
+    private volatile long priority = 0;
 
     public HighAvailabilityManagerImpl(ManagementContextInternal managementContext) {
         this.managementContext = managementContext;
@@ -204,54 +206,112 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
 
     @Override
     public void start(HighAvailabilityMode startMode) {
-        ownNodeId = managementContext.getManagementNodeId();
         nodeStateTransitionComplete = true;
         // always start in standby; it may get promoted to master or hot_standby in this method
+        // (depending on startMode; but for startMode STANDBY or HOT_STANDBY it will not promote until the next election)
         nodeState = ManagementNodeState.STANDBY;
         running = true;
+        changeMode(startMode, true, true);
+    }
+    
+    @Override
+    public void changeMode(HighAvailabilityMode startMode) {
+        changeMode(startMode, false, false);
+    }
+    
+    @VisibleForTesting
+    @Beta
+    public void changeMode(HighAvailabilityMode startMode, boolean preventElectionOnExplicitStandbyMode, boolean failOnExplicitStandbyModeIfNoMaster) {
+        if (!running)
+            throw new IllegalStateException("Can only change mode when already running; invoke 'start' first");
         
+        ownNodeId = managementContext.getManagementNodeId();
         // TODO Small race in that we first check, and then we'll do checkMaster() on first poll,
         // so another node could have already become master or terminated in that window.
         ManagementNodeSyncRecord existingMaster = hasHealthyMaster();
+        boolean weAreMaster = existingMaster!=null && ownNodeId.equals(existingMaster.getNodeId());
         
         // catch error in some tests where mgmt context has a different mgmt context
         if (managementContext.getHighAvailabilityManager()!=this)
             throw new IllegalStateException("Cannot start an HA manager on a management context with a different HA manager!");
         
+        if (weAreMaster) {
+            // demotion may be required; do this before triggering an election
+            switch (startMode) {
+            case MASTER:
+            case AUTO:
+                // no action needed
+                break;
+            case HOT_STANDBY: demoteToStandby(true); break;
+            case STANDBY: demoteToStandby(false); break;
+            case DISABLED: demoteToFailed(); break;
+            default:
+                throw new IllegalStateException("Unexpected high availability mode "+startMode+" requested for "+this);
+            }
+        }
+        
+        // now do election
         switch (startMode) {
         case AUTO:
             // don't care; let's start and see if we promote ourselves
             publishAndCheck(true);
             if (nodeState == ManagementNodeState.STANDBY || nodeState == ManagementNodeState.HOT_STANDBY) {
-                String masterNodeId = getManagementPlaneSyncState().getMasterNodeId();
-                ManagementNodeSyncRecord masterNodeDetails = getManagementPlaneSyncState().getManagementNodes().get(masterNodeId);
-                LOG.info("Management node "+ownNodeId+" started as HA " + nodeState + " autodetected, master is "+masterNodeId +
-                    (masterNodeDetails==null || masterNodeDetails.getUri()==null ? " (no url)" : " at "+masterNodeDetails.getUri()));
+                ManagementPlaneSyncRecord newState = getManagementPlaneSyncState();
+                String masterNodeId = newState.getMasterNodeId();
+                ManagementNodeSyncRecord masterNodeDetails = newState.getManagementNodes().get(masterNodeId);
+                LOG.info("Management node "+ownNodeId+" running as HA " + nodeState + " autodetected, " +
+                    (Strings.isBlank(masterNodeId) ? "no master currently (other node should promote itself soon)" : "master "
+                        + (existingMaster==null ? "(new) " : "")
+                        + "is "+masterNodeId +
+                        (masterNodeDetails==null || masterNodeDetails.getUri()==null ? " (no url)" : " at "+masterNodeDetails.getUri())));
             } else if (nodeState == ManagementNodeState.MASTER) {
-                LOG.info("Management node "+ownNodeId+" starting as HA MASTER autodetected");
+                LOG.info("Management node "+ownNodeId+" running as HA MASTER autodetected");
             } else {
-                throw new IllegalStateException("Management node "+ownNodeId+" started as auto in unexpected mode "+nodeState);
+                throw new IllegalStateException("Management node "+ownNodeId+" set to HA AUTO, encountered unexpected mode "+nodeState);
             }
             break;
         case MASTER:
             if (existingMaster == null) {
                 promoteToMaster();
-                LOG.info("Management node "+ownNodeId+" started as HA MASTER explicitly");
+                LOG.info("Management node "+ownNodeId+" running as HA MASTER explicitly");
+            } else if (!weAreMaster) {
+                throw new IllegalStateException("Master already exists; cannot run as master (master "+existingMaster.toVerboseString()+"); "
+                    + "to trigger a promotion, set a priority and demote the current master");
             } else {
-                throw new IllegalStateException("Master already exists; cannot start as master ("+existingMaster.toVerboseString()+")");
+                LOG.info("Management node "+ownNodeId+" already running as HA MASTER, when set explicitly");
             }
             break;
         case STANDBY:
         case HOT_STANDBY:
-            if (existingMaster != null) {
+            if (!preventElectionOnExplicitStandbyMode)
                 publishAndCheck(true);
-                LOG.info("Management node "+ownNodeId+" started as "+startMode+" explicitly");
+            if (failOnExplicitStandbyModeIfNoMaster && existingMaster==null) {
+                LOG.error("Management node "+ownNodeId+" detected no master when "+startMode+" requested and existing master required; failing.");
+                throw new IllegalStateException("No existing master; cannot start as "+startMode);
+            }
+            
+            String message = "Management node "+ownNodeId+" running as HA "+getNodeState()+" (";
+            if (getNodeState().toString().equals(startMode.toString()))
+                message += "explicitly requested";
+            else if (startMode==HighAvailabilityMode.HOT_STANDBY && getNodeState()==ManagementNodeState.STANDBY)
+                message += "caller requested "+startMode+", will attempt rebind directly";
+            else
+                message += "caller requested "+startMode;
+            
+            if (getNodeState()==ManagementNodeState.MASTER) {
+                message += " but election re-promoted this node)";
             } else {
-                throw new IllegalStateException("No existing master; cannot start as standby");
+                ManagementPlaneSyncRecord newState = getManagementPlaneSyncState();
+                if (Strings.isBlank(newState.getMasterNodeId())) {
+                    message += "); no master currently (subsequent election may repair)";
+                } else {
+                    message += "); master "+newState.getMasterNodeId();
+                }
             }
+            LOG.info(message);
             break;
         default:
-            throw new IllegalStateException("Unexpected high availability start-mode "+startMode+" for "+this);
+            throw new IllegalStateException("Unexpected high availability mode "+startMode+" requested for "+this);
         }
         
         if (startMode==HighAvailabilityMode.AUTO) {
@@ -266,15 +326,40 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
             nodeStateTransitionComplete = false;
             // inform the world that we are transitioning (not eligible for promotion while going in to hot standby)
             publishHealth();
-            attemptHotStandby();
-            publishHealth();
+            try {
+                attemptHotStandby();
+                nodeStateTransitionComplete = true;
+                publishHealth();
+                
+                if (getNodeState()==ManagementNodeState.HOT_STANDBY) {
+                    LOG.info("Management node "+ownNodeId+" now running as HA "+ManagementNodeState.HOT_STANDBY+"; "
+                        + managementContext.getApplications().size()+" application"+Strings.s(managementContext.getApplications().size())+" loaded");
+                } else {
+                    LOG.warn("Management node "+ownNodeId+" unable to promote to "+ManagementNodeState.HOT_STANDBY+" (currently "+getNodeState()+"); "
+                        + "(see log for further details)");
+                }
+            } catch (Exception e) {
+                LOG.warn("Management node "+ownNodeId+" unable to promote to "+ManagementNodeState.HOT_STANDBY+" (currently "+getNodeState()+"); rethrowing: "+Exceptions.collapseText(e));
+                throw Exceptions.propagate(e);
+            }
+        } else {
+            nodeStateTransitionComplete = true;
         }
-        
-        nodeStateTransitionComplete = true;
         registerPollTask();
     }
 
     @Override
+    public void setPriority(long priority) {
+        this.priority = priority;
+        if (persister!=null) publishHealth();
+    }
+    
+    @Override
+    public long getPriority() {
+        return priority;
+    }
+    
+    @Override
     public void stop() {
         LOG.debug("Stopping "+this);
         boolean wasRunning = running; // ensure idempotent
@@ -373,7 +458,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
         if (LOG.isTraceEnabled()) LOG.trace("Published management-node health: {}", memento);
     }
     
-    protected synchronized void publishDemotionFromMaster(boolean clearMaster) {
+    protected synchronized void publishDemotionFromMaster() {
         checkState(getNodeState() != ManagementNodeState.MASTER, "node status must not be master when demoting", getNodeState());
         
         if (persister == null) {
@@ -384,7 +469,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
         ManagementNodeSyncRecord memento = createManagementNodeSyncRecord(false);
         ManagementPlaneSyncRecordDeltaImpl.Builder deltaBuilder = ManagementPlaneSyncRecordDeltaImpl.builder()
                 .node(memento);
-        if (clearMaster) deltaBuilder.clearMaster(ownNodeId);
+        deltaBuilder.clearMaster(ownNodeId);
         
         Delta delta = deltaBuilder.build();
         persister.delta(delta);
@@ -466,7 +551,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
                 return;
             } else {
                 if (ownNodeRecord!=null && ownNodeRecord.getStatus() == ManagementNodeState.MASTER) {
-                    LOG.error("HA subsystem detected change of master, stolen from us ("+ownNodeId+"), deferring to "+currMasterNodeId);
+                    LOG.error("Management node "+ownNodeId+" detected master change, stolen from us, deferring to "+currMasterNodeId);
                     newMasterNodeRecord = currMasterNodeRecord;
                     demotingSelfInFavourOfOtherMaster = true;
                 } else {
@@ -494,7 +579,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
         
         if (demotingSelfInFavourOfOtherMaster) {
             LOG.debug("Master-change for this node only, demoting "+ownNodeRecord.toVerboseString()+" in favour of official master "+newMasterNodeRecord.toVerboseString());
-            demoteToStandby(true);
+            demoteToStandby(BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_DEFAULT_STANDBY_IS_HOT_PROPERTY));
             return;
         }
         
@@ -516,13 +601,16 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
                 });
         }
         if (!initializing) {
-            LOG.warn("HA subsystem detected change of master, from " 
-                + currMasterNodeId + " (" + (currMasterNodeRecord==null ? "?" : currMasterNodeRecord.getRemoteTimestamp()) + ")"
+            String message = "Management node "+ownNodeId+" detected ";
+            if (weAreNewMaster) message += "we should be master, changing from ";
+            else message += "master change, from ";
+            message +=currMasterNodeId + " (" + (currMasterNodeRecord==null ? "?" : currMasterNodeRecord.getRemoteTimestamp()) + ")"
                 + " to "
                 + (newMasterNodeId == null ? "<none>" :
                     (weAreNewMaster ? "us " : "")
                     + newMasterNodeId + " (" + newMasterNodeRecord.getRemoteTimestamp() + ")" 
-                    + (newMasterNodeUri!=null ? " "+newMasterNodeUri : "")  ));
+                    + (newMasterNodeUri!=null ? " "+newMasterNodeUri : "")  );
+            LOG.warn(message);
         }
 
         // New master is ourself: promote
@@ -567,7 +655,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
         nodeState = ManagementNodeState.FAILED;
         onDemotionStopTasks();
         nodeStateTransitionComplete = true;
-        publishDemotionFromMaster(true);
+        publishDemotionFromMaster();
     }
 
     protected void demoteToStandby(boolean hot) {
@@ -579,12 +667,15 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
         nodeStateTransitionComplete = false;
         nodeState = ManagementNodeState.STANDBY;
         onDemotionStopTasks();
+        nodeStateTransitionComplete = true;
+        publishDemotionFromMaster();
+        
         if (hot) {
-            publishDemotionFromMaster(false);
+            nodeStateTransitionComplete = false;
             attemptHotStandby();
+            nodeStateTransitionComplete = true;
+            publishHealth();
         }
-        nodeStateTransitionComplete = true;
-        publishDemotionFromMaster(false);
     }
     
     protected void onDemotionStopTasks() {
@@ -703,6 +794,7 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
                 .brooklynVersion(BrooklynVersion.get())
                 .nodeId(ownNodeId)
                 .status(getNodeState())
+                .priority(getPriority())
                 .localTimestamp(timestamp)
                 .uri(managementContext.getManagementNodeUri().orNull());
         if (useLocalTimestampAsRemoteTimestamp)

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordDeltaImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordDeltaImpl.java b/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordDeltaImpl.java
index a9be0fd..b54b9ef 100644
--- a/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordDeltaImpl.java
+++ b/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordDeltaImpl.java
@@ -45,6 +45,7 @@ public class ManagementPlaneSyncRecordDeltaImpl implements Delta {
         private Collection <String> removedNodeIds = Sets.newLinkedHashSet();
         private MasterChange masterChange = MasterChange.NO_CHANGE;
         private String master;
+        private String expectedOldMaster;
         
         public Builder node(ManagementNodeSyncRecord node) {
             nodes.add(checkNotNull(node, "node")); return this;
@@ -57,8 +58,9 @@ public class ManagementPlaneSyncRecordDeltaImpl implements Delta {
             master = checkNotNull(nodeId, "masterId");
             return this;
         }
-        public Builder clearMaster(String nodeId) {
+        public Builder clearMaster(String optionalExpectedNodeId) {
             masterChange = MasterChange.CLEAR_MASTER;
+            this.expectedOldMaster = optionalExpectedNodeId;
             return this;
         }
         public Delta build() {
@@ -70,12 +72,14 @@ public class ManagementPlaneSyncRecordDeltaImpl implements Delta {
     private final Collection <String> removedNodeIds;
     private final MasterChange masterChange;
     private String masterId;
+    private String expectedOldMaster;
     
     ManagementPlaneSyncRecordDeltaImpl(Builder builder) {
         nodes = builder.nodes;
         removedNodeIds = builder.removedNodeIds;
         masterChange = builder.masterChange;
         masterId = builder.master;
+        this.expectedOldMaster = builder.expectedOldMaster;
         checkState((masterChange == MasterChange.SET_MASTER) ? (masterId != null) : (masterId == null), 
                 "invalid combination: change=%s; masterId=%s", masterChange, masterId);
     }
@@ -99,4 +103,9 @@ public class ManagementPlaneSyncRecordDeltaImpl implements Delta {
     public String getNewMasterOrNull() {
         return masterId;
     }
+    
+    @Override
+    public String getExpectedMasterToClear() {
+        return expectedOldMaster;
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java b/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java
index 3a74e9a..a00f197 100644
--- a/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java
+++ b/core/src/main/java/brooklyn/management/ha/ManagementPlaneSyncRecordPersisterToObjectStore.java
@@ -248,17 +248,26 @@ public class ManagementPlaneSyncRecordPersisterToObjectStore implements Manageme
         case NO_CHANGE:
             break; // no-op
         case SET_MASTER:
-            persistMaster(checkNotNull(delta.getNewMasterOrNull()));
+            persistMaster(checkNotNull(delta.getNewMasterOrNull()), null);
             break;
         case CLEAR_MASTER:
-            persistMaster("");
+            persistMaster("", delta.getExpectedMasterToClear());
             break; // no-op
         default:
             throw new IllegalStateException("Unknown state for master-change: "+delta.getMasterChange());
         }
     }
 
-    private void persistMaster(String nodeId) {
+    private void persistMaster(String nodeId, String optionalExpectedId) {
+        if (optionalExpectedId!=null) {
+            String currentRemoteMaster = masterWriter.get();
+            if (currentRemoteMaster==null) {
+                // nothing at remote is okay
+            } else if (!currentRemoteMaster.trim().equals(optionalExpectedId.trim())) {
+                LOG.warn("Master at server is "+currentRemoteMaster+"; expected "+optionalExpectedId+" in order to set as "+nodeId+", so not applying (yet)");
+                return;
+            }
+        }
         masterWriter.put(nodeId);
         try {
             masterWriter.waitForCurrentWrites(SYNC_WRITE_TIMEOUT);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java b/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
index afe42df..46e3f97 100644
--- a/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
@@ -20,7 +20,6 @@ package brooklyn.management.internal;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import java.io.IOException;
 import java.net.URI;
 import java.net.URL;
 import java.util.Collection;
@@ -563,6 +562,18 @@ public class NonDeploymentManagementContext implements ManagementContextInternal
         public ManagementPlaneSyncRecord getManagementPlaneSyncState() {
             throw new IllegalStateException("Non-deployment context "+NonDeploymentManagementContext.this+" is not valid for this operation.");
         }
+        @Override
+        public void changeMode(HighAvailabilityMode startMode) {
+            throw new IllegalStateException("Non-deployment context "+NonDeploymentManagementContext.this+" is not valid for this operation.");
+        }
+        @Override
+        public void setPriority(long priority) {
+            throw new IllegalStateException("Non-deployment context "+NonDeploymentManagementContext.this+" is not valid for this operation.");
+        }
+        @Override
+        public long getPriority() {
+            throw new IllegalStateException("Non-deployment context "+NonDeploymentManagementContext.this+" is not valid for this operation.");
+        }
     }
     
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/test/java/brooklyn/management/ha/HighAvailabilityManagerSplitBrainTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/management/ha/HighAvailabilityManagerSplitBrainTest.java b/core/src/test/java/brooklyn/management/ha/HighAvailabilityManagerSplitBrainTest.java
index da1fd0a..a0b7feb 100644
--- a/core/src/test/java/brooklyn/management/ha/HighAvailabilityManagerSplitBrainTest.java
+++ b/core/src/test/java/brooklyn/management/ha/HighAvailabilityManagerSplitBrainTest.java
@@ -41,6 +41,7 @@ import brooklyn.entity.rebind.persister.InMemoryObjectStore;
 import brooklyn.entity.rebind.persister.ListeningObjectStore;
 import brooklyn.entity.rebind.persister.PersistMode;
 import brooklyn.entity.rebind.persister.PersistenceObjectStore;
+import brooklyn.internal.BrooklynFeatureEnablement;
 import brooklyn.location.Location;
 import brooklyn.management.internal.ManagementContextInternal;
 import brooklyn.test.Asserts;
@@ -256,8 +257,11 @@ public class HighAvailabilityManagerSplitBrainTest {
         ManagementPlaneSyncRecord memento1b = n1.ha.getManagementPlaneSyncState();
         log.info(n1+" HA now: "+memento1b);
         
+        ManagementNodeState expectedStateAfterDemotion = BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_DEFAULT_STANDBY_IS_HOT_PROPERTY) ?
+            ManagementNodeState.HOT_STANDBY : ManagementNodeState.STANDBY;
+        
         // n1 comes back and demotes himself 
-        assertEquals(memento1b.getManagementNodes().get(n1.ownNodeId).getStatus(), ManagementNodeState.HOT_STANDBY);
+        assertEquals(memento1b.getManagementNodes().get(n1.ownNodeId).getStatus(), expectedStateAfterDemotion);
         assertEquals(memento1b.getManagementNodes().get(n2.ownNodeId).getStatus(), ManagementNodeState.MASTER);
         assertEquals(memento1b.getMasterNodeId(), n2.ownNodeId);
         assertEquals(memento1b.getManagementNodes().get(n1.ownNodeId).getRemoteTimestamp(), time2);
@@ -266,7 +270,7 @@ public class HighAvailabilityManagerSplitBrainTest {
         // n2 now sees itself as master, with n1 in standby again
         ManagementPlaneSyncRecord memento2c = n2.ha.getManagementPlaneSyncState();
         log.info(n2+" HA now: "+memento2c);
-        assertEquals(memento2c.getManagementNodes().get(n1.ownNodeId).getStatus(), ManagementNodeState.HOT_STANDBY);
+        assertEquals(memento2c.getManagementNodes().get(n1.ownNodeId).getStatus(), expectedStateAfterDemotion);
         assertEquals(memento2c.getManagementNodes().get(n2.ownNodeId).getStatus(), ManagementNodeState.MASTER);
         assertEquals(memento2c.getMasterNodeId(), n2.ownNodeId);
         assertEquals(memento2c.getManagementNodes().get(n1.ownNodeId).getRemoteTimestamp(), time2);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java b/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
index 492f2f1..1a41c19 100644
--- a/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
+++ b/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
@@ -114,6 +114,10 @@ public class HotStandbyTest {
         public String toString() {
             return nodeName+" "+ownNodeId;
         }
+
+        public RebindManagerImpl rebinder() {
+            return (RebindManagerImpl)mgmt.getRebindManager();
+        }
     }
     
     @BeforeMethod(alwaysRun=true)
@@ -443,6 +447,8 @@ public class HotStandbyTest {
     }
 
     @Test(groups="Integration") // because it's slow
+    // Sept 2014 - there is a small leak, of 200 bytes per child created and destroyed;
+    // but this goes away when the app is destroyed; it may be a benign record
     public void testHotStandbyDoesNotLeakLotsOfRebindsCreatingAndDestroyingAChildEntity() throws Exception {
         log.info("Starting test "+JavaClassNames.niceClassAndMethod());
         final int DELTA = 2;
@@ -481,4 +487,76 @@ public class HotStandbyTest {
         
         assertUsedMemoryLessThan("And now all unmanaged", initialUsed + DELTA*1000*1000);
     }
+    
+    protected void assertHotStandby(HaMgmtNode n1) {
+        assertEquals(n1.ha.getNodeState(), ManagementNodeState.HOT_STANDBY);
+        Assert.assertTrue(n1.rebinder().isReadOnlyRunning());
+        Assert.assertFalse(n1.rebinder().isPersistenceRunning());
+    }
+
+    protected void assertMaster(HaMgmtNode n1) {
+        assertEquals(n1.ha.getNodeState(), ManagementNodeState.MASTER);
+        Assert.assertFalse(n1.rebinder().isReadOnlyRunning());
+        Assert.assertTrue(n1.rebinder().isPersistenceRunning());
+    }
+
+    @Test
+    public void testChangeMode() throws Exception {
+        HaMgmtNode n1 = createMaster(Duration.PRACTICALLY_FOREVER);
+        TestApplication app = createFirstAppAndPersist(n1);
+        HaMgmtNode n2 = createHotStandby(Duration.PRACTICALLY_FOREVER);
+
+        TestEntity child = app.addChild(EntitySpec.create(TestEntity.class).configure(TestEntity.CONF_NAME, "first-child"));
+        Entities.manage(child);
+        TestApplication app2 = TestApplication.Factory.newManagedInstanceForTests(n1.mgmt);
+        app2.setConfig(TestEntity.CONF_NAME, "second-app");
+
+        forcePersistNow(n1);
+        n2.ha.setPriority(1);
+        n1.ha.changeMode(HighAvailabilityMode.HOT_STANDBY);
+        
+        // both now hot standby
+        assertHotStandby(n1);
+        assertHotStandby(n2);
+        
+        assertEquals(n1.mgmt.getApplications().size(), 2);
+        Application app2RO = n1.mgmt.lookup(app2.getId(), Application.class);
+        Assert.assertNotNull(app2RO);
+        assertEquals(app2RO.getConfig(TestEntity.CONF_NAME), "second-app");
+        try {
+            ((TestApplication)app2RO).setAttribute(TestEntity.SEQUENCE, 4);
+            Assert.fail("Should not have allowed sensor to be set");
+        } catch (Exception e) {
+            Assert.assertTrue(e.toString().toLowerCase().contains("read-only"), "Error message did not contain expected text: "+e);
+        }
+
+        n1.ha.changeMode(HighAvailabilityMode.AUTO);
+        n2.ha.changeMode(HighAvailabilityMode.HOT_STANDBY, true, false);
+        // both still hot standby (n1 will defer to n2 as it has higher priority)
+        assertHotStandby(n1);
+        assertHotStandby(n2);
+        
+        // with priority 1, n2 will now be elected
+        n2.ha.changeMode(HighAvailabilityMode.AUTO);
+        assertHotStandby(n1);
+        assertMaster(n2);
+        
+        assertEquals(n2.mgmt.getApplications().size(), 2);
+        Application app2B = n2.mgmt.lookup(app2.getId(), Application.class);
+        Assert.assertNotNull(app2B);
+        assertEquals(app2B.getConfig(TestEntity.CONF_NAME), "second-app");
+        ((TestApplication)app2B).setAttribute(TestEntity.SEQUENCE, 4);
+        
+        forcePersistNow(n2);
+        forceRebindNow(n1);
+        Application app2BRO = n1.mgmt.lookup(app2.getId(), Application.class);
+        Assert.assertNotNull(app2BRO);
+        EntityTestUtils.assertAttributeEquals(app2BRO, TestEntity.SEQUENCE, 4);
+    }
+
+    @Test(groups="Integration", invocationCount=20)
+    public void testChangeModeManyTimes() throws Exception {
+        testChangeMode();
+    }
+    
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/core/src/test/java/brooklyn/management/ha/MasterChooserTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/management/ha/MasterChooserTest.java b/core/src/test/java/brooklyn/management/ha/MasterChooserTest.java
index 6f8ba48..4373ea0 100644
--- a/core/src/test/java/brooklyn/management/ha/MasterChooserTest.java
+++ b/core/src/test/java/brooklyn/management/ha/MasterChooserTest.java
@@ -106,11 +106,39 @@ public class MasterChooserTest {
         memento.addNode(newManagerMemento("node5", ManagementNodeState.MASTER, now));
         if (includeHot)
             memento.addNode(newManagerMemento("node6",  ManagementNodeState.HOT_STANDBY, now));
-        Duration heartbeatTimeout = Duration.THIRTY_SECONDS;
-        return getIds(chooser.sort(chooser.filterHealthy(memento, heartbeatTimeout, now)));
+        return getIds(chooser.sort(chooser.filterHealthy(memento, Duration.THIRTY_SECONDS, now)));
     }
-    
+
+    @Test
+    public void testExplicityPriority() throws Exception {
+        chooser = new AlphabeticMasterChooser();
+        memento.addNode(newManagerMemento("node1", ManagementNodeState.STANDBY, now, BrooklynVersion.get(), 2L));
+        memento.addNode(newManagerMemento("node2", ManagementNodeState.STANDBY, now, BrooklynVersion.get(), -1L));
+        memento.addNode(newManagerMemento("node3", ManagementNodeState.STANDBY, now, BrooklynVersion.get(), null));
+        List<String> order = getIds(chooser.sort(chooser.filterHealthy(memento, Duration.THIRTY_SECONDS, now)));
+        assertEquals(order, ImmutableList.of("node1", "node3", "node2"));
+    }
+
+    @Test
+    public void testVersionsMaybeNull() throws Exception {
+        chooser = new AlphabeticMasterChooser();
+        memento.addNode(newManagerMemento("node1", ManagementNodeState.STANDBY, now, "v10", null));
+        memento.addNode(newManagerMemento("node2", ManagementNodeState.STANDBY, now, "v3-snapshot", null));
+        memento.addNode(newManagerMemento("node3", ManagementNodeState.STANDBY, now, "v3-snapshot", -1L));
+        memento.addNode(newManagerMemento("node4", ManagementNodeState.STANDBY, now, "v3-snapshot", null));
+        memento.addNode(newManagerMemento("node5", ManagementNodeState.STANDBY, now, "v3-stable", null));
+        memento.addNode(newManagerMemento("node6", ManagementNodeState.STANDBY, now, "v1", null));
+        memento.addNode(newManagerMemento("node7", ManagementNodeState.STANDBY, now, null, null));
+        List<String> order = getIds(chooser.sort(chooser.filterHealthy(memento, Duration.THIRTY_SECONDS, now)));
+        assertEquals(order, ImmutableList.of("node1", "node5", "node6", "node2", "node4", "node7", "node3"));
+    }
+
     private ManagementNodeSyncRecord newManagerMemento(String nodeId, ManagementNodeState status, long timestamp) {
-        return BasicManagementNodeSyncRecord.builder().brooklynVersion(BrooklynVersion.get()).nodeId(nodeId).status(status).localTimestamp(timestamp).remoteTimestamp(timestamp).build();
+        return newManagerMemento(nodeId, status, timestamp, BrooklynVersion.get(), null);
+    }
+    private ManagementNodeSyncRecord newManagerMemento(String nodeId, ManagementNodeState status, long timestamp,
+            String version, Long priority) {
+        return BasicManagementNodeSyncRecord.builder().brooklynVersion(version).nodeId(nodeId).status(status).localTimestamp(timestamp).remoteTimestamp(timestamp).
+            priority(priority).build();
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherRebindToCloudObjectStoreTest.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherRebindToCloudObjectStoreTest.java b/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherRebindToCloudObjectStoreTest.java
index 6d9a71a..e4462e3 100644
--- a/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherRebindToCloudObjectStoreTest.java
+++ b/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherRebindToCloudObjectStoreTest.java
@@ -20,13 +20,9 @@ package brooklyn.launcher;
 
 import static org.testng.Assert.assertEquals;
 
-import java.io.File;
-
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-import com.google.common.io.Files;
-
 import brooklyn.config.BrooklynProperties;
 import brooklyn.config.BrooklynServerConfig;
 import brooklyn.entity.proxying.EntitySpec;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/32c926b9/utils/common/src/main/java/brooklyn/util/text/NaturalOrderComparator.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/text/NaturalOrderComparator.java b/utils/common/src/main/java/brooklyn/util/text/NaturalOrderComparator.java
index 5826a73..34d96e3 100644
--- a/utils/common/src/main/java/brooklyn/util/text/NaturalOrderComparator.java
+++ b/utils/common/src/main/java/brooklyn/util/text/NaturalOrderComparator.java
@@ -54,7 +54,8 @@ import java.util.Comparator;
  * e.g. "10">"9", including when those numbers occur in the midst of equal text; e.g. "a10" > "a9";
  * but not if the text differs; e.g. "a10" < "b9"
  * <p>
- * class is thread-safe  
+ * class is thread-safe. nulls not supported. (to support nulls, wrap in guava:
+ * <code>Ordering.from(NaturalOrderComparator.INSTANCE).nullsFirst()</code>)
  */
 public class NaturalOrderComparator implements Comparator<String> {
     


Mime
View raw message