Fix for duplicated event issue.
1) Waits for the end of the leader election;
2) Partially reverts 18417 - there is no always the need
to notify the delegate;
3) Reverts 18899, since 19904 partially reverted 16278
which was the original reason for 18899;
4) Removes offline timestamp after an admin-removal
patch2: introduces 4) to fix the issues of the previous patch
patch1: duplicated events are still present after an admin
removal (remove or wipe-out please) - the issue is not related
to the changes reported above.
Change-Id: I6e6b2ce21b08d08e8188dea362a904e6dc5505d4
diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
index 97d6e7b..b538a75 100644
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
@@ -167,13 +167,16 @@
descs.putDeviceDesc(deviceDescription);
Device newDevice = composeDevice(deviceId, providerDescs);
+ DeviceEvent event = null;
if (oldDevice == null) {
// ADD
- return createDevice(providerId, newDevice);
+ event = createDevice(providerId, newDevice);
} else {
// UPDATE or ignore (no change or stale)
- return updateDevice(providerId, oldDevice, newDevice);
+ event = updateDevice(providerId, oldDevice, newDevice);
}
+ notifyDelegateIfNotNull(event);
+ return event;
}
}
@@ -661,6 +664,12 @@
portDesc.portSpeed(), annotations);
}
+ private void notifyDelegateIfNotNull(DeviceEvent event) {
+ if (event != null) {
+ notifyDelegate(event);
+ }
+ }
+
/**
* @return primary ProviderID, or randomly chosen one if none exists
*/
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index be24991..b114d0c 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
+import com.google.common.util.concurrent.Futures;
import org.onlab.util.KryoNamespace;
import org.onlab.util.Tools;
import org.onosproject.cluster.ClusterService;
@@ -388,7 +389,6 @@
DeviceEvent event = store.removeDevice(deviceId);
if (event != null) {
log.info("Device {} administratively removed", deviceId);
- post(event);
}
}
@@ -588,10 +588,12 @@
deviceDescription = deviceAnnotationOp.combine(deviceId, deviceDescription, Optional.of(annoConfig));
}
- MastershipRole role = mastershipService.requestRoleForSync(deviceId);
+ // Wait for the end of the election. sync call of requestRoleFor
+ // wait only 3s and it is not entirely safe since the leadership
+ // election timer can be higher.
+ MastershipRole role = Futures.getUnchecked(mastershipService.requestRoleFor(deviceId));
log.info("Local role is {} for {}", role, deviceId);
- DeviceEvent event = store.createOrUpdateDevice(provider().id(), deviceId,
- deviceDescription);
+ store.createOrUpdateDevice(provider().id(), deviceId, deviceDescription);
applyRole(deviceId, role);
if (portConfig != null) {
@@ -610,11 +612,6 @@
} else {
log.info("Device {} registered", deviceId);
}
-
- if (event != null) {
- log.trace("event: {} {}", event.type(), event);
- post(event);
- }
}
private PortDescription ensurePortEnabledState(PortDescription desc, boolean enabled) {
@@ -1123,7 +1120,7 @@
(dev == null) ? null : BasicDeviceOperator.descriptionOf(dev);
desc = BasicDeviceOperator.combine(cfg, desc);
if (desc != null && dp != null) {
- de = store.createOrUpdateDevice(dp.id(), did, desc);
+ store.createOrUpdateDevice(dp.id(), did, desc);
}
}
} else if (event.configClass().equals(PortDescriptionsConfig.class)) {
@@ -1149,7 +1146,7 @@
Optional<Config> prevConfig = event.prevConfig();
desc = deviceAnnotationOp.combine(did, desc, prevConfig);
if (desc != null && dp != null) {
- de = store.createOrUpdateDevice(dp.id(), did, desc);
+ store.createOrUpdateDevice(dp.id(), did, desc);
}
} else if (portOpsIndex.containsKey(event.configClass())) {
ConnectPoint cpt = (ConnectPoint) event.subject();
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
index c30e399..885d6b0 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
@@ -330,11 +330,12 @@
mergedDesc = device.get(providerId).getDeviceDesc();
}
- // FIXME: This may result in duplicate events as each instance reports on the new device
- // regardless of whether it is a master or not.
- log.debug("Notifying peers of a device update topology event for providerId: {} and deviceId: {}",
- providerId, deviceId);
- notifyPeers(new InternalDeviceEvent(providerId, deviceId, mergedDesc));
+ // If this node is the master for the device, update peers.
+ if (isMaster) {
+ log.debug("Notifying peers of a device update topology event for providerId: {} and deviceId: {}",
+ providerId, deviceId);
+ notifyPeers(new InternalDeviceEvent(providerId, deviceId, mergedDesc));
+ }
notifyDelegateIfNotNull(deviceEvent);
return deviceEvent;
@@ -1074,8 +1075,8 @@
log.debug("Notifying peers of a device removed topology event for deviceId: {}",
deviceId);
notifyPeers(new InternalDeviceRemovedEvent(deviceId, timestamp));
- notifyDelegateIfNotNull(event);
}
+ notifyDelegateIfNotNull(event);
// Relinquish mastership if acquired to remove the device.
if (relinquishAtEnd) {
@@ -1117,6 +1118,8 @@
}
markOfflineInternal(deviceId, timestamp);
descs.clear();
+ // Forget about the device
+ offline.remove(deviceId);
return device == null ? null :
new DeviceEvent(DeviceEvent.Type.DEVICE_REMOVED, device, null);
}