backport GossipDeviceStore changes back to trivial.

Change-Id: Ic21835d65f1902769f3ab51d3c9c6174c4e3a16b

backport GossipDeviceStore changes back to trivial.
Change-Id: Ic21835d65f1902769f3ab51d3c9c6174c4e3a16b
Yuta HIGUCHI
Commit 2c546a1ef85dd507bf7ecfcdb09996c70ef91eea 2c546a1e 1 parent eada0715
Showing 1 changed file with 53 additions and 44 deletions
core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
View file @2c546a1
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
View file @2c546a1
@@ -5,8 +5,6 @@ import com.google.common.collect.ImmutableList;
 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 org.slf4j.Logger;
 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 class SimpleDeviceStore
     public static final String DEVICE_NOT_FOUND = "Device with ID %s not found";
     // collection of Description given from various providers
-    private final ConcurrentMap<DeviceId,
+    private final ConcurrentMap<DeviceId, Map<ProviderId, DeviceDescriptions>>
-                            ConcurrentMap<ProviderId, DeviceDescriptions>>
                                 deviceDescs = Maps.newConcurrentMap();
     // cache of Device and Ports generated by compositing descriptions from providers
@@ -117,15 +115,16 @@ public class SimpleDeviceStore
                                      DeviceId deviceId,
                                      DeviceDescription deviceDescription) {
-        ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
+        Map<ProviderId, DeviceDescriptions> providerDescs
-            = getDeviceDescriptions(deviceId);
+            = getOrCreateDeviceDescriptions(deviceId);
         synchronized (providerDescs) {
             // locking per device
             DeviceDescriptions descs
-                = createIfAbsentUnchecked(providerDescs, providerId,
+                = getOrCreateProviderDeviceDescriptions(providerDescs,
-                        new InitDeviceDescs(deviceDescription));
+                                                        providerId,
+                                                        deviceDescription);
             Device oldDevice = devices.get(deviceId);
             // update description
@@ -192,8 +191,8 @@ public class SimpleDeviceStore
     @Override
     public DeviceEvent markOffline(DeviceId deviceId) {
-        ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
+        Map<ProviderId, DeviceDescriptions> providerDescs
-            = getDeviceDescriptions(deviceId);
+            = getOrCreateDeviceDescriptions(deviceId);
         // locking device
         synchronized (providerDescs) {
@@ -218,7 +217,7 @@ public class SimpleDeviceStore
         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 @@ public class SimpleDeviceStore
                                             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,
+                events.add(new DeviceEvent(PORT_REMOVED, device, e.getValue()));
-                                           ports.get(portNumber)));
                 iterator.remove();
             }
         }
@@ -306,10 +305,36 @@ public class SimpleDeviceStore
                 NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
     }
-    private ConcurrentMap<ProviderId, DeviceDescriptions> getDeviceDescriptions(
+    private Map<ProviderId, DeviceDescriptions> getOrCreateDeviceDescriptions(
             DeviceId deviceId) {
-        return createIfAbsentUnchecked(deviceDescs, deviceId,
+        Map<ProviderId, DeviceDescriptions> r;
-                NewConcurrentHashMap.<ProviderId, DeviceDescriptions>ifNeeded());
+        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 @@ public class SimpleDeviceStore
         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 @@ public class SimpleDeviceStore
     @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 @@ public class SimpleDeviceStore
      * @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 @@ public class SimpleDeviceStore
      *
      * @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);
+        ProviderId primary = pickPrimaryPID(descsMap);
-        DeviceDescriptions primDescs = providerDescs.get(primary);
+        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 @@ public class SimpleDeviceStore
             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 @@ public class SimpleDeviceStore
     /**
      * @return primary ProviderID, or randomly chosen one if none exists
      */
-    private ProviderId pickPrimaryPID(
+    private ProviderId pickPrimaryPID(Map<ProviderId, DeviceDescriptions> descsMap) {
-            ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs) {
         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 @@ public class SimpleDeviceStore
         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.
      */