backport GossipDeviceStore changes back to trivial.
Change-Id: Ic21835d65f1902769f3ab51d3c9c6174c4e3a16b
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
index 514a22e..48e259b 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
@@ -5,8 +5,6 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
-import org.apache.commons.lang3.concurrent.ConcurrentException;
-import org.apache.commons.lang3.concurrent.ConcurrentInitializer;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -35,6 +33,7 @@
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -71,8 +70,7 @@
public static final String DEVICE_NOT_FOUND = "Device with ID %s not found";
// collection of Description given from various providers
- private final ConcurrentMap<DeviceId,
- ConcurrentMap<ProviderId, DeviceDescriptions>>
+ private final ConcurrentMap<DeviceId, Map<ProviderId, DeviceDescriptions>>
deviceDescs = Maps.newConcurrentMap();
// cache of Device and Ports generated by compositing descriptions from providers
@@ -117,15 +115,16 @@
DeviceId deviceId,
DeviceDescription deviceDescription) {
- ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
- = getDeviceDescriptions(deviceId);
+ Map<ProviderId, DeviceDescriptions> providerDescs
+ = getOrCreateDeviceDescriptions(deviceId);
synchronized (providerDescs) {
// locking per device
DeviceDescriptions descs
- = createIfAbsentUnchecked(providerDescs, providerId,
- new InitDeviceDescs(deviceDescription));
+ = getOrCreateProviderDeviceDescriptions(providerDescs,
+ providerId,
+ deviceDescription);
Device oldDevice = devices.get(deviceId);
// update description
@@ -192,8 +191,8 @@
@Override
public DeviceEvent markOffline(DeviceId deviceId) {
- ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
- = getDeviceDescriptions(deviceId);
+ Map<ProviderId, DeviceDescriptions> providerDescs
+ = getOrCreateDeviceDescriptions(deviceId);
// locking device
synchronized (providerDescs) {
@@ -218,7 +217,7 @@
Device device = devices.get(deviceId);
checkArgument(device != null, DEVICE_NOT_FOUND, deviceId);
- ConcurrentMap<ProviderId, DeviceDescriptions> descsMap = deviceDescs.get(deviceId);
+ Map<ProviderId, DeviceDescriptions> descsMap = deviceDescs.get(deviceId);
checkArgument(descsMap != null, DEVICE_NOT_FOUND, deviceId);
List<DeviceEvent> events = new ArrayList<>();
@@ -287,12 +286,12 @@
Map<PortNumber, Port> ports,
Set<PortNumber> processed) {
List<DeviceEvent> events = new ArrayList<>();
- Iterator<PortNumber> iterator = ports.keySet().iterator();
+ Iterator<Entry<PortNumber, Port>> iterator = ports.entrySet().iterator();
while (iterator.hasNext()) {
- PortNumber portNumber = iterator.next();
+ Entry<PortNumber, Port> e = iterator.next();
+ PortNumber portNumber = e.getKey();
if (!processed.contains(portNumber)) {
- events.add(new DeviceEvent(PORT_REMOVED, device,
- ports.get(portNumber)));
+ events.add(new DeviceEvent(PORT_REMOVED, device, e.getValue()));
iterator.remove();
}
}
@@ -306,10 +305,36 @@
NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
}
- private ConcurrentMap<ProviderId, DeviceDescriptions> getDeviceDescriptions(
+ private Map<ProviderId, DeviceDescriptions> getOrCreateDeviceDescriptions(
DeviceId deviceId) {
- return createIfAbsentUnchecked(deviceDescs, deviceId,
- NewConcurrentHashMap.<ProviderId, DeviceDescriptions>ifNeeded());
+ Map<ProviderId, DeviceDescriptions> r;
+ r = deviceDescs.get(deviceId);
+ if (r != null) {
+ return r;
+ }
+ r = new HashMap<>();
+ final Map<ProviderId, DeviceDescriptions> concurrentlyAdded;
+ concurrentlyAdded = deviceDescs.putIfAbsent(deviceId, r);
+ if (concurrentlyAdded != null) {
+ return concurrentlyAdded;
+ } else {
+ return r;
+ }
+ }
+
+ // Guarded by deviceDescs value (=Device lock)
+ private DeviceDescriptions getOrCreateProviderDeviceDescriptions(
+ Map<ProviderId, DeviceDescriptions> device,
+ ProviderId providerId, DeviceDescription deltaDesc) {
+
+ synchronized (device) {
+ DeviceDescriptions r = device.get(providerId);
+ if (r == null) {
+ r = new DeviceDescriptions(deltaDesc);
+ device.put(providerId, r);
+ }
+ return r;
+ }
}
@Override
@@ -318,7 +343,7 @@
Device device = devices.get(deviceId);
checkArgument(device != null, DEVICE_NOT_FOUND, deviceId);
- ConcurrentMap<ProviderId, DeviceDescriptions> descsMap = deviceDescs.get(deviceId);
+ Map<ProviderId, DeviceDescriptions> descsMap = deviceDescs.get(deviceId);
checkArgument(descsMap != null, DEVICE_NOT_FOUND, deviceId);
synchronized (descsMap) {
@@ -367,7 +392,7 @@
@Override
public DeviceEvent removeDevice(DeviceId deviceId) {
- ConcurrentMap<ProviderId, DeviceDescriptions> descs = getDeviceDescriptions(deviceId);
+ Map<ProviderId, DeviceDescriptions> descs = getOrCreateDeviceDescriptions(deviceId);
synchronized (descs) {
Device device = devices.remove(deviceId);
// should DEVICE_REMOVED carry removed ports?
@@ -390,7 +415,7 @@
* @return Device instance
*/
private Device composeDevice(DeviceId deviceId,
- ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs) {
+ Map<ProviderId, DeviceDescriptions> providerDescs) {
checkArgument(!providerDescs.isEmpty(), "No Device descriptions supplied");
@@ -429,14 +454,14 @@
*
* @param device device the port is on
* @param number port number
- * @param providerDescs Collection of Descriptions from multiple providers
+ * @param descsMap Collection of Descriptions from multiple providers
* @return Port instance
*/
private Port composePort(Device device, PortNumber number,
- ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs) {
+ Map<ProviderId, DeviceDescriptions> descsMap) {
- ProviderId primary = pickPrimaryPID(providerDescs);
- DeviceDescriptions primDescs = providerDescs.get(primary);
+ ProviderId primary = pickPrimaryPID(descsMap);
+ DeviceDescriptions primDescs = descsMap.get(primary);
// if no primary, assume not enabled
// TODO: revisit this default port enabled/disabled behavior
boolean isEnabled = false;
@@ -448,7 +473,7 @@
annotations = merge(annotations, portDesc.annotations());
}
- for (Entry<ProviderId, DeviceDescriptions> e : providerDescs.entrySet()) {
+ for (Entry<ProviderId, DeviceDescriptions> e : descsMap.entrySet()) {
if (e.getKey().equals(primary)) {
continue;
}
@@ -470,10 +495,9 @@
/**
* @return primary ProviderID, or randomly chosen one if none exists
*/
- private ProviderId pickPrimaryPID(
- ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs) {
+ private ProviderId pickPrimaryPID(Map<ProviderId, DeviceDescriptions> descsMap) {
ProviderId fallBackPrimary = null;
- for (Entry<ProviderId, DeviceDescriptions> e : providerDescs.entrySet()) {
+ for (Entry<ProviderId, DeviceDescriptions> e : descsMap.entrySet()) {
if (!e.getKey().isAncillary()) {
return e.getKey();
} else if (fallBackPrimary == null) {
@@ -484,21 +508,6 @@
return fallBackPrimary;
}
- public static final class InitDeviceDescs
- implements ConcurrentInitializer<DeviceDescriptions> {
-
- private final DeviceDescription deviceDesc;
-
- public InitDeviceDescs(DeviceDescription deviceDesc) {
- this.deviceDesc = checkNotNull(deviceDesc);
- }
- @Override
- public DeviceDescriptions get() throws ConcurrentException {
- return new DeviceDescriptions(deviceDesc);
- }
- }
-
-
/**
* Collection of Description of a Device and it's Ports given from a Provider.
*/