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 fc8e9e4..ddb25eb 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
@@ -165,13 +165,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;
}
}
@@ -659,6 +662,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 5c634d1..15ed053 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
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
+import com.google.common.util.concurrent.Futures;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -384,7 +385,6 @@
DeviceEvent event = store.removeDevice(deviceId);
if (event != null) {
log.info("Device {} administratively removed", deviceId);
- post(event);
}
}
@@ -555,10 +555,12 @@
// Generate updated description and establish my Role
deviceDescription = BasicDeviceOperator.combine(cfg, deviceDescription);
- 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) {
@@ -577,11 +579,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) {
@@ -1039,7 +1036,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)) {
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 5b26a91..0c01933 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
@@ -322,11 +322,13 @@
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;
}
@@ -1055,6 +1057,7 @@
deviceId);
notifyPeers(new InternalDeviceRemovedEvent(deviceId, timestamp));
}
+ notifyDelegateIfNotNull(event);
// Relinquish mastership if acquired to remove the device.
if (relinquishAtEnd) {
@@ -1096,6 +1099,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);
}