relinquishes mastership when device disconnects
Change-Id: I1aecc8862ce297569c358e1deb5ddc5fb52d5dd3
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 20ebc40..a0da87c 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
@@ -78,22 +78,15 @@
checkNotNull(deviceId, DEVICE_ID_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 {
- 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 @@
@Override
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)) {
return;
}
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 eb409a5..f1567fd 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
@@ -202,7 +202,7 @@
log.info("Device {} connected", deviceId);
mastershipService.requestRoleFor(deviceId);
provider().roleChanged(event.subject(),
- mastershipService.getLocalRole(deviceId));
+ mastershipService.requestRoleFor(deviceId));
post(event);
}
}
diff --git a/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java b/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java
index fd67681..d14df9c 100644
--- a/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java
@@ -113,7 +113,6 @@
mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
mgr.setRole(NID_LOCAL, DEV_OTHER, STANDBY);
assertEquals("should be one device:", 1, mgr.getDevicesOf(NID_LOCAL).size());
-
//hand both devices to NID_LOCAL
mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER);
assertEquals("should be two devices:", 2, mgr.getDevicesOf(NID_LOCAL).size());
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java b/core/store/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java
index 6690707..09853fd 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java
@@ -5,7 +5,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
@@ -27,8 +26,6 @@
import org.onlab.packet.IpPrefix;
import org.slf4j.Logger;
-import com.google.common.collect.Lists;
-
import static org.onlab.onos.cluster.MastershipEvent.Type.*;
/**
@@ -50,8 +47,8 @@
//devices mapped to their masters, to emulate multiple nodes
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
protected final Map<DeviceId, AtomicInteger> termMap = new HashMap<>();
@@ -67,38 +64,28 @@
@Override
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) {
+ synchronized (this) {
+ switch (role) {
+ case MASTER:
+ return null;
+ case STANDBY:
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) {
- masterMap.put(deviceId, nodeId);
- backups.remove(nodeId);
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);
}
@@ -120,51 +107,61 @@
@Override
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
public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
+ //just query
NodeId current = masterMap.get(deviceId);
- List<NodeId> backups = backupMap.get(deviceId);
+ MastershipRole role;
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;
+ //degenerate case - only node is its own backup
+ if (backups.contains(nodeId)) {
+ role = MastershipRole.STANDBY;
+ } else {
+ role = MastershipRole.NONE;
}
-
- //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);
- }
-
- } else if (current.equals(nodeId)) {
- return MastershipRole.MASTER;
} 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
@@ -179,43 +176,43 @@
@Override
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);
- 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;
}
- //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;
}
}