relinquishes mastership when device disconnects

Change-Id: I1aecc8862ce297569c358e1deb5ddc5fb52d5dd3

@@ -78,22 +78,15 @@ implements MastershipService, MastershipAdminService {
         checkNotNull(deviceId, DEVICE_ID_NULL);
         checkNotNull(deviceId, DEVICE_ID_NULL);
         checkNotNull(role, ROLE_NULL);
         checkNotNull(role, ROLE_NULL);
 
 
-        MastershipRole current = store.getRole(nodeId, deviceId);
-        if (role.equals(current)) {
-            return;
+        MastershipEvent event = null;
+        if (role.equals(MastershipRole.MASTER)) {
+            event = store.setMaster(nodeId, deviceId);
         } else {
         } else {
-            MastershipEvent event = null;
-            if (role.equals(MastershipRole.MASTER)) {
-                //current was STANDBY, wanted MASTER
-                event = store.setMaster(nodeId, deviceId);
-            } else {
-                //current was MASTER, wanted STANDBY
-                event = store.unsetMaster(nodeId, deviceId);
-            }
+            event = store.unsetMaster(nodeId, deviceId);
+        }
 
 
-            if (event != null) {
-                post(event);
-            }
+        if (event != null) {
+            post(event);
         }
         }
     }
     }
 
 
@@ -105,10 +98,7 @@ implements MastershipService, MastershipAdminService {
 
 
     @Override
     @Override
     public void relinquishMastership(DeviceId deviceId) {
     public void relinquishMastership(DeviceId deviceId) {
-        checkNotNull(deviceId, DEVICE_ID_NULL);
-
-        MastershipRole role = store.getRole(
-                clusterService.getLocalNode().id(), deviceId);
+        MastershipRole role = getLocalRole(deviceId);
         if (!role.equals(MastershipRole.MASTER)) {
         if (!role.equals(MastershipRole.MASTER)) {
             return;
             return;
         }
         }

@@ -202,7 +202,7 @@ public class DeviceManager
                 log.info("Device {} connected", deviceId);
                 log.info("Device {} connected", deviceId);
                 mastershipService.requestRoleFor(deviceId);
                 mastershipService.requestRoleFor(deviceId);
                 provider().roleChanged(event.subject(),
                 provider().roleChanged(event.subject(),
-                        mastershipService.getLocalRole(deviceId));
+                        mastershipService.requestRoleFor(deviceId));
                 post(event);
                 post(event);
             }
             }
         }
         }

@@ -113,7 +113,6 @@ public class MastershipManagerTest {
         mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
         mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
         mgr.setRole(NID_LOCAL, DEV_OTHER, STANDBY);
         mgr.setRole(NID_LOCAL, DEV_OTHER, STANDBY);
         assertEquals("should be one device:", 1, mgr.getDevicesOf(NID_LOCAL).size());
         assertEquals("should be one device:", 1, mgr.getDevicesOf(NID_LOCAL).size());
-
         //hand both devices to NID_LOCAL
         //hand both devices to NID_LOCAL
         mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER);
         mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER);
         assertEquals("should be two devices:", 2, mgr.getDevicesOf(NID_LOCAL).size());
         assertEquals("should be two devices:", 2, mgr.getDevicesOf(NID_LOCAL).size());

@@ -5,7 +5,6 @@ import static org.slf4j.LoggerFactory.getLogger;
 import java.util.Collections;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Map;
 import java.util.Set;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -27,8 +26,6 @@ import org.onlab.onos.store.AbstractStore;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.IpPrefix;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 
 
-import com.google.common.collect.Lists;
-
 import static org.onlab.onos.cluster.MastershipEvent.Type.*;
 import static org.onlab.onos.cluster.MastershipEvent.Type.*;
 
 
 /**
 /**
@@ -50,8 +47,8 @@ public class SimpleMastershipStore
 
 
     //devices mapped to their masters, to emulate multiple nodes
     //devices mapped to their masters, to emulate multiple nodes
     protected final Map<DeviceId, NodeId> masterMap = new HashMap<>();
     protected final Map<DeviceId, NodeId> masterMap = new HashMap<>();
-    //emulate backups
-    protected final Map<DeviceId, List<NodeId>> backupMap = new HashMap<>();
+    //emulate backups with pile of nodes
+    protected final Set<NodeId> backups = new HashSet<>();
     //terms
     //terms
     protected final Map<DeviceId, AtomicInteger> termMap = new HashMap<>();
     protected final Map<DeviceId, AtomicInteger> termMap = new HashMap<>();
 
 
@@ -67,38 +64,28 @@ public class SimpleMastershipStore
 
 
     @Override
     @Override
     public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
     public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
+        MastershipRole role = getRole(nodeId, deviceId);
 
 
-        NodeId current = masterMap.get(deviceId);
-        List<NodeId> backups = backupMap.get(deviceId);
-
-        if (current == null) {
-            if (backups == null) {
-                //add new mapping to everything
-                synchronized (this) {
-                    masterMap.put(deviceId, nodeId);
-                    backups = Lists.newLinkedList();
-                    backupMap.put(deviceId, backups);
-                    termMap.put(deviceId, new AtomicInteger());
-                }
-            } else {
-                //set master to new node and remove from backups if there
-                synchronized (this) {
+        synchronized (this) {
+            switch (role) {
+                case MASTER:
+                    return null;
+                case STANDBY:
                     masterMap.put(deviceId, nodeId);
                     masterMap.put(deviceId, nodeId);
-                    backups.remove(nodeId);
                     termMap.get(deviceId).incrementAndGet();
                     termMap.get(deviceId).incrementAndGet();
-                }
+                    backups.add(nodeId);
+                    break;
+                case NONE:
+                    masterMap.put(deviceId, nodeId);
+                    termMap.put(deviceId, new AtomicInteger());
+                    backups.add(nodeId);
+                    break;
+                default:
+                    log.warn("unknown Mastership Role {}", role);
+                    return null;
             }
             }
-        } else if (current.equals(nodeId)) {
-            return null;
-        } else {
-            //add current to backup, set master to new node
-            masterMap.put(deviceId, nodeId);
-            backups.add(current);
-            backups.remove(nodeId);
-            termMap.get(deviceId).incrementAndGet();
         }
         }
 
 
-        updateStandby(nodeId, deviceId);
         return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
         return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
     }
     }
 
 
@@ -120,51 +107,61 @@ public class SimpleMastershipStore
 
 
     @Override
     @Override
     public MastershipRole requestRole(DeviceId deviceId) {
     public MastershipRole requestRole(DeviceId deviceId) {
-        return getRole(instance.id(), deviceId);
+        //query+possible reelection
+        NodeId node = instance.id();
+        MastershipRole role = getRole(node, deviceId);
+
+        switch (role) {
+            case MASTER:
+                break;
+            case STANDBY:
+                synchronized (this) {
+                    //try to "re-elect", since we're really not distributed
+                    NodeId rel = reelect(node);
+                    if (rel == null) {
+                        masterMap.put(deviceId, node);
+                        termMap.put(deviceId, new AtomicInteger());
+                        role = MastershipRole.MASTER;
+                    }
+                    backups.add(node);
+                }
+                break;
+            case NONE:
+                //first to get to it, say we are master
+                synchronized (this) {
+                    masterMap.put(deviceId, node);
+                    termMap.put(deviceId, new AtomicInteger());
+                    backups.add(node);
+                    role = MastershipRole.MASTER;
+                }
+                break;
+            default:
+                log.warn("unknown Mastership Role {}", role);
+        }
+        return role;
     }
     }
 
 
     @Override
     @Override
     public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
     public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
+        //just query
         NodeId current = masterMap.get(deviceId);
         NodeId current = masterMap.get(deviceId);
-        List<NodeId> backups = backupMap.get(deviceId);
+        MastershipRole role;
 
 
         if (current == null) {
         if (current == null) {
-            //masterMap or backup doesn't contain device. Say new node is MASTER
-            if (backups == null) {
-                synchronized (this) {
-                    masterMap.put(deviceId, nodeId);
-                    backups = Lists.newLinkedList();
-                    backupMap.put(deviceId, backups);
-                    termMap.put(deviceId, new AtomicInteger());
-                }
-                updateStandby(nodeId, deviceId);
-                return MastershipRole.MASTER;
-            }
-
-            //device once existed, but got removed, and is now getting a backup.
-            if (!backups.contains(nodeId)) {
-                synchronized (this) {
-                    backups.add(nodeId);
-                    termMap.put(deviceId, new AtomicInteger());
-                }
-                updateStandby(nodeId, deviceId);
+            //degenerate case - only node is its own backup
+            if (backups.contains(nodeId)) {
+                role = MastershipRole.STANDBY;
+            } else {
+                role = MastershipRole.NONE;
             }
             }
-
-        } else if (current.equals(nodeId)) {
-            return  MastershipRole.MASTER;
         } else {
         } else {
-            //once created, a device never has a null backups list.
-            if (!backups.contains(nodeId)) {
-                //we must have requested STANDBY setting
-                synchronized (this) {
-                    backups.add(nodeId);
-                    termMap.put(deviceId, new AtomicInteger());
-                }
-                updateStandby(nodeId, deviceId);
+            if (current.equals(nodeId)) {
+                role = MastershipRole.MASTER;
+            } else {
+                role = MastershipRole.STANDBY;
             }
             }
         }
         }
-
-        return MastershipRole.STANDBY;
+        return role;
     }
     }
 
 
     @Override
     @Override
@@ -179,43 +176,43 @@ public class SimpleMastershipStore
 
 
     @Override
     @Override
     public MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId) {
     public MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId) {
-        NodeId node = masterMap.get(deviceId);
-
-        //TODO case where node is completely removed from the cluster?
-        if (node.equals(nodeId)) {
-            synchronized (this) {
-                //pick new node.
-                List<NodeId> backups = backupMap.get(deviceId);
-
-                //no backups, so device is hosed
-                if (backups.isEmpty()) {
-                    masterMap.remove(deviceId);
+        MastershipRole role = getRole(nodeId, deviceId);
+        synchronized (this) {
+            switch (role) {
+                case MASTER:
+                    NodeId backup = reelect(nodeId);
+                    if (backup == null) {
+                        masterMap.remove(deviceId);
+                    } else {
+                        masterMap.put(deviceId, backup);
+                        termMap.get(deviceId).incrementAndGet();
+                        return new MastershipEvent(MASTER_CHANGED, deviceId, backup);
+                    }
+                case STANDBY:
+                case NONE:
+                    if (!termMap.containsKey(deviceId)) {
+                        termMap.put(deviceId, new AtomicInteger());
+                    }
                     backups.add(nodeId);
                     backups.add(nodeId);
-                    return null;
-                }
-                NodeId backup = backups.remove(0);
-                masterMap.put(deviceId, backup);
-                backups.add(nodeId);
-                return new MastershipEvent(MASTER_CHANGED, deviceId, backup);
+                    break;
+                default:
+                    log.warn("unknown Mastership Role {}", role);
             }
             }
         }
         }
         return null;
         return null;
     }
     }
 
 
-    //add node as STANDBY to maps un-scalably.
-    private void updateStandby(NodeId nodeId, DeviceId deviceId) {
-        for (Map.Entry<DeviceId, List<NodeId>> e : backupMap.entrySet()) {
-            DeviceId dev = e.getKey();
-            if (dev.equals(deviceId)) {
-                continue;
-            }
-            synchronized (this) {
-                List<NodeId> nodes = e.getValue();
-                if (!nodes.contains(nodeId)) {
-                    nodes.add(nodeId);
-                }
+    //dumbly selects next-available node that's not the current one
+    //emulate leader election
+    private NodeId reelect(NodeId nodeId) {
+        NodeId backup = null;
+        for (NodeId n : backups) {
+            if (!n.equals(nodeId)) {
+                backup = n;
+                break;
             }
             }
         }
         }
+        return backup;
     }
     }
 
 
 }
 }