refactored MastershipStore to not use ILock

Change-Id: Ic254f6faddba3427d3380910ca90d3d65a29f40b

@@ -2,6 +2,7 @@ package org.onlab.onos.store.mastership.impl;
 
 import static org.onlab.onos.mastership.MastershipEvent.Type.MASTER_CHANGED;
 
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -22,12 +23,15 @@ import org.onlab.onos.mastership.MastershipTerm;
 import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.store.common.AbstractHazelcastStore;
+import org.onlab.onos.store.common.SMap;
+import org.onlab.onos.store.serializers.KryoSerializer;
 
 import com.google.common.collect.ImmutableSet;
-import com.hazelcast.core.ILock;
 import com.hazelcast.core.IMap;
 import com.hazelcast.core.MultiMap;
 
+import static org.onlab.onos.net.MastershipRole.*;
+
 /**
  * Distributed implementation of the mastership store. The store is
  * responsible for the master selection process.
@@ -38,36 +42,26 @@ public class DistributedMastershipStore
 extends AbstractHazelcastStore<MastershipEvent, MastershipStoreDelegate>
 implements MastershipStore {
 
-    //arbitrary lock name
-    private static final String LOCK = "lock";
     //initial term/TTL value
     private static final Integer INIT = 0;
 
-    //devices to masters
-    protected IMap<byte[], byte[]> masters;
+    //device to node roles
+    protected SMap<DeviceId, RoleValue> roleMap;
     //devices to terms
-    protected IMap<byte[], Integer> terms;
-
-    //re-election related, disjoint-set structures:
-    //device-nodes multiset of available nodes
-    protected MultiMap<byte[], byte[]> standbys;
-    //device-nodes multiset for nodes that have given up on device
-    protected MultiMap<byte[], byte[]> unusable;
+    protected SMap<DeviceId, Integer> terms;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected ClusterService clusterService;
 
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     @Override
     @Activate
     public void activate() {
         super.activate();
 
-        masters = theInstance.getMap("masters");
-        terms = theInstance.getMap("terms");
-        standbys = theInstance.getMultiMap("backups");
-        unusable = theInstance.getMultiMap("unusable");
-
-        masters.addEntryListener(new RemoteMasterShipEventHandler(), true);
+        roleMap = new SMap(theInstance.getMap("nodeRoles"), new KryoSerializer());
+        terms = new SMap(theInstance.getMap("terms"), new KryoSerializer());
+       // roleMap.addEntryListener(new RemoteMasterShipEventHandler(), true);
 
         log.info("Started");
     }
@@ -79,12 +73,9 @@ implements MastershipStore {
 
     @Override
     public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
-        byte[] did = serialize(deviceId);
-        byte[] nid = serialize(nodeId);
-
-        NodeId current = deserialize(masters.get(did));
+        NodeId current = getNode(MASTER, deviceId);
         if (current == null) {
-            if (standbys.containsEntry(did, nid)) {
+            if (isRole(STANDBY, nodeId, deviceId)) {
                 //was previously standby, or set to standby from master
                 return MastershipRole.STANDBY;
             } else {
@@ -103,69 +94,66 @@ implements MastershipStore {
 
     @Override
     public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
-        byte [] did = serialize(deviceId);
-        byte [] nid = serialize(nodeId);
 
-        ILock lock = theInstance.getLock(LOCK);
-        lock.lock();
+        MastershipRole role = getRole(nodeId, deviceId);
+        roleMap.lock(deviceId);
         try {
-            MastershipRole role = getRole(nodeId, deviceId);
+            RoleValue rv = getRoleValue(deviceId);
             switch (role) {
                 case MASTER:
                     //reinforce mastership
-                    evict(nid, did);
+                    rv.reassign(nodeId, STANDBY, NONE);
                     return null;
                 case STANDBY:
-                    //make current master standby
-                    byte [] current = masters.get(did);
+                    NodeId current = rv.get(MASTER);
                     if (current != null) {
-                        backup(current, did);
+                        //backup and replace current master
+                        rv.reassign(nodeId, NONE, STANDBY);
+                        rv.replace(current, nodeId, MASTER);
+                    } else {
+                        //no master before so just add.
+                        rv.add(MASTER, nodeId);
                     }
-                    //assign specified node as new master
-                    masters.put(did, nid);
-                    evict(nid, did);
-                    updateTerm(did);
+                    rv.reassign(nodeId, STANDBY, NONE);
+                    updateTerm(deviceId);
                     return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
                 case NONE:
-                    masters.put(did, nid);
-                    evict(nid, did);
-                    updateTerm(did);
+                    rv.add(MASTER, nodeId);
+                    rv.reassign(nodeId, STANDBY, NONE);
+                    updateTerm(deviceId);
                     return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
                 default:
                     log.warn("unknown Mastership Role {}", role);
                     return null;
             }
         } finally {
-            lock.unlock();
+            roleMap.unlock(deviceId);
         }
     }
 
     @Override
     public NodeId getMaster(DeviceId deviceId) {
-        return deserialize(masters.get(serialize(deviceId)));
+        return getMaster(deviceId);
     }
 
 
     @Override
     public List<NodeId> getNodes(DeviceId deviceId) {
-        byte [] did = serialize(deviceId);
         List<NodeId> nodes = new LinkedList<>();
 
-        //add current master to head - if there is one
-        ILock lock = theInstance.getLock(LOCK);
-        lock.lock();
+        //add current master to head - if there is one.
+        roleMap.lock(deviceId);
         try {
-            byte [] master = masters.get(did);
+            RoleValue rv = getRoleValue(deviceId);
+            NodeId master = rv.get(MASTER);
             if (master != null) {
-                nodes.add((NodeId) deserialize(master));
-            }
-
-            for (byte [] el : standbys.get(serialize(deviceId))) {
-                nodes.add((NodeId) deserialize(el));
+                nodes.add(master);
             }
-            return nodes;
+            //We ignore NONE nodes.
+            nodes.addAll(rv.nodesOfRole(STANDBY));
+            return Collections.unmodifiableList(nodes);
         } finally {
-            lock.unlock();
+            roleMap.unlock(deviceId);
         }
     }
 
@@ -173,9 +161,9 @@ implements MastershipStore {
     public Set<DeviceId> getDevices(NodeId nodeId) {
         ImmutableSet.Builder<DeviceId> builder = ImmutableSet.builder();
 
-        for (Map.Entry<byte[], byte[]> entry : masters.entrySet()) {
-            if (nodeId.equals(deserialize(entry.getValue()))) {
-                builder.add((DeviceId) deserialize(entry.getKey()));
+        for (Map.Entry<DeviceId, RoleValue> el : roleMap.entrySet()) {
+            if (nodeId.equals(el.getValue().get(MASTER))) {
+                builder.add(el.getKey());
             }
         }
 
@@ -185,26 +173,24 @@ implements MastershipStore {
     @Override
     public MastershipRole requestRole(DeviceId deviceId) {
         NodeId local = clusterService.getLocalNode().id();
-        byte [] did = serialize(deviceId);
-        byte [] lnid = serialize(local);
 
-        ILock lock = theInstance.getLock(LOCK);
-        lock.lock();
+        roleMap.lock(deviceId);
         try {
+            RoleValue rv = getRoleValue(deviceId);
             MastershipRole role = getRole(local, deviceId);
             switch (role) {
                 case MASTER:
-                    evict(lnid, did);
+                    rv.reassign(local, STANDBY, NONE);
                     break;
                 case STANDBY:
-                    backup(lnid, did);
-                    terms.putIfAbsent(did, INIT);
+                    rv.reassign(local, NONE, STANDBY);
+                    terms.putIfAbsent(deviceId, INIT);
                     break;
                 case NONE:
                     //claim mastership
-                    masters.put(did, lnid);
-                    evict(lnid, did);
-                    updateTerm(did);
+                    rv.add(MASTER, local);
+                    rv.reassign(local, STANDBY, NONE);
+                    updateTerm(deviceId);
                     role = MastershipRole.MASTER;
                     break;
                 default:
@@ -212,128 +198,128 @@ implements MastershipStore {
             }
             return role;
         } finally {
-            lock.unlock();
+            roleMap.unlock(deviceId);
         }
     }
 
     @Override
     public MastershipTerm getTermFor(DeviceId deviceId) {
-        byte[] did = serialize(deviceId);
-        if ((masters.get(did) == null) ||
-                (terms.get(did) == null)) {
+        RoleValue rv = getRoleValue(deviceId);
+        if ((rv.get(MASTER) == null) || (terms.get(deviceId) == null)) {
             return null;
         }
-        return MastershipTerm.of(
-                (NodeId) deserialize(masters.get(did)), terms.get(did));
+        return MastershipTerm.of(rv.get(MASTER), terms.get(deviceId));
     }
 
     @Override
     public MastershipEvent setStandby(NodeId nodeId, DeviceId deviceId) {
-        byte [] did = serialize(deviceId);
-        byte [] nid = serialize(nodeId);
         MastershipEvent event = null;
 
-        ILock lock = theInstance.getLock(LOCK);
-        lock.lock();
+        roleMap.lock(deviceId);
         try {
+            RoleValue rv = getRoleValue(deviceId);
             MastershipRole role = getRole(nodeId, deviceId);
             switch (role) {
                 case MASTER:
                     event = reelect(nodeId, deviceId);
-                    backup(nid, did);
-                    break;
+                    //fall through to reinforce role
                 case STANDBY:
                     //fall through to reinforce role
                 case NONE:
-                    backup(nid, did);
+                    rv.reassign(nodeId, NONE, STANDBY);
                     break;
                 default:
                     log.warn("unknown Mastership Role {}", role);
             }
             return event;
         } finally {
-            lock.unlock();
+            roleMap.unlock(deviceId);
         }
     }
 
     @Override
     public MastershipEvent relinquishRole(NodeId nodeId, DeviceId deviceId) {
-        byte [] did = serialize(deviceId);
-        byte [] nid = serialize(nodeId);
         MastershipEvent event = null;
 
-        ILock lock = theInstance.getLock(LOCK);
-        lock.lock();
+        roleMap.lock(deviceId);
         try {
+            RoleValue rv = getRoleValue(deviceId);
             MastershipRole role = getRole(nodeId, deviceId);
             switch (role) {
                 case MASTER:
                     event = reelect(nodeId, deviceId);
-                    evict(nid, did);
-                    break;
+                    //fall through to reinforce relinquishment
                 case STANDBY:
                     //fall through to reinforce relinquishment
                 case NONE:
-                    evict(nid, did);
+                    rv.reassign(nodeId, STANDBY, NONE);
                     break;
                 default:
                 log.warn("unknown Mastership Role {}", role);
             }
             return event;
         } finally {
-            lock.unlock();
+            roleMap.unlock(deviceId);
         }
     }
 
     //helper to fetch a new master candidate for a given device.
     private MastershipEvent reelect(NodeId current, DeviceId deviceId) {
-        byte [] did = serialize(deviceId);
-        byte [] nid = serialize(current);
+        RoleValue rv = roleMap.get(deviceId);
 
         //if this is an queue it'd be neater.
-        byte [] backup = null;
-        for (byte [] n : standbys.get(serialize(deviceId))) {
-            if (!current.equals(deserialize(n))) {
+        NodeId backup = null;
+        for (NodeId n : rv.nodesOfRole(STANDBY)) {
+            if (!current.equals(n)) {
                 backup = n;
                 break;
             }
         }
 
         if (backup == null) {
-            masters.remove(did, nid);
+            rv.remove(MASTER, current);
             return null;
         } else {
-            masters.put(did, backup);
-            evict(backup, did);
-            Integer term = terms.get(did);
-            terms.put(did, ++term);
+            rv.replace(current, backup, MASTER);
+            rv.reassign(backup, STANDBY, NONE);
+            Integer term = terms.get(deviceId);
+            terms.put(deviceId, ++term);
             return new MastershipEvent(
-                    MASTER_CHANGED, deviceId, (NodeId) deserialize(backup));
+                    MASTER_CHANGED, deviceId, backup);
         }
     }
 
-    //adds node to pool(s) of backups and moves them from unusable.
-    private void backup(byte [] nodeId, byte [] deviceId) {
-        if (!standbys.containsEntry(deviceId, nodeId)) {
-            standbys.put(deviceId, nodeId);
-        }
-        if (unusable.containsEntry(deviceId, nodeId)) {
-            unusable.remove(deviceId, nodeId);
+    //return the RoleValue structure for a device, or create one
+    private RoleValue getRoleValue(DeviceId deviceId) {
+        RoleValue value = roleMap.get(deviceId);
+        if (value == null) {
+            value = new RoleValue();
+            roleMap.put(deviceId, value);
         }
+        return value;
     }
 
-    //adds node to unusable and evicts it from backup pool.
-    private void evict(byte [] nodeId, byte [] deviceId) {
-        if (!unusable.containsEntry(deviceId, nodeId)) {
-            unusable.put(deviceId, nodeId);
+    //get first applicable node out of store-unique structure.
+    private NodeId getNode(MastershipRole role, DeviceId deviceId) {
+        RoleValue value = roleMap.get(deviceId);
+        if (value != null) {
+            return value.get(role);
         }
-        if (standbys.containsEntry(deviceId, nodeId)) {
-            standbys.remove(deviceId, nodeId);
+        return null;
+    }
+
+    //check if node is a certain role given a device
+    private boolean isRole(
+            MastershipRole role, NodeId nodeId, DeviceId deviceId) {
+        RoleValue value = roleMap.get(deviceId);
+        if (value != null) {
+            return value.contains(role, nodeId);
         }
+        return false;
     }
 
     //adds or updates term information.
-    private void updateTerm(byte [] deviceId) {
+    private void updateTerm(DeviceId deviceId) {
         Integer term = terms.get(deviceId);
         if (term == null) {
             terms.put(deviceId, INIT);

+package org.onlab.onos.store.mastership.impl;
+
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+import org.onlab.onos.cluster.NodeId;
+import org.onlab.onos.net.MastershipRole;
+
+/**
+ * A structure that holds node mastership roles associated with a
+ * {@link DeviceId}. This structure needs to be locked through IMap.
+ */
+public class RoleValue {
+
+    Map<MastershipRole, List<NodeId>> value;
+
+    public RoleValue() {
+        value.put(MastershipRole.MASTER, new LinkedList<NodeId>());
+        value.put(MastershipRole.STANDBY, new LinkedList<NodeId>());
+        value.put(MastershipRole.NONE, new LinkedList<NodeId>());
+    }
+
+    public Map<MastershipRole, List<NodeId>> value() {
+        return Collections.unmodifiableMap(value);
+    }
+
+    public List<NodeId> nodesOfRole(MastershipRole type) {
+        return value.get(type);
+    }
+
+    public NodeId get(MastershipRole type) {
+        return value.get(type).isEmpty() ? null : value.get(type).get(0);
+    }
+
+    public boolean contains(MastershipRole type, NodeId nodeId) {
+        return value.get(type).contains(nodeId);
+    }
+
+    /**
+     * Associates a node to a certain role.
+     *
+     * @param type the role
+     * @param nodeId the node ID of the node to associate
+     */
+    public void add(MastershipRole type, NodeId nodeId) {
+        List<NodeId> nodes = value.get(type);
+
+        if (!nodes.contains(nodeId)) {
+            nodes.add(nodeId);
+        }
+    }
+
+    /**
+     * Removes a node from a certain role.
+     *
+     * @param type the role
+     * @param nodeId the ID of the node to remove
+     * @return
+     */
+    public boolean remove(MastershipRole type, NodeId nodeId) {
+        List<NodeId> nodes = value.get(type);
+        if (!nodes.isEmpty()) {
+            return nodes.remove(nodeId);
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Reassigns a node from one role to another. If the node was not of the
+     * old role, it will still be assigned the new role.
+     *
+     * @param nodeId the Node ID of node changing roles
+     * @param from the old role
+     * @param to the new role
+     */
+    // might want to add anyways as default behavior
+    public void reassign(NodeId nodeId, MastershipRole from, MastershipRole to) {
+        remove(from, nodeId);
+        add(to, nodeId);
+    }
+
+    /**
+     * Replaces a node in one role with another node. Even if there is no node to
+     * replace, the new node is associated to the role.
+     *
+     * @param from the old NodeId to replace
+     * @param to the new NodeId
+     * @param type the role associated with the old NodeId
+     */
+    // might want to add anyways as default behavior
+    public void replace(NodeId from, NodeId to, MastershipRole type) {
+        remove(type, from);
+        add(type, to);
+    }
+
+}

@@ -27,6 +27,7 @@ import org.onlab.onos.mastership.MastershipStoreDelegate;
 import org.onlab.onos.mastership.MastershipTerm;
 import org.onlab.onos.mastership.MastershipEvent.Type;
 import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.store.common.StoreManager;
 import org.onlab.onos.store.common.StoreService;
 import org.onlab.onos.store.common.TestStoreManager;
@@ -101,7 +102,7 @@ public class DistributedMastershipStoreTest {
 
     @Test
     public void getMaster() {
-        assertTrue("wrong store state:", dms.masters.isEmpty());
+        assertTrue("wrong store state:", dms.roleMap.isEmpty());
 
         testStore.put(DID1, N1, true, false, false);
         assertEquals("wrong master:", N1, dms.getMaster(DID1));
@@ -110,7 +111,7 @@ public class DistributedMastershipStoreTest {
 
     @Test
     public void getDevices() {
-        assertTrue("wrong store state:", dms.masters.isEmpty());
+        assertTrue("wrong store state:", dms.roleMap.isEmpty());
 
         testStore.put(DID1, N1, true, false, false);
         testStore.put(DID2, N1, true, false, false);
@@ -161,7 +162,7 @@ public class DistributedMastershipStoreTest {
         assertEquals("wrong event:", Type.MASTER_CHANGED, dms.setMaster(N2, DID2).type());
         assertEquals("wrong term", MastershipTerm.of(N2, 0), dms.getTermFor(DID2));
         //disconnect and reconnect - sign of failing re-election or single-instance channel
-        testStore.reset(true, false, false);
+        dms.roleMap.clear();
         dms.setMaster(N2, DID2);
         assertEquals("wrong term", MastershipTerm.of(N2, 1), dms.getTermFor(DID2));
     }
@@ -191,13 +192,15 @@ public class DistributedMastershipStoreTest {
         assertEquals("wrong role for node:", NONE, dms.getRole(N2, DID1));
         assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID1));
 
-        assertEquals("wrong number of retired nodes", 2, dms.unusable.size());
+        assertEquals("wrong number of retired nodes", 2,
+                dms.roleMap.get(DID1).nodesOfRole(NONE).size());
 
         //bring nodes back
         assertEquals("wrong role for NONE:", MASTER, dms.requestRole(DID1));
         testStore.setCurrent(CN1);
         assertEquals("wrong role for NONE:", STANDBY, dms.requestRole(DID1));
-        assertEquals("wrong number of backup nodes", 1, dms.standbys.size());
+        assertEquals("wrong number of backup nodes", 1,
+                dms.roleMap.get(DID1).nodesOfRole(STANDBY).size());
 
         //NONE - nothing happens
         assertNull("wrong event:", dms.relinquishRole(N1, DID2));
@@ -238,55 +241,44 @@ public class DistributedMastershipStoreTest {
         //helper to populate master/backup structures
         public void put(DeviceId dev, NodeId node,
                 boolean master, boolean backup, boolean term) {
-            byte [] n = serialize(node);
-            byte [] d = serialize(dev);
+            RoleValue rv = dms.roleMap.get(dev);
+            if (rv == null) {
+                rv = new RoleValue();
+                dms.roleMap.put(dev, rv);
+            }
 
             if (master) {
-                dms.masters.put(d, n);
-                dms.unusable.put(d, n);
-                dms.standbys.remove(d, n);
+                rv.add(MASTER, node);
+                rv.reassign(node, STANDBY, NONE);
             }
             if (backup) {
-                dms.standbys.put(d, n);
-                dms.masters.remove(d, n);
-                dms.unusable.remove(d, n);
+                rv.add(STANDBY, node);
+                rv.remove(MASTER, node);
+                rv.remove(NONE, node);
             }
             if (term) {
-                dms.terms.put(d, 0);
+                dms.terms.put(dev, 0);
             }
         }
 
         //a dumb utility function.
         public void dump() {
-            System.out.println("standbys");
-            for (Map.Entry<byte [], byte []> e : standbys.entrySet()) {
-                System.out.println(deserialize(e.getKey()) + ":" + deserialize(e.getValue()));
-            }
-            System.out.println("unusable");
-            for (Map.Entry<byte [], byte []> e : unusable.entrySet()) {
-                System.out.println(deserialize(e.getKey()) + ":" + deserialize(e.getValue()));
-            }
-        }
-
-        //clears structures
-        public void reset(boolean store, boolean backup, boolean term) {
-            if (store) {
-                dms.masters.clear();
-                dms.unusable.clear();
-            }
-            if (backup) {
-                dms.standbys.clear();
-            }
-            if (term) {
-                dms.terms.clear();
+            for (Map.Entry<DeviceId, RoleValue> el : dms.roleMap.entrySet()) {
+                System.out.println("DID: " + el.getKey());
+                for (MastershipRole role : MastershipRole.values()) {
+                    System.out.println(role.toString() + ":");
+                    for (NodeId n : el.getValue().nodesOfRole(role)) {
+                        System.out.println("\t" + n);
+                    }
+                }
             }
         }
 
         //increment term for a device
         public void increment(DeviceId dev) {
-            Integer t = dms.terms.get(serialize(dev));
+            Integer t = dms.terms.get(dev);
             if (t != null) {
-                dms.terms.put(serialize(dev), ++t);
+                dms.terms.put(dev, ++t);
             }
         }
 

@@ -19,7 +19,6 @@ import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.Element;
 import org.onlab.onos.net.Link;
 import org.onlab.onos.net.LinkKey;
-import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.net.Port;
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.device.DefaultDeviceDescription;
@@ -27,6 +26,7 @@ import org.onlab.onos.net.device.DefaultPortDescription;
 import org.onlab.onos.net.link.DefaultLinkDescription;
 import org.onlab.onos.net.provider.ProviderId;
 import org.onlab.onos.store.Timestamp;
+import org.onlab.onos.store.mastership.impl.RoleValue;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.util.KryoPool;
@@ -66,7 +66,7 @@ public final class KryoPoolUtil {
                     DefaultDevice.class,
                     DefaultDeviceDescription.class,
                     DefaultLinkDescription.class,
-                    MastershipRole.class,
+                    RoleValue.class,
                     Port.class,
                     DefaultPortDescription.class,
                     Element.class,
@@ -84,7 +84,7 @@ public final class KryoPoolUtil {
             .register(ConnectPoint.class, new ConnectPointSerializer())
             .register(DefaultLink.class, new DefaultLinkSerializer())
             .register(MastershipTerm.class, new MastershipTermSerializer())
-            .register(MastershipRole.class, new MastershipRoleSerializer())
+            .register(RoleValue.class, new RoleValueSerializer())
 
             .build();
 

+package org.onlab.onos.store.serializers;
+
+import java.util.List;
+import java.util.Map;
+
+import org.onlab.onos.cluster.NodeId;
+import org.onlab.onos.net.MastershipRole;
+import org.onlab.onos.store.mastership.impl.RoleValue;
+
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.Serializer;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+
+/**
+ * Serializer for RoleValues used by {@link DistributedMastershipStore}
+ */
+public class RoleValueSerializer extends Serializer<RoleValue> {
+
+    //RoleValues are assumed to hold a Map of MastershipRoles (an enum)
+    //to a List of NodeIds.
+
+    @Override
+    public RoleValue read(Kryo kryo, Input input, Class<RoleValue> type) {
+        RoleValue rv = new RoleValue();
+        int size = input.readInt();
+        for (int i = 0; i < size; i++) {
+            MastershipRole role = MastershipRole.values()[input.readInt()];
+            int s = input.readInt();
+            for (int j = 0; j < s; j++) {
+                rv.add(role, new NodeId(input.readString()));
+            }
+        }
+        return rv;
+    }
+
+    @Override
+    public void write(Kryo kryo, Output output, RoleValue type) {
+        output.writeInt(type.value().size());
+
+        for (Map.Entry<MastershipRole, List<NodeId>> el :
+                type.value().entrySet()) {
+            output.writeInt(el.getKey().ordinal());
+
+            List<NodeId> nodes = el.getValue();
+            output.writeInt(nodes.size());
+            for (NodeId n : nodes) {
+                output.writeString(n.toString());
+            }
+        }
+    }
+
+}

@@ -26,6 +26,7 @@ import org.onlab.onos.net.MastershipRole;
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.SparseAnnotations;
 import org.onlab.onos.net.provider.ProviderId;
+import org.onlab.onos.store.mastership.impl.RoleValue;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.util.KryoPool;
@@ -58,6 +59,13 @@ public class KryoSerializerTest {
             .remove("A1")
             .set("B3", "b3")
             .build();
+    private static final RoleValue RV = new RoleValue();
+    static {
+        RV.add(MastershipRole.MASTER, new NodeId("node1"));
+        RV.add(MastershipRole.STANDBY, new NodeId("node2"));
+        RV.add(MastershipRole.STANDBY, new NodeId("node3"));
+        RV.add(MastershipRole.NONE, new NodeId("node4"));
+    };
 
     private static KryoPool kryos;
 

@@ -2,6 +2,7 @@ package org.onlab.onos.store.trivial.impl;
 
 import static org.slf4j.LoggerFactory.getLogger;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -97,7 +98,14 @@ public class SimpleMastershipStore
 
     @Override
     public List<NodeId> getNodes(DeviceId deviceId) {
-        return null;
+        List<NodeId> nodes = new ArrayList<>();
+
+        nodes.addAll(backups);
+        if (!nodes.contains(masterMap.get(deviceId))) {
+            nodes.add(masterMap.get(deviceId));
+        }
+
+        return Collections.unmodifiableList(nodes);
     }
 
     @Override