modificiations to emit BACKUP_CHANGED Mastership events

Change-Id: Id61dcc9dc42c8c246313afbec8d19142e6c855a5

Conflicts:
	core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java

@@ -40,7 +40,9 @@ public class MastershipEvent extends AbstractEvent<MastershipEvent.Type, DeviceI
         MASTER_CHANGED,
 
         /**
-         * Signifies that the list of backup nodes has changed.
+         * Signifies that the list of backup nodes has changed. If
+         * the change in the backups list is accompanied by a change in
+         * master, the event is subsumed by MASTER_CHANGED.
          */
         BACKUPS_CHANGED
     }

@@ -16,6 +16,7 @@
 package org.onlab.onos.store.mastership.impl;
 
 import static org.onlab.onos.mastership.MastershipEvent.Type.MASTER_CHANGED;
+import static org.onlab.onos.mastership.MastershipEvent.Type.BACKUPS_CHANGED;
 import static org.apache.commons.lang3.concurrent.ConcurrentUtils.putIfAbsent;
 
 import java.util.HashSet;
@@ -43,6 +44,7 @@ import org.onlab.onos.store.serializers.KryoNamespaces;
 import org.onlab.onos.store.serializers.KryoSerializer;
 import org.onlab.util.KryoNamespace;
 
+import com.google.common.base.Objects;
 import com.hazelcast.core.EntryEvent;
 import com.hazelcast.core.EntryListener;
 import com.hazelcast.core.IAtomicLong;
@@ -297,8 +299,7 @@ implements MastershipStore {
                 case NONE:
                     rv.reassign(nodeId, NONE, STANDBY);
                     roleMap.put(deviceId, rv);
-                    // TODO: BACKUPS_CHANGED?
-                    return null;
+                    return new MastershipEvent(BACKUPS_CHANGED, deviceId, rv.roleInfo());
                 default:
                     log.warn("unknown Mastership Role {}", currentRole);
             }
@@ -327,7 +328,8 @@ implements MastershipStore {
                         roleMap.put(deviceId, rv);
                         return new MastershipEvent(MASTER_CHANGED, deviceId, rv.roleInfo());
                     } else {
-                        // no master candidate
+                        // No master candidate - no more backups, device is likely
+                        // fully disconnected
                         roleMap.put(deviceId, rv);
                         // Should there be new event type?
                         return null;
@@ -338,8 +340,7 @@ implements MastershipStore {
                     boolean modified = rv.reassign(nodeId, STANDBY, NONE);
                     if (modified) {
                         roleMap.put(deviceId, rv);
-                        // TODO: BACKUPS_CHANGED?
-                        return null;
+                        return new MastershipEvent(BACKUPS_CHANGED, deviceId, rv.roleInfo());
                     }
                     return null;
                 default:
@@ -441,8 +442,18 @@ implements MastershipStore {
 
         @Override
         public void entryUpdated(EntryEvent<DeviceId, RoleValue> event) {
-            notifyDelegate(new MastershipEvent(
-                    MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
+            // compare old and current RoleValues. If master is different,
+            // emit MASTER_CHANGED. else, emit BACKUPS_CHANGED.
+            RoleValue oldValue = event.getOldValue();
+            RoleValue newValue = event.getValue();
+
+            if (Objects.equal(oldValue.get(MASTER), newValue.get(MASTER))) {
+                notifyDelegate(new MastershipEvent(
+                        MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
+            } else {
+                notifyDelegate(new MastershipEvent(
+                        BACKUPS_CHANGED, event.getKey(), event.getValue().roleInfo()));
+            }
         }
 
         @Override

@@ -55,6 +55,13 @@ final class RoleValue {
         return value.get(type);
     }
 
+    /**
+     * Returns the first node to match the MastershipRole, or if there
+     * are none, null.
+     *
+     * @param type the role
+     * @return a node ID or null
+     */
     public NodeId get(MastershipRole type) {
         return value.get(type).isEmpty() ? null : value.get(type).get(0);
     }

@@ -214,11 +214,11 @@ public class DistributedMastershipStoreTest {
                 dms.roleMap.get(DID1).nodesOfRole(STANDBY).size());
 
         //If STANDBY, should drop to NONE
-        assertNull("wrong event:", dms.relinquishRole(N1, DID1));
+        assertEquals("wrong event:", Type.BACKUPS_CHANGED, dms.relinquishRole(N1, DID1).type());
         assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID1));
 
         //NONE - nothing happens
-        assertNull("wrong event:", dms.relinquishRole(N1, DID2));
+        assertEquals("wrong event:", Type.BACKUPS_CHANGED, dms.relinquishRole(N1, DID2).type());
         assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID2));
 
     }

@@ -51,13 +51,6 @@ import com.google.common.cache.CacheBuilder;
  * After a role request is submitted the role changer keeps track of the
  * pending request, collects the reply (if any) and times out the request
  * if necessary.
- *
- * To simplify role handling we only keep track of the /last/ pending
- * role reply send to the switch. If multiple requests are pending and
- * we receive replies for earlier requests we ignore them. However, this
- * way of handling pending requests implies that we could wait forever if
- * a new request is submitted before the timeout triggers. If necessary
- * we could work around that though.
  */
 class RoleManager implements RoleHandler {
     protected static final long NICIRA_EXPERIMENTER = 0x2320;