role reassignment tweaks
Change-Id: Ie6d412787330e67a13e605a34f0824cf70882f85
diff --git a/core/api/src/main/java/org/onlab/onos/cluster/MastershipService.java b/core/api/src/main/java/org/onlab/onos/cluster/MastershipService.java
index d417516..ffee162 100644
--- a/core/api/src/main/java/org/onlab/onos/cluster/MastershipService.java
+++ b/core/api/src/main/java/org/onlab/onos/cluster/MastershipService.java
@@ -33,7 +33,7 @@
/**
* Abandons mastership of the specified device on the local node thus
* forcing selection of a new master. If the local node is not a master
- * for this device, no action will be taken.
+ * for this device, no master selection will occur.
*
* @param deviceId the identifier of the device
*/
diff --git a/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java b/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java
index bedc5e9..dc5603f 100644
--- a/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java
+++ b/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java
@@ -66,12 +66,25 @@
MastershipTerm getTermFor(DeviceId deviceId);
/**
- * Revokes a controller instance's mastership over a device and hands
- * over mastership to another controller instance.
+ * Sets a controller instance's mastership role to STANDBY for a device.
+ * If the role is MASTER, another controller instance will be selected
+ * as a candidate master.
*
* @param nodeId the controller instance identifier
- * @param deviceId device to revoke mastership for
+ * @param deviceId device to revoke mastership role for
* @return a mastership event
*/
- MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId);
+ MastershipEvent setStandby(NodeId nodeId, DeviceId deviceId);
+
+ /**
+ * Allows a controller instance to give up its current role for a device.
+ * If the role is MASTER, another controller instance will be selected
+ * as a candidate master.
+ *
+ * @param nodeId the controller instance identifier
+ * @param deviceId device to revoke mastership role for
+ * @return a mastership event
+ */
+ MastershipEvent relinquishRole(NodeId nodeId, DeviceId deviceId);
+
}
diff --git a/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java b/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java
index a0da87c..92b345c 100644
--- a/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java
+++ b/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java
@@ -82,7 +82,7 @@
if (role.equals(MastershipRole.MASTER)) {
event = store.setMaster(nodeId, deviceId);
} else {
- event = store.unsetMaster(nodeId, deviceId);
+ event = store.setStandby(nodeId, deviceId);
}
if (event != null) {
@@ -98,13 +98,10 @@
@Override
public void relinquishMastership(DeviceId deviceId) {
- MastershipRole role = getLocalRole(deviceId);
- if (!role.equals(MastershipRole.MASTER)) {
- return;
- }
+ MastershipEvent event = null;
+ event = store.relinquishRole(
+ clusterService.getLocalNode().id(), deviceId);
- MastershipEvent event = store.unsetMaster(
- clusterService.getLocalNode().id(), deviceId);
if (event != null) {
post(event);
}
diff --git a/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
index bb221da..5400fb0 100644
--- a/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
@@ -142,7 +142,7 @@
// Applies the specified role to the device; ignores NONE
private void applyRole(DeviceId deviceId, MastershipRole newRole) {
- if (newRole != MastershipRole.NONE) {
+ if (newRole.equals(MastershipRole.NONE)) {
Device device = store.getDevice(deviceId);
DeviceProvider provider = getProvider(device.providerId());
if (provider != null) {
@@ -196,11 +196,8 @@
DeviceEvent event = store.createOrUpdateDevice(provider().id(),
deviceId, deviceDescription);
- // If there was a change of any kind, trigger role selection
- // process.
if (event != null) {
log.info("Device {} connected", deviceId);
- //mastershipService.requestRoleFor(deviceId);
provider().roleChanged(event.subject(),
mastershipService.requestRoleFor(deviceId));
post(event);
@@ -212,11 +209,11 @@
checkNotNull(deviceId, DEVICE_ID_NULL);
checkValidity();
DeviceEvent event = store.markOffline(deviceId);
+ //we're no longer capable of being master or a candidate.
+ mastershipService.relinquishMastership(deviceId);
- //we're no longer capable of mastership.
if (event != null) {
log.info("Device {} disconnected", deviceId);
- mastershipService.relinquishMastership(deviceId);
post(event);
}
}
@@ -267,17 +264,23 @@
}
// Intercepts mastership events
- private class InternalMastershipListener
- implements MastershipListener {
+ private class InternalMastershipListener implements MastershipListener {
+
@Override
public void event(MastershipEvent event) {
- if (event.master().equals(clusterService.getLocalNode().id())) {
- MastershipTerm term = mastershipService.requestTermService()
- .getMastershipTerm(event.subject());
- clockService.setMastershipTerm(event.subject(), term);
- applyRole(event.subject(), MastershipRole.MASTER);
+ DeviceId did = event.subject();
+ if (isAvailable(did)) {
+ if (event.master().equals(clusterService.getLocalNode().id())) {
+ MastershipTerm term = termService.getMastershipTerm(did);
+ clockService.setMastershipTerm(did, term);
+ applyRole(did, MastershipRole.MASTER);
+ } else {
+ applyRole(did, MastershipRole.STANDBY);
+ }
} else {
- applyRole(event.subject(), MastershipRole.STANDBY);
+ //device dead to node, give up
+ mastershipService.relinquishMastership(did);
+ applyRole(did, MastershipRole.STANDBY);
}
}
}
diff --git a/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java b/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
index 7d09cca..ea47b0c 100644
--- a/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
@@ -1,6 +1,7 @@
package org.onlab.onos.net.device.impl;
import com.google.common.collect.Sets;
+
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
@@ -258,7 +259,8 @@
}
}
- private static class TestMastershipService extends MastershipServiceAdapter {
+ private static class TestMastershipService
+ extends MastershipServiceAdapter {
@Override
public MastershipRole getLocalRole(DeviceId deviceId) {
return MastershipRole.MASTER;
diff --git a/core/store/hz/cluster/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java b/core/store/hz/cluster/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java
index 04833e6..71d42fa 100644
--- a/core/store/hz/cluster/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java
+++ b/core/store/hz/cluster/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java
@@ -10,7 +10,6 @@
import org.apache.felix.scr.annotations.Deactivate;
import org.apache.felix.scr.annotations.Reference;
import org.apache.felix.scr.annotations.ReferenceCardinality;
-import org.apache.felix.scr.annotations.ReferencePolicy;
import org.apache.felix.scr.annotations.Service;
import org.onlab.onos.cluster.ClusterService;
import org.onlab.onos.cluster.MastershipEvent;
@@ -20,15 +19,16 @@
import org.onlab.onos.cluster.NodeId;
import org.onlab.onos.net.DeviceId;
import org.onlab.onos.net.MastershipRole;
-import org.onlab.onos.net.device.DeviceService;
import org.onlab.onos.store.common.AbstractHazelcastStore;
import com.google.common.collect.ImmutableSet;
import com.hazelcast.core.ILock;
import com.hazelcast.core.IMap;
+import com.hazelcast.core.MultiMap;
/**
- * Distributed implementation of the cluster nodes store.
+ * Distributed implementation of the mastership store. The store is
+ * responsible for the master selection process.
*/
@Component(immediate = true)
@Service
@@ -38,35 +38,34 @@
//arbitrary lock name
private static final String LOCK = "lock";
- //initial term value
+ //initial term/TTL value
private static final Integer INIT = 0;
- //placeholder non-null value
- private static final Byte NIL = 0x0;
//devices to masters
- protected IMap<byte[], byte[]> rawMasters;
+ protected IMap<byte[], byte[]> masters;
//devices to terms
- protected IMap<byte[], Integer> rawTerms;
- //collection of nodes. values are ignored, as it's used as a makeshift 'set'
- protected IMap<byte[], Byte> backups;
+ 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;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected ClusterService clusterService;
- //FIXME: need to guarantee that this will be met, sans circular dependencies
- @Reference(policy = ReferencePolicy.DYNAMIC)
- protected DeviceService deviceService;
-
@Override
@Activate
public void activate() {
super.activate();
- rawMasters = theInstance.getMap("masters");
- rawTerms = theInstance.getMap("terms");
- backups = theInstance.getMap("backups");
+ masters = theInstance.getMap("masters");
+ terms = theInstance.getMap("terms");
+ standbys = theInstance.getMultiMap("backups");
+ unusable = theInstance.getMultiMap("unusable");
- rawMasters.addEntryListener(new RemoteMasterShipEventHandler(), true);
+ masters.addEntryListener(new RemoteMasterShipEventHandler(), true);
log.info("Started");
}
@@ -77,6 +76,30 @@
}
@Override
+ public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
+ byte[] did = serialize(deviceId);
+ byte[] nid = serialize(nodeId);
+
+ NodeId current = deserialize(masters.get(did));
+ if (current == null) {
+ if (standbys.containsEntry(did, nid)) {
+ //was previously standby, or set to standby from master
+ return MastershipRole.STANDBY;
+ } else {
+ return MastershipRole.NONE;
+ }
+ } else {
+ if (current.equals(nodeId)) {
+ //*should* be in unusable, not always
+ return MastershipRole.MASTER;
+ } else {
+ //may be in backups or unusable from earlier retirement
+ return MastershipRole.STANDBY;
+ }
+ }
+ }
+
+ @Override
public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
byte [] did = serialize(deviceId);
byte [] nid = serialize(nodeId);
@@ -85,30 +108,31 @@
lock.lock();
try {
MastershipRole role = getRole(nodeId, deviceId);
- Integer term = rawTerms.get(did);
switch (role) {
case MASTER:
+ //reinforce mastership
+ evict(nid, did);
return null;
case STANDBY:
- rawMasters.put(did, nid);
- rawTerms.put(did, ++term);
- backups.putIfAbsent(nid, NIL);
- break;
- case NONE:
- rawMasters.put(did, nid);
- //new switch OR state transition after being orphaned
- if (term == null) {
- rawTerms.put(did, INIT);
- } else {
- rawTerms.put(did, ++term);
+ //make current master standby
+ byte [] current = masters.get(did);
+ if (current != null) {
+ backup(current, did);
}
- backups.put(nid, NIL);
- break;
+ //assign specified node as new master
+ masters.put(did, nid);
+ evict(nid, did);
+ updateTerm(did);
+ return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
+ case NONE:
+ masters.put(did, nid);
+ evict(nid, did);
+ updateTerm(did);
+ return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
default:
log.warn("unknown Mastership Role {}", role);
return null;
}
- return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
} finally {
lock.unlock();
}
@@ -116,14 +140,14 @@
@Override
public NodeId getMaster(DeviceId deviceId) {
- return deserialize(rawMasters.get(serialize(deviceId)));
+ return deserialize(masters.get(serialize(deviceId)));
}
@Override
public Set<DeviceId> getDevices(NodeId nodeId) {
ImmutableSet.Builder<DeviceId> builder = ImmutableSet.builder();
- for (Map.Entry<byte[], byte[]> entry : rawMasters.entrySet()) {
+ for (Map.Entry<byte[], byte[]> entry : masters.entrySet()) {
if (nodeId.equals(deserialize(entry.getValue()))) {
builder.add((DeviceId) deserialize(entry.getKey()));
}
@@ -134,11 +158,8 @@
@Override
public MastershipRole requestRole(DeviceId deviceId) {
- // first to empty slot for device in master map is MASTER
- // depending on how backups are organized, might need to trigger election
- // so only controller doesn't set itself to backup for another device
- byte [] did = serialize(deviceId);
NodeId local = clusterService.getLocalNode().id();
+ byte [] did = serialize(deviceId);
byte [] lnid = serialize(local);
ILock lock = theInstance.getLock(LOCK);
@@ -147,15 +168,17 @@
MastershipRole role = getRole(local, deviceId);
switch (role) {
case MASTER:
+ evict(lnid, did);
break;
case STANDBY:
- backups.put(lnid, NIL);
- rawTerms.putIfAbsent(did, INIT);
+ backup(lnid, did);
+ terms.putIfAbsent(did, INIT);
break;
case NONE:
- rawMasters.put(did, lnid);
- rawTerms.putIfAbsent(did, INIT);
- backups.put(lnid, NIL);
+ //claim mastership
+ masters.put(did, lnid);
+ evict(lnid, did);
+ updateTerm(did);
role = MastershipRole.MASTER;
break;
default:
@@ -168,41 +191,21 @@
}
@Override
- public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
- byte[] did = serialize(deviceId);
-
- NodeId current = deserialize(rawMasters.get(did));
- MastershipRole role = null;
-
- if (current == null) {
- //IFF no controllers have claimed mastership over it
- role = MastershipRole.NONE;
- } else {
- if (current.equals(nodeId)) {
- role = MastershipRole.MASTER;
- } else {
- role = MastershipRole.STANDBY;
- }
- }
-
- return role;
- }
-
- @Override
public MastershipTerm getTermFor(DeviceId deviceId) {
byte[] did = serialize(deviceId);
-
- if ((rawMasters.get(did) == null) ||
- (rawTerms.get(did) == null)) {
+ if ((masters.get(did) == null) ||
+ (terms.get(did) == null)) {
return null;
}
return MastershipTerm.of(
- (NodeId) deserialize(rawMasters.get(did)), rawTerms.get(did));
+ (NodeId) deserialize(masters.get(did)), terms.get(did));
}
@Override
- public MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId) {
+ 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();
@@ -210,54 +213,113 @@
MastershipRole role = getRole(nodeId, deviceId);
switch (role) {
case MASTER:
- //hand off device to another
- NodeId backup = reelect(nodeId, deviceId);
- if (backup == null) {
- //goes back to NONE
- rawMasters.remove(did);
- } else {
- //goes to STANDBY for local, MASTER for someone else
- Integer term = rawTerms.get(did);
- rawMasters.put(did, serialize(backup));
- rawTerms.put(did, ++term);
- return new MastershipEvent(MASTER_CHANGED, deviceId, backup);
- }
+ event = reelect(nodeId, deviceId);
+ backup(nid, did);
+ break;
case STANDBY:
+ //fall through to reinforce role
case NONE:
+ backup(nid, did);
break;
default:
log.warn("unknown Mastership Role {}", role);
}
- return null;
+ return event;
} finally {
lock.unlock();
}
}
- //helper for "re-electing" a new master for a given device
- private NodeId reelect(NodeId current, DeviceId deviceId) {
+ @Override
+ public MastershipEvent relinquishRole(NodeId nodeId, DeviceId deviceId) {
+ byte [] did = serialize(deviceId);
+ byte [] nid = serialize(nodeId);
+ MastershipEvent event = null;
- for (byte [] node : backups.keySet()) {
- NodeId nid = deserialize(node);
- //if a device dies we shouldn't pick another master for it.
- if (!current.equals(nid) && (deviceService.isAvailable(deviceId))) {
- return nid;
+ ILock lock = theInstance.getLock(LOCK);
+ lock.lock();
+ try {
+ MastershipRole role = getRole(nodeId, deviceId);
+ switch (role) {
+ case MASTER:
+ event = reelect(nodeId, deviceId);
+ evict(nid, did);
+ break;
+ case STANDBY:
+ //fall through to reinforce relinquishment
+ case NONE:
+ evict(nid, did);
+ break;
+ default:
+ log.warn("unknown Mastership Role {}", role);
}
+ return event;
+ } finally {
+ lock.unlock();
}
- return null;
}
- //adds node to pool(s) of backup
- private void backup(NodeId nodeId, DeviceId deviceId) {
- //TODO might be useful to isolate out this function and reelect() if we
- //get more backup/election schemes
+ //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);
+
+ //if this is an queue it'd be neater.
+ byte [] backup = null;
+ for (byte [] n : standbys.get(serialize(deviceId))) {
+ if (!current.equals(deserialize(n))) {
+ backup = n;
+ break;
+ }
+ }
+
+ if (backup == null) {
+ masters.remove(did, nid);
+ return null;
+ } else {
+ masters.put(did, backup);
+ evict(backup, did);
+ Integer term = terms.get(did);
+ terms.put(did, ++term);
+ return new MastershipEvent(
+ MASTER_CHANGED, deviceId, (NodeId) deserialize(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);
+ }
+ }
+
+ //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);
+ }
+ if (standbys.containsEntry(deviceId, nodeId)) {
+ standbys.remove(deviceId, nodeId);
+ }
+ }
+
+ //adds or updates term information.
+ private void updateTerm(byte [] deviceId) {
+ Integer term = terms.get(deviceId);
+ if (term == null) {
+ terms.put(deviceId, INIT);
+ } else {
+ terms.put(deviceId, ++term);
+ }
}
private class RemoteMasterShipEventHandler extends RemoteEventHandler<DeviceId, NodeId> {
@Override
protected void onAdd(DeviceId deviceId, NodeId nodeId) {
- //only addition indicates a change in mastership
notifyDelegate(new MastershipEvent(MASTER_CHANGED, deviceId, nodeId));
}
@@ -268,6 +330,7 @@
@Override
protected void onUpdate(DeviceId deviceId, NodeId oldNodeId, NodeId nodeId) {
+ //only addition indicates a change in mastership
//notifyDelegate(new MastershipEvent(MASTER_CHANGED, deviceId, nodeId));
}
}
diff --git a/core/store/hz/cluster/src/test/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStoreTest.java b/core/store/hz/cluster/src/test/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStoreTest.java
index c3efdfe..9178e90 100644
--- a/core/store/hz/cluster/src/test/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStoreTest.java
+++ b/core/store/hz/cluster/src/test/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStoreTest.java
@@ -5,7 +5,7 @@
import static org.junit.Assert.assertTrue;
import static org.onlab.onos.net.MastershipRole.*;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -26,13 +26,7 @@
import org.onlab.onos.cluster.MastershipStoreDelegate;
import org.onlab.onos.cluster.MastershipTerm;
import org.onlab.onos.cluster.NodeId;
-import org.onlab.onos.net.Device;
import org.onlab.onos.net.DeviceId;
-import org.onlab.onos.net.MastershipRole;
-import org.onlab.onos.net.Port;
-import org.onlab.onos.net.PortNumber;
-import org.onlab.onos.net.device.DeviceListener;
-import org.onlab.onos.net.device.DeviceService;
import org.onlab.onos.store.common.StoreManager;
import org.onlab.onos.store.common.StoreService;
import org.onlab.onos.store.common.TestStoreManager;
@@ -87,7 +81,6 @@
dms = new TestDistributedMastershipStore(storeMgr, serializationMgr);
dms.clusterService = new TestClusterService();
- dms.deviceService = new TestDeviceService();
dms.activate();
testStore = (TestDistributedMastershipStore) dms;
@@ -105,14 +98,14 @@
@Test
public void getRole() {
assertEquals("wrong role:", NONE, dms.getRole(N1, DID1));
- testStore.put(DID1, N1, true, true, true);
+ testStore.put(DID1, N1, true, false, true);
assertEquals("wrong role:", MASTER, dms.getRole(N1, DID1));
assertEquals("wrong role:", STANDBY, dms.getRole(N2, DID1));
}
@Test
public void getMaster() {
- assertTrue("wrong store state:", dms.rawMasters.isEmpty());
+ assertTrue("wrong store state:", dms.masters.isEmpty());
testStore.put(DID1, N1, true, false, false);
assertEquals("wrong master:", N1, dms.getMaster(DID1));
@@ -121,7 +114,7 @@
@Test
public void getDevices() {
- assertTrue("wrong store state:", dms.rawMasters.isEmpty());
+ assertTrue("wrong store state:", dms.masters.isEmpty());
testStore.put(DID1, N1, true, false, false);
testStore.put(DID2, N1, true, false, false);
@@ -139,20 +132,17 @@
//if already MASTER, nothing should happen
testStore.put(DID2, N1, true, false, false);
assertEquals("wrong role for MASTER:", MASTER, dms.requestRole(DID2));
- assertTrue("wrong state for store:",
- dms.backups.isEmpty() & dms.rawTerms.isEmpty());
//populate maps with DID1, N1 thru NONE case
assertEquals("wrong role for NONE:", MASTER, dms.requestRole(DID1));
- assertTrue("wrong state for store:",
- !dms.backups.isEmpty() & !dms.rawTerms.isEmpty());
+ assertTrue("wrong state for store:", !dms.terms.isEmpty());
assertEquals("wrong term",
MastershipTerm.of(N1, 0), dms.getTermFor(DID1));
//CN2 now local. DID2 has N1 as MASTER so N2 is STANDBY
testStore.setCurrent(CN2);
assertEquals("wrong role for STANDBY:", STANDBY, dms.requestRole(DID2));
- assertEquals("wrong number of entries:", 2, dms.rawTerms.size());
+ assertEquals("wrong number of entries:", 2, dms.terms.size());
//change term and requestRole() again; should persist
testStore.increment(DID2);
@@ -181,34 +171,41 @@
}
@Test
- public void unsetMaster() {
+ public void relinquishRole() {
//populate maps with DID1, N1 as MASTER thru NONE case
testStore.setCurrent(CN1);
assertEquals("wrong role for NONE:", MASTER, dms.requestRole(DID1));
//no backup, no new MASTER/event
- assertNull("wrong event:", dms.unsetMaster(N1, DID1));
+ assertNull("wrong event:", dms.relinquishRole(N1, DID1));
dms.requestRole(DID1);
- ((TestDeviceService) dms.deviceService).active.add(DID1);
//add backup CN2, get it elected MASTER by relinquishing
testStore.setCurrent(CN2);
- dms.requestRole(DID1);
- assertEquals("wrong event:", Type.MASTER_CHANGED, dms.unsetMaster(N1, DID1).type());
+ assertEquals("wrong role for NONE:", STANDBY, dms.requestRole(DID1));
+ assertEquals("wrong event:", Type.MASTER_CHANGED, dms.relinquishRole(N1, DID1).type());
assertEquals("wrong master", N2, dms.getMaster(DID1));
//STANDBY - nothing here, either
- assertNull("wrong event:", dms.unsetMaster(N1, DID1));
+ assertNull("wrong event:", dms.relinquishRole(N1, DID1));
assertEquals("wrong role for node:", STANDBY, dms.getRole(N1, DID1));
- //NONE - nothing happens
- assertNull("wrong event:", dms.unsetMaster(N1, DID2));
- assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID2));
+ //all nodes "give up" on device, which goes back to NONE.
+ assertNull("wrong event:", dms.relinquishRole(N2, DID1));
+ assertEquals("wrong role for node:", NONE, dms.getRole(N2, DID1));
+ assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID1));
- //for a device that turned off (not active) - status to NONE
- ((TestDeviceService) dms.deviceService).active.clear();
- assertNull("extraneous event:", dms.unsetMaster(N2, DID1));
- assertEquals("wrong role", NONE, dms.getRole(N2, DID1));
+ assertEquals("wrong number of retired nodes", 2, dms.unusable.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());
+
+ //NONE - nothing happens
+ assertNull("wrong event:", dms.relinquishRole(N1, DID2));
+ assertEquals("wrong role for node:", NONE, dms.getRole(N1, DID2));
}
@@ -244,36 +241,55 @@
//helper to populate master/backup structures
public void put(DeviceId dev, NodeId node,
- boolean store, boolean backup, boolean term) {
- if (store) {
- dms.rawMasters.put(serialize(dev), serialize(node));
+ boolean master, boolean backup, boolean term) {
+ byte [] n = serialize(node);
+ byte [] d = serialize(dev);
+
+ if (master) {
+ dms.masters.put(d, n);
+ dms.unusable.put(d, n);
+ dms.standbys.remove(d, n);
}
if (backup) {
- dms.backups.put(serialize(node), (byte) 0);
+ dms.standbys.put(d, n);
+ dms.masters.remove(d, n);
+ dms.unusable.remove(d, n);
}
if (term) {
- dms.rawTerms.put(serialize(dev), 0);
+ dms.terms.put(d, 0);
+ }
+ }
+
+ 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.rawMasters.clear();
+ dms.masters.clear();
+ dms.unusable.clear();
}
if (backup) {
- dms.backups.clear();
+ dms.standbys.clear();
}
if (term) {
- dms.rawTerms.clear();
+ dms.terms.clear();
}
}
//increment term for a device
public void increment(DeviceId dev) {
- Integer t = dms.rawTerms.get(serialize(dev));
+ Integer t = dms.terms.get(serialize(dev));
if (t != null) {
- dms.rawTerms.put(serialize(dev), ++t);
+ dms.terms.put(serialize(dev), ++t);
}
}
@@ -317,52 +333,4 @@
}
- private class TestDeviceService implements DeviceService {
-
- Set<DeviceId> active = Sets.newHashSet();
-
- @Override
- public int getDeviceCount() {
- return 0;
- }
-
- @Override
- public Iterable<Device> getDevices() {
- return null;
- }
-
- @Override
- public Device getDevice(DeviceId deviceId) {
- return null;
- }
-
- @Override
- public MastershipRole getRole(DeviceId deviceId) {
- return null;
- }
-
- @Override
- public List<Port> getPorts(DeviceId deviceId) {
- return null;
- }
-
- @Override
- public Port getPort(DeviceId deviceId, PortNumber portNumber) {
- return null;
- }
-
- @Override
- public boolean isAvailable(DeviceId deviceId) {
- return active.contains(deviceId);
- }
-
- @Override
- public void addListener(DeviceListener listener) {
- }
-
- @Override
- public void removeListener(DeviceListener listener) {
- }
-
- }
}
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStore.java
index 0439d79..e8096ea 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStore.java
@@ -174,7 +174,7 @@
}
@Override
- public MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId) {
+ public MastershipEvent setStandby(NodeId nodeId, DeviceId deviceId) {
MastershipRole role = getRole(nodeId, deviceId);
synchronized (this) {
switch (role) {
@@ -214,4 +214,9 @@
return backup;
}
+ @Override
+ public MastershipEvent relinquishRole(NodeId nodeId, DeviceId deviceId) {
+ return setStandby(nodeId, deviceId);
+ }
+
}
diff --git a/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStoreTest.java b/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStoreTest.java
index 32d3d68..1e8e5c7 100644
--- a/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStoreTest.java
+++ b/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleMastershipStoreTest.java
@@ -129,22 +129,22 @@
public void unsetMaster() {
//NONE - record backup but take no other action
put(DID1, N1, false, false);
- sms.unsetMaster(N1, DID1);
+ sms.setStandby(N1, DID1);
assertTrue("not backed up", sms.backups.contains(N1));
sms.termMap.clear();
- sms.unsetMaster(N1, DID1);
+ sms.setStandby(N1, DID1);
assertTrue("term not set", sms.termMap.containsKey(DID1));
//no backup, MASTER
put(DID1, N1, true, true);
- assertNull("wrong event", sms.unsetMaster(N1, DID1));
+ assertNull("wrong event", sms.setStandby(N1, DID1));
assertNull("wrong node", sms.masterMap.get(DID1));
//backup, switch
sms.masterMap.clear();
put(DID1, N1, true, true);
put(DID2, N2, true, true);
- assertEquals("wrong event", MASTER_CHANGED, sms.unsetMaster(N1, DID1).type());
+ assertEquals("wrong event", MASTER_CHANGED, sms.setStandby(N1, DID1).type());
}
//helper to populate master/backup structures
diff --git a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OFChannelHandler.java b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OFChannelHandler.java
index 04aef8d..0a3ee86 100644
--- a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OFChannelHandler.java
+++ b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OFChannelHandler.java
@@ -981,11 +981,13 @@
// switch was a duplicate-dpid, calling the method below would clear
// all state for the original switch (with the same dpid),
// which we obviously don't want.
+ log.info("{}:removal called");
sw.removeConnectedSwitch();
} else {
// A duplicate was disconnected on this ChannelHandler,
// this is the same switch reconnecting, but the original state was
// not cleaned up - XXX check liveness of original ChannelHandler
+ log.info("{}:duplicate found");
duplicateDpidFound = Boolean.FALSE;
}
} else {
diff --git a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OpenFlowControllerImpl.java b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OpenFlowControllerImpl.java
index e8ebcd1..716f7ec 100644
--- a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OpenFlowControllerImpl.java
+++ b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/OpenFlowControllerImpl.java
@@ -307,9 +307,11 @@
connectedSwitches.remove(dpid);
OpenFlowSwitch sw = activeMasterSwitches.remove(dpid);
if (sw == null) {
+ log.warn("sw was null for {}", dpid);
sw = activeEqualSwitches.remove(dpid);
}
for (OpenFlowSwitchListener l : ofSwitchListener) {
+ log.warn("removal for {}", dpid);
l.switchRemoved(dpid);
}
}