Store new devices locally to prevent race conditions prior to master->standby replication
Change-Id: I37b7b886856475fbcb0a838df49420c042ec8386
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 b95066c..4dd45a8 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
@@ -75,6 +75,7 @@
import org.onosproject.store.Timestamp;
import org.onosproject.store.cluster.messaging.ClusterCommunicationService;
import org.onosproject.store.cluster.messaging.MessageSubject;
+import org.onosproject.store.impl.MastershipBasedTimestamp;
import org.onosproject.store.impl.Timestamped;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.serializers.StoreSerializer;
@@ -170,6 +171,7 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected MastershipTermService termService;
+ private static final Timestamp DEFAULT_TIMESTAMP = new MastershipBasedTimestamp(0, 0);
protected static final StoreSerializer SERIALIZER = StoreSerializer.using(KryoNamespace.newBuilder()
.register(DistributedStoreSerializers.STORE_COMMON)
@@ -302,28 +304,33 @@
DeviceDescription deviceDescription) {
NodeId localNode = clusterService.getLocalNode().id();
NodeId deviceNode = mastershipService.getMasterFor(deviceId);
+ final boolean isMaster = localNode.equals(deviceNode);
// Process device update only if we're the master,
// otherwise signal the actual master.
DeviceEvent deviceEvent = null;
- if (localNode.equals(deviceNode)) {
- final Timestamp newTimestamp = deviceClockService.getTimestamp(deviceId);
- final Timestamped<DeviceDescription> deltaDesc = new Timestamped<>(deviceDescription, newTimestamp);
- final Timestamped<DeviceDescription> mergedDesc;
- final Map<ProviderId, DeviceDescriptions> device = getOrCreateDeviceDescriptionsMap(deviceId);
+ // If this node is the master for the device, acquire a new timestamp. Otherwise,
+ // use a 0,0 or tombstone timestamp to create the device if it doesn't already exist.
+ final Timestamp newTimestamp = isMaster
+ ? deviceClockService.getTimestamp(deviceId)
+ : removalRequest.getOrDefault(deviceId, DEFAULT_TIMESTAMP);
+ final Timestamped<DeviceDescription> deltaDesc = new Timestamped<>(deviceDescription, newTimestamp);
+ final Timestamped<DeviceDescription> mergedDesc;
+ final Map<ProviderId, DeviceDescriptions> device = getOrCreateDeviceDescriptionsMap(deviceId);
- synchronized (device) {
- deviceEvent = createOrUpdateDeviceInternal(providerId, deviceId, deltaDesc);
- mergedDesc = device.get(providerId).getDeviceDesc();
- }
+ synchronized (device) {
+ deviceEvent = createOrUpdateDeviceInternal(providerId, deviceId, deltaDesc);
+ mergedDesc = device.get(providerId).getDeviceDesc();
+ }
+ // If this node is the master for the device, update peers.
+ if (isMaster) {
if (deviceEvent != null) {
log.debug("Notifying peers of a device update topology event for providerId: {} and deviceId: {}",
providerId, deviceId);
notifyPeers(new InternalDeviceEvent(providerId, deviceId, mergedDesc));
}
-
} else {
return null;
}
@@ -391,6 +398,7 @@
markOnline(newDevice.id(), timestamp);
}
+ log.debug("Device {} added", newDevice.id());
return new DeviceEvent(DeviceEvent.Type.DEVICE_ADDED, newDevice, null);
}
@@ -421,6 +429,7 @@
providerId, oldDevice, devices.get(newDevice.id()), newDevice);
}
+ log.debug("Device {} updated", newDevice.id());
event = new DeviceEvent(DeviceEvent.Type.DEVICE_UPDATED, newDevice, null);
}
@@ -1028,7 +1037,10 @@
}
}
- if (!myId.equals(master)) {
+ boolean isMaster = myId.equals(master);
+
+ // If this node is not the master, forward the request.
+ if (!isMaster) {
log.debug("{} has control of {}, forwarding remove request",
master, deviceId);
@@ -1037,20 +1049,21 @@
/* error log:
log.error("Failed to forward {} remove request to {}", deviceId, master, e);
*/
-
- // event will be triggered after master processes it.
- return null;
}
- // I have control..
+ // If this node is the master, get a timestamp. Otherwise, default to the current device timestamp.
+ Timestamp timestamp = isMaster ? deviceClockService.getTimestamp(deviceId) : null;
- Timestamp timestamp = deviceClockService.getTimestamp(deviceId);
DeviceEvent event = removeDeviceInternal(deviceId, timestamp);
- if (event != null) {
+
+ // If this node is the master, update peers.
+ if (isMaster && event != null) {
log.debug("Notifying peers of a device removed topology event for deviceId: {}",
deviceId);
notifyPeers(new InternalDeviceRemovedEvent(deviceId, timestamp));
}
+
+ // Relinquish mastership if acquired to remove the device.
if (relinquishAtEnd) {
log.debug("Relinquishing temporary role acquired for {}", deviceId);
mastershipService.relinquishMastership(deviceId);
@@ -1058,8 +1071,7 @@
return event;
}
- private DeviceEvent removeDeviceInternal(DeviceId deviceId,
- Timestamp timestamp) {
+ private DeviceEvent removeDeviceInternal(DeviceId deviceId, Timestamp timestamp) {
Map<ProviderId, DeviceDescriptions> descs = getOrCreateDeviceDescriptionsMap(deviceId);
synchronized (descs) {
@@ -1071,6 +1083,12 @@
}
Timestamp lastTimestamp = primDescs.getLatestTimestamp();
+
+ // If no timestamp is set, default the timestamp to the last timestamp for the device.
+ if (timestamp == null) {
+ timestamp = lastTimestamp;
+ }
+
if (timestamp.compareTo(lastTimestamp) <= 0) {
// outdated event ignore
return null;