avoid transient changes to MastershipStore from being posted as events
Change-Id: Id033cf50f865e44439138f5b3814ebaedb832b73
diff --git a/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java b/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
index fff6333..8411b09 100644
--- a/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
+++ b/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
@@ -47,4 +47,11 @@
*/
void roleChanged(Device device, MastershipRole newRole);
+ /**
+ * Checks the reachability (connectivity) of a device from this provider.
+ *
+ * @param device device to check
+ * @return true if reachable, false otherwise
+ */
+ boolean isReachable(Device device);
}
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 d3cce22..6180ada 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
@@ -97,10 +97,20 @@
checkNotNull(role, ROLE_NULL);
MastershipEvent event = null;
- if (role.equals(MastershipRole.MASTER)) {
- event = store.setMaster(nodeId, deviceId);
- } else {
- event = store.setStandby(nodeId, deviceId);
+
+ switch (role) {
+ case MASTER:
+ event = store.setMaster(nodeId, deviceId);
+ break;
+ case STANDBY:
+ event = store.setStandby(nodeId, deviceId);
+ break;
+ case NONE:
+ event = store.relinquishRole(nodeId, deviceId);
+ break;
+ default:
+ log.info("Unknown role; ignoring");
+ return;
}
if (event != null) {
@@ -259,6 +269,10 @@
@Override
public void notify(MastershipEvent event) {
+ if (clusterService.getLocalNode().id().equals(event.roleInfo().master())) {
+ log.info("ignoring locally-generated event {}", event);
+ // return;
+ }
log.info("dispatching mastership event {}", event);
eventDispatcher.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 3559c4b..e284052 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
@@ -159,32 +159,37 @@
// Applies the specified role to the device; ignores NONE
private void applyRole(DeviceId deviceId, MastershipRole newRole) {
- if (!newRole.equals(MastershipRole.NONE)) {
- Device device = store.getDevice(deviceId);
- // FIXME: Device might not be there yet. (eventual consistent)
- if (device == null) {
- return;
- }
- DeviceProvider provider = getProvider(device.providerId());
- if (provider != null) {
- provider.roleChanged(device, newRole);
-
- // only trigger event when request was sent to provider
- // TODO: consider removing this from Device event type?
- post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device));
- }
+ if (newRole.equals(MastershipRole.NONE)) {
+ return;
}
- }
- // Queries a device for port information.
- private void queryPortInfo(DeviceId deviceId) {
Device device = store.getDevice(deviceId);
// FIXME: Device might not be there yet. (eventual consistent)
if (device == null) {
return;
}
+
DeviceProvider provider = getProvider(device.providerId());
- provider.triggerProbe(device);
+ if (provider != null) {
+ provider.roleChanged(device, newRole);
+ // only trigger event when request was sent to provider
+ // TODO: consider removing this from Device event type?
+ post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device));
+
+ if (newRole.equals(MastershipRole.MASTER)) {
+ provider.triggerProbe(device);
+ }
+ }
+ }
+
+ // Check a device for control channel connectivity.
+ private boolean isReachable(Device device) {
+ // FIXME: Device might not be there yet. (eventual consistent)
+ if (device == null) {
+ return false;
+ }
+ DeviceProvider provider = getProvider(device.providerId());
+ return provider.isReachable(device);
}
@Override
@@ -236,7 +241,6 @@
log.info("Device {} connected", deviceId);
// check my Role
MastershipRole role = mastershipService.requestRoleFor(deviceId);
- log.info("requestedRole, became {} for {}", role, deviceId);
if (role != MastershipRole.MASTER) {
// TODO: Do we need to explicitly tell the Provider that
// this instance is no longer the MASTER? probably not
@@ -405,14 +409,16 @@
// only set the new term if I am the master
deviceClockProviderService.setMastershipTerm(did, term);
- // FIXME: we should check that the device is connected on our end.
- // currently, this is not straight forward as the actual switch
- // implementation is hidden from the registry. Maybe we can ask the
- // provider.
// if the device is null here, we are the first master to claim the
// device. No worries, the DeviceManager will create one soon.
Device device = getDevice(did);
if ((device != null) && !isAvailable(did)) {
+ if (!isReachable(device)) {
+ log.warn("Device {} has disconnected after this event", did);
+ mastershipService.relinquishMastership(did);
+ applyRole(did, MastershipRole.STANDBY);
+ return;
+ }
//flag the device as online. Is there a better way to do this?
DeviceEvent devEvent = store.createOrUpdateDevice(device.providerId(), did,
new DefaultDeviceDescription(
@@ -422,9 +428,11 @@
post(devEvent);
}
applyRole(did, MastershipRole.MASTER);
- // re-collect device information to fix potential staleness
- queryPortInfo(did);
} else if (event.roleInfo().backups().contains(myNodeId)) {
+ if (!isReachable(getDevice(did))) {
+ log.warn("Device {} has disconnected after this event", did);
+ 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 d079378..25fa922 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
@@ -278,6 +278,11 @@
deviceReceived = device;
roleReceived = newRole;
}
+
+ @Override
+ public boolean isReachable(Device device) {
+ return false;
+ }
}
private static class TestListener implements DeviceListener {
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
index 75c0405..d1917d6 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
@@ -272,6 +272,10 @@
switch (role) {
case MASTER:
event = reelect(nodeId, deviceId, rv);
+ if (event != null) {
+ Integer term = terms.get(deviceId);
+ terms.put(deviceId, ++term);
+ }
//fall through to reinforce relinquishment
case STANDBY:
//fall through to reinforce relinquishment
@@ -304,15 +308,11 @@
if (backup == null) {
log.info("{} giving up and going to NONE for {}", current, deviceId);
rv.remove(MASTER, current);
- roleMap.put(deviceId, rv);
return null;
} else {
log.info("{} trying to pass mastership for {} to {}", current, deviceId, backup);
rv.replace(current, backup, MASTER);
rv.reassign(backup, STANDBY, NONE);
- roleMap.put(deviceId, rv);
- Integer term = terms.get(deviceId);
- terms.put(deviceId, ++term);
return new MastershipEvent(MASTER_CHANGED, deviceId, rv.roleInfo());
}
}
@@ -366,7 +366,7 @@
@Override
public void entryUpdated(EntryEvent<DeviceId, RoleValue> event) {
-
+ // this subsumes entryAdded event
notifyDelegate(new MastershipEvent(
MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
}