Performance improvement when handling host move events
- Avoid querying shouldProgram every time
- Override flows directly instead of remove and add
- Remove unnecessary event handling delay since we have in-order execution now
- Avoid re-initiation of shouldProgram
- Make sure executors are shut down during SR deactivation
Change-Id: I28e383ed2dcb66d503da25934456008e83683b78
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 52b02a3..7503ba8 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -89,6 +89,7 @@
// Keep track on which ONOS instance should program the device pair.
// There should be only one instance that programs the same pair.
Map<Set<DeviceId>, NodeId> shouldProgram;
+ Map<DeviceId, Boolean> shouldProgramCache;
/**
* Represents the default routing population status.
@@ -114,15 +115,26 @@
* @param srManager SegmentRoutingManager object
*/
DefaultRoutingHandler(SegmentRoutingManager srManager) {
+ this.shouldProgram = srManager.storageService.<Set<DeviceId>, NodeId>consistentMapBuilder()
+ .withName("sr-should-program")
+ .withSerializer(Serializer.using(KryoNamespaces.API))
+ .withRelaxedReadConsistency()
+ .build().asJavaMap();
+ this.shouldProgramCache = Maps.newConcurrentMap();
+ update(srManager);
+ }
+
+ /**
+ * Updates a DefaultRoutingHandler object.
+ *
+ * @param srManager SegmentRoutingManager object
+ */
+ void update(SegmentRoutingManager srManager) {
this.srManager = srManager;
this.rulePopulator = checkNotNull(srManager.routingRulePopulator);
this.config = checkNotNull(srManager.deviceConfiguration);
this.populationStatus = Status.IDLE;
this.currentEcmpSpgMap = Maps.newHashMap();
- this.shouldProgram = srManager.storageService.<Set<DeviceId>, NodeId>consistentMapBuilder()
- .withName("sr-should-program")
- .withSerializer(Serializer.using(KryoNamespaces.API))
- .build().asJavaMap();
}
/**
@@ -1150,7 +1162,7 @@
*
* @param deviceId the device for which graphs need to be purged
*/
- protected void purgeEcmpGraph(DeviceId deviceId) {
+ void purgeEcmpGraph(DeviceId deviceId) {
statusLock.lock();
try {
@@ -1423,6 +1435,11 @@
* @return true if current instance should handle the routing for given device
*/
boolean shouldProgram(DeviceId deviceId) {
+ Boolean cached = shouldProgramCache.get(deviceId);
+ if (cached != null) {
+ return cached;
+ }
+
Optional<DeviceId> pairDeviceId = srManager.getPairDeviceId(deviceId);
NodeId currentNodeId = srManager.clusterService.getLocalNode().id();
@@ -1464,9 +1481,11 @@
if (king != null) {
log.debug("{} should handle routing for {}/pair={}", king, deviceId, pairDeviceId);
+ shouldProgramCache.put(deviceId, king.equals(currentNodeId));
return king.equals(currentNodeId);
} else {
log.error("Fail to elect a king for {}/pair={}. Abort.", deviceId, pairDeviceId);
+ shouldProgramCache.remove(deviceId);
return false;
}
}
@@ -1484,6 +1503,10 @@
return nodeIds.size() == 0 ? null : nodeIds.get(0);
}
+ void invalidateShouldProgramCache(DeviceId deviceId) {
+ shouldProgramCache.remove(deviceId);
+ }
+
/**
* Returns a set of device ID, containing given device and its pair device if exist.
*
@@ -1625,7 +1648,7 @@
*
* @param deviceId Switch ID to set the rules
*/
- public void populatePortAddressingRules(DeviceId deviceId) {
+ void populatePortAddressingRules(DeviceId deviceId) {
// Although device is added, sometimes device store does not have the
// ports for this device yet. It results in missing filtering rules in the
// switch. We will attempt it a few times. If it still does not work,
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 9590632..e4efabb 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -36,9 +36,6 @@
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkArgument;
@@ -165,17 +162,7 @@
Set<HostLocation> newLocations = event.subject().locations();
Set<IpAddress> newIps = event.subject().ipAddresses();
- // FIXME: Delay event handling a little bit to wait for the previous redirection flows to be completed
- // The permanent solution would be introducing CompletableFuture and wait for it
- if (prevLocations.size() == 1 && newLocations.size() == 2) {
- log.debug("Delay event handling when host {}/{} moves from 1 to 2 locations", hostMac, hostVlanId);
- ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
- executorService.schedule(() ->
- processHostMoved(hostMac, hostVlanId, prevLocations, prevIps, newLocations, newIps),
- HOST_MOVED_DELAY_MS, TimeUnit.MILLISECONDS);
- } else {
- processHostMoved(hostMac, hostVlanId, prevLocations, prevIps, newLocations, newIps);
- }
+ processHostMoved(hostMac, hostVlanId, prevLocations, prevIps, newLocations, newIps);
}
private void processHostMoved(MacAddress hostMac, VlanId hostVlanId, Set<HostLocation> prevLocations,
@@ -209,7 +196,7 @@
return;
}
- // Remove bridging rule and routing rules for unchanged IPs if the host moves from a switch to another.
+ // Remove flows for unchanged IPs only when the host moves from a switch to another.
// Otherwise, do not remove and let the adding part update the old flow
if (!newDeviceIds.contains(prevLocation.deviceId())) {
processBridgingRule(prevLocation.deviceId(), prevLocation.port(), hostMac, hostVlanId, true);
@@ -245,8 +232,7 @@
});
// For each new location, add all new IPs.
- Sets.difference(newLocations, prevLocations).stream()
- .forEach(newLocation -> {
+ Sets.difference(newLocations, prevLocations).forEach(newLocation -> {
processBridgingRule(newLocation.deviceId(), newLocation.port(), hostMac, hostVlanId, false);
newIps.forEach(ip ->
processRoutingRule(newLocation.deviceId(), newLocation.port(), hostMac, hostVlanId,
@@ -255,8 +241,7 @@
});
// For each unchanged location, add new IPs and remove old IPs.
- Sets.intersection(newLocations, prevLocations).stream()
- .forEach(unchangedLocation -> {
+ Sets.intersection(newLocations, prevLocations).forEach(unchangedLocation -> {
Sets.difference(prevIps, newIps).forEach(ip ->
processRoutingRule(unchangedLocation.deviceId(), unchangedLocation.port(), hostMac,
hostVlanId, ip, true)
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index b278b8c..9f97987 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -16,10 +16,6 @@
package org.onosproject.segmentrouting;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.RemovalCause;
-import com.google.common.cache.RemovalNotification;
import com.google.common.collect.Sets;
import org.onlab.packet.IpPrefix;
@@ -40,8 +36,6 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
/**
@@ -51,35 +45,8 @@
private static final Logger log = LoggerFactory.getLogger(RouteHandler.class);
private final SegmentRoutingManager srManager;
- private static final int WAIT_TIME_MS = 1000;
- /**
- * The routeEventCache is implemented to avoid race condition by giving more time to the
- * underlying flow subsystem to process previous populateSubnet call.
- */
- private Cache<IpPrefix, RouteEvent> routeEventCache = CacheBuilder.newBuilder()
- .expireAfterWrite(WAIT_TIME_MS, TimeUnit.MILLISECONDS)
- .removalListener((RemovalNotification<IpPrefix, RouteEvent> notification) -> {
- IpPrefix prefix = notification.getKey();
- RouteEvent routeEvent = notification.getValue();
- RemovalCause cause = notification.getCause();
- log.debug("routeEventCache removal event. prefix={}, routeEvent={}, cause={}",
- prefix, routeEvent, cause);
-
- switch (notification.getCause()) {
- case REPLACED:
- case EXPIRED:
- dequeueRouteEvent(routeEvent);
- break;
- default:
- break;
- }
- }).build();
-
RouteHandler(SegmentRoutingManager srManager) {
this.srManager = srManager;
-
- Executors.newSingleThreadScheduledExecutor()
- .scheduleAtFixedRate(routeEventCache::cleanUp, 0, WAIT_TIME_MS, TimeUnit.MILLISECONDS);
}
protected void init(DeviceId deviceId) {
@@ -98,7 +65,7 @@
}
void processRouteAdded(RouteEvent event) {
- enqueueRouteEvent(event);
+ processRouteAddedInternal(event.alternatives());
}
private void processRouteAddedInternal(Collection<ResolvedRoute> routes) {
@@ -140,11 +107,13 @@
}
void processRouteUpdated(RouteEvent event) {
- enqueueRouteEvent(event);
+ processRouteUpdatedInternal(Sets.newHashSet(event.alternatives()),
+ Sets.newHashSet(event.prevAlternatives()));
}
void processAlternativeRoutesChanged(RouteEvent event) {
- enqueueRouteEvent(event);
+ processRouteUpdatedInternal(Sets.newHashSet(event.alternatives()),
+ Sets.newHashSet(event.prevAlternatives()));
}
private void processRouteUpdatedInternal(Set<ResolvedRoute> routes, Set<ResolvedRoute> oldRoutes) {
@@ -212,7 +181,7 @@
}
void processRouteRemoved(RouteEvent event) {
- enqueueRouteEvent(event);
+ processRouteRemovedInternal(event.alternatives());
}
private void processRouteRemovedInternal(Collection<ResolvedRoute> routes) {
@@ -268,6 +237,9 @@
Set<HostLocation> prevLocations = event.prevSubject().locations();
Set<HostLocation> newLocations = event.subject().locations();
+ Set<DeviceId> newDeviceIds = newLocations.stream().map(HostLocation::deviceId)
+ .collect(Collectors.toSet());
+
// For each old location
Sets.difference(prevLocations, newLocations).forEach(prevLocation -> {
// Redirect the flows to pair link if configured
@@ -285,10 +257,14 @@
return;
}
- // No pair information supplied. Remove route
- log.debug("HostMoved. revokeRoute {}, {}, {}, {}", prevLocation, prefix, hostMac, hostVlanId);
- srManager.defaultRoutingHandler.revokeRoute(prevLocation.deviceId(), prefix,
- hostMac, hostVlanId, prevLocation.port());
+ // No pair information supplied.
+ // Remove flows for unchanged IPs only when the host moves from a switch to another.
+ // Otherwise, do not remove and let the adding part update the old flow
+ if (!newDeviceIds.contains(prevLocation.deviceId())) {
+ log.debug("HostMoved. revokeRoute {}, {}, {}, {}", prevLocation, prefix, hostMac, hostVlanId);
+ srManager.defaultRoutingHandler.revokeRoute(prevLocation.deviceId(), prefix,
+ hostMac, hostVlanId, prevLocation.port());
+ }
});
// For each new location, add all new IPs.
@@ -315,28 +291,4 @@
return Objects.nonNull(srManager.deviceConfiguration) &&
Objects.nonNull(srManager.defaultRoutingHandler);
}
-
- void enqueueRouteEvent(RouteEvent routeEvent) {
- log.debug("Enqueue routeEvent {}", routeEvent);
- routeEventCache.put(routeEvent.subject().prefix(), routeEvent);
- }
-
- void dequeueRouteEvent(RouteEvent routeEvent) {
- log.debug("Dequeue routeEvent {}", routeEvent);
- switch (routeEvent.type()) {
- case ROUTE_ADDED:
- processRouteAddedInternal(routeEvent.alternatives());
- break;
- case ROUTE_REMOVED:
- processRouteRemovedInternal(routeEvent.alternatives());
- break;
- case ROUTE_UPDATED:
- case ALTERNATIVE_ROUTES_CHANGED:
- processRouteUpdatedInternal(Sets.newHashSet(routeEvent.alternatives()),
- Sets.newHashSet(routeEvent.prevAlternatives()));
- break;
- default:
- break;
- }
- }
}
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index c200d74..46599ea 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -43,6 +43,8 @@
import org.onosproject.core.ApplicationId;
import org.onosproject.core.CoreService;
import org.onosproject.event.Event;
+import org.onosproject.mastership.MastershipEvent;
+import org.onosproject.mastership.MastershipListener;
import org.onosproject.mastership.MastershipService;
import org.onosproject.mcast.api.McastEvent;
import org.onosproject.mcast.api.McastListener;
@@ -246,19 +248,15 @@
private final InternalMcastListener mcastListener = new InternalMcastListener();
private final InternalRouteEventListener routeListener = new InternalRouteEventListener();
private final InternalTopologyListener topologyListener = new InternalTopologyListener();
+ private final InternalMastershipListener mastershipListener = new InternalMastershipListener();
// Handles device, link, topology and network config events
- private ScheduledExecutorService mainEventExecutor = Executors
- .newScheduledThreadPool(1, groupedThreads("sr-event-main", "%d", log));
+ private ScheduledExecutorService mainEventExecutor;
- // Handles host, route, mcast events
- private ScheduledExecutorService hostEventExecutor = Executors
- .newScheduledThreadPool(1, groupedThreads("sr-event-host", "%d", log));
- private ScheduledExecutorService routeEventExecutor = Executors
- .newScheduledThreadPool(1, groupedThreads("sr-event-route", "%d", log));
- private ScheduledExecutorService mcastEventExecutor = Executors
- .newScheduledThreadPool(1, groupedThreads("sr-event-mcast", "%d", log));
-
+ // Handles host, route and mcast events respectively
+ private ScheduledExecutorService hostEventExecutor;
+ private ScheduledExecutorService routeEventExecutor;
+ private ScheduledExecutorService mcastEventExecutor;
Map<DeviceId, DefaultGroupHandler> groupHandlerMap = new ConcurrentHashMap<>();
/**
@@ -325,12 +323,6 @@
}
};
- private static final Object THREAD_SCHED_LOCK = new Object();
- private static int numOfEventsQueued = 0;
- private static int numOfEventsExecuted = 0;
- private static int numOfHandlerExecution = 0;
- private static int numOfHandlerScheduled = 0;
-
/**
* Segment Routing App ID.
*/
@@ -345,6 +337,11 @@
protected void activate(ComponentContext context) {
appId = coreService.registerApplication(APP_NAME);
+ mainEventExecutor = Executors.newSingleThreadScheduledExecutor(groupedThreads("sr-event-main", "%d", log));
+ hostEventExecutor = Executors.newSingleThreadScheduledExecutor(groupedThreads("sr-event-host", "%d", log));
+ routeEventExecutor = Executors.newSingleThreadScheduledExecutor(groupedThreads("sr-event-route", "%d", log));
+ mcastEventExecutor = Executors.newSingleThreadScheduledExecutor(groupedThreads("sr-event-mcast", "%d", log));
+
log.debug("Creating EC map nsnextobjectivestore");
EventuallyConsistentMapBuilder<DestinationSetNextObjectiveStoreKey, NextNeighbors>
nsNextObjMapBuilder = storageService.eventuallyConsistentMapBuilder();
@@ -442,6 +439,7 @@
multicastRouteService.addListener(mcastListener);
routeService.addListener(routeListener);
topologyService.addListener(topologyListener);
+ mastershipService.addListener(mastershipListener);
linkHandler.init();
l2TunnelHandler.init();
@@ -472,6 +470,11 @@
@Deactivate
protected void deactivate() {
+ mainEventExecutor.shutdown();
+ hostEventExecutor.shutdown();
+ routeEventExecutor.shutdown();
+ mcastEventExecutor.shutdown();
+
cfgService.removeListener(cfgListener);
cfgService.unregisterConfigFactory(deviceConfigFactory);
cfgService.unregisterConfigFactory(appConfigFactory);
@@ -486,6 +489,7 @@
multicastRouteService.removeListener(mcastListener);
routeService.removeListener(routeListener);
topologyService.removeListener(topologyListener);
+ mastershipService.removeListener(mastershipListener);
neighbourResolutionService.unregisterNeighbourHandlers(appId);
@@ -847,20 +851,6 @@
}
/**
- * Determine if current instance is the master of given connect point.
- *
- * @param cp connect point
- * @return true if current instance is the master of given connect point
- */
- public boolean isMasterOf(ConnectPoint cp) {
- boolean isMaster = mastershipService.isLocalMaster(cp.deviceId());
- if (!isMaster) {
- log.debug(NOT_MASTER, cp);
- }
- return isMaster;
- }
-
- /**
* Returns locations of given resolved route.
*
* @param resolvedRoute resolved route
@@ -1332,6 +1322,16 @@
}
}
+ private void createOrUpdateDefaultRoutingHandler() {
+ if (defaultRoutingHandler == null) {
+ log.info("Creating new DefaultRoutingHandler");
+ defaultRoutingHandler = new DefaultRoutingHandler(this);
+ } else {
+ log.info("Updating DefaultRoutingHandler");
+ defaultRoutingHandler.update(this);
+ }
+ }
+
/**
* Registers the given connect point with the NRS, this is necessary
* to receive the NDP and ARP packets from the NRS.
@@ -1370,7 +1370,7 @@
icmpHandler = new IcmpHandler(srManager);
ipHandler = new IpHandler(srManager);
routingRulePopulator = new RoutingRulePopulator(srManager);
- defaultRoutingHandler = new DefaultRoutingHandler(srManager);
+ createOrUpdateDefaultRoutingHandler();
tunnelHandler = new TunnelHandler(linkService, deviceConfiguration,
groupHandlerMap, tunnelStore);
@@ -1532,6 +1532,27 @@
}
}
+ private class InternalMastershipListener implements MastershipListener {
+ @Override
+ public void event(MastershipEvent event) {
+ DeviceId deviceId = event.subject();
+ Optional<DeviceId> pairDeviceId = getPairDeviceId(deviceId);
+
+ switch (event.type()) {
+ case MASTER_CHANGED:
+ log.info("Invalidating shouldProgram cache for {}/pair={} due to mastership change",
+ deviceId, pairDeviceId);
+ defaultRoutingHandler.invalidateShouldProgramCache(deviceId);
+ pairDeviceId.ifPresent(defaultRoutingHandler::invalidateShouldProgramCache);
+ break;
+ case BACKUPS_CHANGED:
+ case SUSPENDED:
+ default:
+ break;
+ }
+ }
+ }
+
private void updateInterface(InterfaceConfig conf, InterfaceConfig prevConf) {
try {
Set<Interface> intfs = conf.getInterfaces();
diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
index 05abcfc..28847c6 100644
--- a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
+++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
@@ -66,6 +66,12 @@
dfh = new DefaultRoutingHandler(srManager);
}
+ private void clearCache() {
+ dfh.invalidateShouldProgramCache(DEV1A);
+ dfh.invalidateShouldProgramCache(DEV1B);
+ dfh.invalidateShouldProgramCache(DEV2);
+ }
+
// Node 1 is the master of switch 1A, 1B, and 2
@Test
public void testShouldHandleRoutingCase1() {
@@ -87,6 +93,7 @@
assertTrue(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -96,6 +103,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 3 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
@@ -127,6 +135,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program 2
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -136,6 +145,7 @@
assertTrue(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 3 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
@@ -168,6 +178,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -177,6 +188,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 3 should program 2
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
@@ -208,6 +220,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -217,6 +230,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 3 should program 1A, 1B and 2
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
@@ -233,6 +247,7 @@
replay(srManager.mastershipService);
reset(srManager.clusterService);
+ clearCache();
// Node 1 should program 1A, 1B
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
@@ -242,6 +257,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -251,6 +267,7 @@
assertFalse(dfh.shouldProgram(DEV2));
reset(srManager.clusterService);
+ clearCache();
// Node 3 should program 2
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
@@ -279,6 +296,7 @@
assertTrue(dfh.shouldProgram(DEV1B));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -293,6 +311,7 @@
replay(srManager.mastershipService);
reset(srManager.clusterService);
+ clearCache();
// Node 1 should program 1A, 1B
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
@@ -301,6 +320,7 @@
assertTrue(dfh.shouldProgram(DEV1B));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
@@ -327,6 +347,7 @@
assertFalse(dfh.shouldProgram(DEV1B));
reset(srManager.clusterService);
+ clearCache();
// Node 2 should program no device
expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
index 3605727..ea44703 100644
--- a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
@@ -358,14 +358,9 @@
hostHandler.processHostMovedEvent(new HostEvent(HostEvent.Type.HOST_MOVED, host2, host1));
assertEquals(2, ROUTING_TABLE.size());
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP11.toIpPrefix())).portNumber);
- assertEquals(P9, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
assertEquals(2, BRIDGING_TABLE.size());
assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
- assertEquals(P9, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
- // FIXME: Delay event handling a little bit to wait for the previous redirection flows to be completed
- // The permanent solution would be introducing CompletableFuture and wait for it
- Thread.sleep(HostHandler.HOST_MOVED_DELAY_MS + 50);
- assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
}
@@ -633,16 +628,10 @@
assertEquals(4, ROUTING_TABLE.size());
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP11.toIpPrefix())).portNumber);
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP12.toIpPrefix())).portNumber);
- assertEquals(P9, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
- assertEquals(P9, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP12.toIpPrefix())).portNumber);
- assertEquals(2, BRIDGING_TABLE.size());
- assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
- assertEquals(P9, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
- // FIXME: Delay event handling a little bit to wait for the previous redirection flows to be completed
- // The permanent solution would be introducing CompletableFuture and wait for it
- Thread.sleep(HostHandler.HOST_MOVED_DELAY_MS + 50);
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP12.toIpPrefix())).portNumber);
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
}
@@ -756,13 +745,10 @@
hostHandler.processHostMovedEvent(new HostEvent(HostEvent.Type.HOST_MOVED, host3, host2));
assertEquals(2, ROUTING_TABLE.size());
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP11.toIpPrefix())).portNumber);
- assertEquals(P9, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
assertEquals(2, BRIDGING_TABLE.size());
assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
- assertEquals(P9, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
- // FIXME: Delay event handling a little bit to wait for the previous redirection flows to be completed
- // The permanent solution would be introducing CompletableFuture and wait for it
- Thread.sleep(HostHandler.HOST_MOVED_DELAY_MS + 50);
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
}
diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
index bdf056e..a727e67 100644
--- a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
+++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
@@ -170,13 +170,7 @@
srManager.cfgService = mockNetworkConfigRegistry;
srManager.routeService = new MockRouteService(ROUTE_STORE);
- routeHandler = new RouteHandler(srManager) {
- // routeEventCache is not necessary for unit tests
- @Override
- void enqueueRouteEvent(RouteEvent routeEvent) {
- dequeueRouteEvent(routeEvent);
- }
- };
+ routeHandler = new RouteHandler(srManager);
ROUTING_TABLE.clear();
BRIDGING_TABLE.clear();