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/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
index 05abcfc..28847c6 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
+++ b/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/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
index 3605727..ea44703 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/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/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
index bdf056e..a727e67 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
+++ b/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();