Yuta HIGUCHI

Adding some tests for GossipDeviceStore + bugfix

Change-Id: Ic0d55fa499b1d66131f059b4a47cd105c55a6e63
...@@ -62,6 +62,11 @@ ...@@ -62,6 +62,11 @@
62 <groupId>org.apache.commons</groupId> 62 <groupId>org.apache.commons</groupId>
63 <artifactId>commons-lang3</artifactId> 63 <artifactId>commons-lang3</artifactId>
64 </dependency> 64 </dependency>
65 + <dependency>
66 + <groupId>org.easymock</groupId>
67 + <artifactId>easymock</artifactId>
68 + <scope>test</scope>
69 + </dependency>
65 </dependencies> 70 </dependencies>
66 71
67 <build> 72 <build>
......
...@@ -58,7 +58,7 @@ class DeviceDescriptions { ...@@ -58,7 +58,7 @@ class DeviceDescriptions {
58 * 58 *
59 * @param newDesc new DeviceDescription 59 * @param newDesc new DeviceDescription
60 */ 60 */
61 - public synchronized void putDeviceDesc(Timestamped<DeviceDescription> newDesc) { 61 + public void putDeviceDesc(Timestamped<DeviceDescription> newDesc) {
62 Timestamped<DeviceDescription> oldOne = deviceDesc; 62 Timestamped<DeviceDescription> oldOne = deviceDesc;
63 Timestamped<DeviceDescription> newOne = newDesc; 63 Timestamped<DeviceDescription> newOne = newDesc;
64 if (oldOne != null) { 64 if (oldOne != null) {
...@@ -76,7 +76,7 @@ class DeviceDescriptions { ...@@ -76,7 +76,7 @@ class DeviceDescriptions {
76 * 76 *
77 * @param newDesc new PortDescription 77 * @param newDesc new PortDescription
78 */ 78 */
79 - public synchronized void putPortDesc(Timestamped<PortDescription> newDesc) { 79 + public void putPortDesc(Timestamped<PortDescription> newDesc) {
80 Timestamped<PortDescription> oldOne = portDescs.get(newDesc.value().portNumber()); 80 Timestamped<PortDescription> oldOne = portDescs.get(newDesc.value().portNumber());
81 Timestamped<PortDescription> newOne = newDesc; 81 Timestamped<PortDescription> newOne = newDesc;
82 if (oldOne != null) { 82 if (oldOne != null) {
......
1 package org.onlab.onos.store.device.impl; 1 package org.onlab.onos.store.device.impl;
2 2
3 +import com.google.common.base.Function;
3 import com.google.common.collect.FluentIterable; 4 import com.google.common.collect.FluentIterable;
4 import com.google.common.collect.ImmutableList; 5 import com.google.common.collect.ImmutableList;
5 import com.google.common.collect.Maps; 6 import com.google.common.collect.Maps;
...@@ -118,7 +119,7 @@ public class GossipDeviceStore ...@@ -118,7 +119,7 @@ public class GossipDeviceStore
118 @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) 119 @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
119 protected ClusterService clusterService; 120 protected ClusterService clusterService;
120 121
121 - private static final KryoSerializer SERIALIZER = new KryoSerializer() { 122 + protected static final KryoSerializer SERIALIZER = new KryoSerializer() {
122 @Override 123 @Override
123 protected void setupKryoPool() { 124 protected void setupKryoPool() {
124 serializerPool = KryoPool.newBuilder() 125 serializerPool = KryoPool.newBuilder()
...@@ -206,14 +207,19 @@ public class GossipDeviceStore ...@@ -206,14 +207,19 @@ public class GossipDeviceStore
206 public synchronized DeviceEvent createOrUpdateDevice(ProviderId providerId, 207 public synchronized DeviceEvent createOrUpdateDevice(ProviderId providerId,
207 DeviceId deviceId, 208 DeviceId deviceId,
208 DeviceDescription deviceDescription) { 209 DeviceDescription deviceDescription) {
209 - Timestamp newTimestamp = clockService.getTimestamp(deviceId); 210 + final Timestamp newTimestamp = clockService.getTimestamp(deviceId);
210 final Timestamped<DeviceDescription> deltaDesc = new Timestamped<>(deviceDescription, newTimestamp); 211 final Timestamped<DeviceDescription> deltaDesc = new Timestamped<>(deviceDescription, newTimestamp);
211 - DeviceEvent event = createOrUpdateDeviceInternal(providerId, deviceId, deltaDesc); 212 + final DeviceEvent event;
213 + final Timestamped<DeviceDescription> mergedDesc;
214 + synchronized (getDeviceDescriptions(deviceId)) {
215 + event = createOrUpdateDeviceInternal(providerId, deviceId, deltaDesc);
216 + mergedDesc = getDeviceDescriptions(deviceId).get(providerId).getDeviceDesc();
217 + }
212 if (event != null) { 218 if (event != null) {
213 log.info("Notifying peers of a device update topology event for providerId: {} and deviceId: {}", 219 log.info("Notifying peers of a device update topology event for providerId: {} and deviceId: {}",
214 providerId, deviceId); 220 providerId, deviceId);
215 try { 221 try {
216 - notifyPeers(new InternalDeviceEvent(providerId, deviceId, deltaDesc)); 222 + notifyPeers(new InternalDeviceEvent(providerId, deviceId, mergedDesc));
217 } catch (IOException e) { 223 } catch (IOException e) {
218 log.error("Failed to notify peers of a device update topology event for providerId: " 224 log.error("Failed to notify peers of a device update topology event for providerId: "
219 + providerId + " and deviceId: " + deviceId, e); 225 + providerId + " and deviceId: " + deviceId, e);
...@@ -317,8 +323,8 @@ public class GossipDeviceStore ...@@ -317,8 +323,8 @@ public class GossipDeviceStore
317 323
318 @Override 324 @Override
319 public DeviceEvent markOffline(DeviceId deviceId) { 325 public DeviceEvent markOffline(DeviceId deviceId) {
320 - Timestamp timestamp = clockService.getTimestamp(deviceId); 326 + final Timestamp timestamp = clockService.getTimestamp(deviceId);
321 - DeviceEvent event = markOfflineInternal(deviceId, timestamp); 327 + final DeviceEvent event = markOfflineInternal(deviceId, timestamp);
322 if (event != null) { 328 if (event != null) {
323 log.info("Notifying peers of a device offline topology event for deviceId: {}", 329 log.info("Notifying peers of a device offline topology event for deviceId: {}",
324 deviceId); 330 deviceId);
...@@ -390,17 +396,33 @@ public class GossipDeviceStore ...@@ -390,17 +396,33 @@ public class GossipDeviceStore
390 public synchronized List<DeviceEvent> updatePorts(ProviderId providerId, 396 public synchronized List<DeviceEvent> updatePorts(ProviderId providerId,
391 DeviceId deviceId, 397 DeviceId deviceId,
392 List<PortDescription> portDescriptions) { 398 List<PortDescription> portDescriptions) {
393 - Timestamp newTimestamp = clockService.getTimestamp(deviceId);
394 399
395 - Timestamped<List<PortDescription>> timestampedPortDescriptions = 400 + final Timestamp newTimestamp = clockService.getTimestamp(deviceId);
396 - new Timestamped<>(portDescriptions, newTimestamp); 401 +
397 - 402 + final Timestamped<List<PortDescription>> timestampedInput
398 - List<DeviceEvent> events = updatePortsInternal(providerId, deviceId, timestampedPortDescriptions); 403 + = new Timestamped<>(portDescriptions, newTimestamp);
404 + final List<DeviceEvent> events;
405 + final Timestamped<List<PortDescription>> merged;
406 +
407 + synchronized (getDeviceDescriptions(deviceId)) {
408 + events = updatePortsInternal(providerId, deviceId, timestampedInput);
409 + final DeviceDescriptions descs = getDeviceDescriptions(deviceId).get(providerId);
410 + List<PortDescription> mergedList =
411 + FluentIterable.from(portDescriptions)
412 + .transform(new Function<PortDescription, PortDescription>() {
413 + @Override
414 + public PortDescription apply(PortDescription input) {
415 + // lookup merged port description
416 + return descs.getPortDesc(input.portNumber()).value();
417 + }
418 + }).toList();
419 + merged = new Timestamped<List<PortDescription>>(mergedList, newTimestamp);
420 + }
399 if (!events.isEmpty()) { 421 if (!events.isEmpty()) {
400 log.info("Notifying peers of a port update topology event for providerId: {} and deviceId: {}", 422 log.info("Notifying peers of a port update topology event for providerId: {} and deviceId: {}",
401 providerId, deviceId); 423 providerId, deviceId);
402 try { 424 try {
403 - notifyPeers(new InternalPortEvent(providerId, deviceId, timestampedPortDescriptions)); 425 + notifyPeers(new InternalPortEvent(providerId, deviceId, merged));
404 } catch (IOException e) { 426 } catch (IOException e) {
405 log.error("Failed to notify peers of a port update topology event or providerId: " 427 log.error("Failed to notify peers of a port update topology event or providerId: "
406 + providerId + " and deviceId: " + deviceId, e); 428 + providerId + " and deviceId: " + deviceId, e);
...@@ -527,16 +549,25 @@ public class GossipDeviceStore ...@@ -527,16 +549,25 @@ public class GossipDeviceStore
527 } 549 }
528 550
529 @Override 551 @Override
530 - public synchronized DeviceEvent updatePortStatus(ProviderId providerId, DeviceId deviceId, 552 + public synchronized DeviceEvent updatePortStatus(ProviderId providerId,
531 - PortDescription portDescription) { 553 + DeviceId deviceId,
532 - Timestamp newTimestamp = clockService.getTimestamp(deviceId); 554 + PortDescription portDescription) {
533 - final Timestamped<PortDescription> deltaDesc = new Timestamped<>(portDescription, newTimestamp); 555 +
534 - DeviceEvent event = updatePortStatusInternal(providerId, deviceId, deltaDesc); 556 + final Timestamp newTimestamp = clockService.getTimestamp(deviceId);
557 + final Timestamped<PortDescription> deltaDesc
558 + = new Timestamped<>(portDescription, newTimestamp);
559 + final DeviceEvent event;
560 + final Timestamped<PortDescription> mergedDesc;
561 + synchronized (getDeviceDescriptions(deviceId)) {
562 + event = updatePortStatusInternal(providerId, deviceId, deltaDesc);
563 + mergedDesc = getDeviceDescriptions(deviceId).get(providerId)
564 + .getPortDesc(portDescription.portNumber());
565 + }
535 if (event != null) { 566 if (event != null) {
536 log.info("Notifying peers of a port status update topology event for providerId: {} and deviceId: {}", 567 log.info("Notifying peers of a port status update topology event for providerId: {} and deviceId: {}",
537 providerId, deviceId); 568 providerId, deviceId);
538 try { 569 try {
539 - notifyPeers(new InternalPortStatusEvent(providerId, deviceId, deltaDesc)); 570 + notifyPeers(new InternalPortStatusEvent(providerId, deviceId, mergedDesc));
540 } catch (IOException e) { 571 } catch (IOException e) {
541 log.error("Failed to notify peers of a port status update topology event or providerId: " 572 log.error("Failed to notify peers of a port status update topology event or providerId: "
542 + providerId + " and deviceId: " + deviceId, e); 573 + providerId + " and deviceId: " + deviceId, e);
...@@ -684,7 +715,7 @@ public class GossipDeviceStore ...@@ -684,7 +715,7 @@ public class GossipDeviceStore
684 * @return Device instance 715 * @return Device instance
685 */ 716 */
686 private Device composeDevice(DeviceId deviceId, 717 private Device composeDevice(DeviceId deviceId,
687 - ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs) { 718 + Map<ProviderId, DeviceDescriptions> providerDescs) {
688 719
689 checkArgument(!providerDescs.isEmpty(), "No Device descriptions supplied"); 720 checkArgument(!providerDescs.isEmpty(), "No Device descriptions supplied");
690 721
......
1 +package org.onlab.onos.store.serializers;
2 +
3 +import org.onlab.util.KryoPool.FamilySerializer;
4 +
5 +import com.esotericsoftware.kryo.Kryo;
6 +import com.esotericsoftware.kryo.io.Input;
7 +import com.esotericsoftware.kryo.io.Output;
8 +import com.google.common.collect.ImmutableList;
9 +import com.google.common.collect.ImmutableList.Builder;
10 +
11 +/**
12 + * Creates {@link ImmutableList} serializer instance.
13 + */
14 +public class ImmutableListSerializer extends FamilySerializer<ImmutableList<?>> {
15 +
16 + /**
17 + * Creates {@link ImmutableList} serializer instance.
18 + */
19 + public ImmutableListSerializer() {
20 + // non-null, immutable
21 + super(false, true);
22 + }
23 + @Override
24 + public void write(Kryo kryo, Output output, ImmutableList<?> object) {
25 + output.writeInt(object.size());
26 + for (Object e : object) {
27 + kryo.writeClassAndObject(output, e);
28 + }
29 + }
30 +
31 + @Override
32 + public ImmutableList<?> read(Kryo kryo, Input input,
33 + Class<ImmutableList<?>> type) {
34 + final int size = input.readInt();
35 + Builder<Object> builder = ImmutableList.builder();
36 + for (int i = 0; i < size; ++i) {
37 + builder.add(kryo.readClassAndObject(input));
38 + }
39 + return builder.build();
40 + }
41 +
42 + @Override
43 + public void registerFamilies(Kryo kryo) {
44 + kryo.register(ImmutableList.of(1).getClass(), this);
45 + kryo.register(ImmutableList.of(1, 2).getClass(), this);
46 + // TODO register required ImmutableList variants
47 + }
48 +
49 +}
...@@ -31,6 +31,9 @@ import org.onlab.packet.IpAddress; ...@@ -31,6 +31,9 @@ import org.onlab.packet.IpAddress;
31 import org.onlab.packet.IpPrefix; 31 import org.onlab.packet.IpPrefix;
32 import org.onlab.util.KryoPool; 32 import org.onlab.util.KryoPool;
33 33
34 +import com.google.common.collect.ImmutableList;
35 +import com.google.common.collect.ImmutableMap;
36 +
34 public final class KryoPoolUtil { 37 public final class KryoPoolUtil {
35 38
36 /** 39 /**
...@@ -47,12 +50,15 @@ public final class KryoPoolUtil { ...@@ -47,12 +50,15 @@ public final class KryoPoolUtil {
47 */ 50 */
48 public static final KryoPool API = KryoPool.newBuilder() 51 public static final KryoPool API = KryoPool.newBuilder()
49 .register(MISC) 52 .register(MISC)
53 + .register(ImmutableMap.class, new ImmutableMapSerializer())
54 + .register(ImmutableList.class, new ImmutableListSerializer())
50 .register( 55 .register(
51 // 56 //
52 ArrayList.class, 57 ArrayList.class,
53 Arrays.asList().getClass(), 58 Arrays.asList().getClass(),
54 HashMap.class, 59 HashMap.class,
55 // 60 //
61 + //
56 ControllerNode.State.class, 62 ControllerNode.State.class,
57 Device.Type.class, 63 Device.Type.class,
58 DefaultAnnotations.class, 64 DefaultAnnotations.class,
......