avoid transient changes to MastershipStore from being posted as events
Change-Id: Id033cf50f865e44439138f5b3814ebaedb832b73
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 {