Fixed issue with leaking various switch-related collectors, e.g. port stats, meters, table stats, flow stats.
Change-Id: If46102708fa88cf5f251a18cb9ce09393fb95752
diff --git a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
index bd4ecc0..c87c93b 100644
--- a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
+++ b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
@@ -180,6 +180,7 @@
controller.removeListener(listener);
providerRegistry.unregister(this);
collectors.values().forEach(PortStatsCollector::stop);
+ collectors.clear();
providerService = null;
LOG.info("Stopped");
}
@@ -374,10 +375,9 @@
providerService.deviceConnected(did, description);
providerService.updatePorts(did, buildPortDescriptions(sw));
- PortStatsCollector psc =
- new PortStatsCollector(sw, portStatsPollFrequency);
+ PortStatsCollector psc = new PortStatsCollector(sw, portStatsPollFrequency);
+ stopCollectorIfNeeded(collectors.put(dpid, psc));
psc.start();
- collectors.put(dpid, psc);
//figure out race condition for collectors.remove() and collectors.put()
if (controller.getSwitch(dpid) == null) {
@@ -385,17 +385,19 @@
}
}
+ private void stopCollectorIfNeeded(PortStatsCollector collector) {
+ if (collector != null) {
+ collector.stop();
+ }
+ }
+
@Override
public void switchRemoved(Dpid dpid) {
if (providerService == null) {
return;
}
providerService.deviceDisconnected(deviceId(uri(dpid)));
-
- PortStatsCollector collector = collectors.remove(dpid);
- if (collector != null) {
- collector.stop();
- }
+ stopCollectorIfNeeded(collectors.remove(dpid));
}
@Override
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowStatsCollector.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowStatsCollector.java
index bd28085..38ebac5 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowStatsCollector.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowStatsCollector.java
@@ -31,7 +31,7 @@
/**
* Collects flow statistics for the specified switch.
*/
-class FlowStatsCollector {
+class FlowStatsCollector implements SwitchDataCollector {
private final Logger log = getLogger(getClass());
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/NewAdaptiveFlowStatsCollector.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/NewAdaptiveFlowStatsCollector.java
index 680b8d1..2d55531 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/NewAdaptiveFlowStatsCollector.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/NewAdaptiveFlowStatsCollector.java
@@ -55,7 +55,7 @@
/**
* Efficiently and adaptively collects flow statistics for the specified switch.
*/
-public class NewAdaptiveFlowStatsCollector {
+public class NewAdaptiveFlowStatsCollector implements SwitchDataCollector {
private final Logger log = getLogger(getClass());
private static final String CHECK_AND_MOVE_LOG =
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java
index 8998a99..a3a0738 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java
@@ -131,7 +131,6 @@
// NewAdaptiveFlowStatsCollector Set
private final Map<Dpid, NewAdaptiveFlowStatsCollector> afsCollectors = Maps.newHashMap();
- private final Map<Dpid, FlowStatsCollector> collectors = Maps.newHashMap();
private final Map<Dpid, TableStatisticsCollector> tableStatsCollectors = Maps.newHashMap();
/**
@@ -142,7 +141,7 @@
}
@Activate
- public void activate(ComponentContext context) {
+ protected void activate(ComponentContext context) {
cfgService.registerProperties(getClass());
providerService = providerRegistry.register(this);
controller.addListener(listener);
@@ -159,7 +158,7 @@
}
@Deactivate
- public void deactivate(ComponentContext context) {
+ protected void deactivate(ComponentContext context) {
cfgService.unregisterProperties(getClass(), false);
stopCollectors();
providerRegistry.unregister(this);
@@ -169,7 +168,7 @@
}
@Modified
- public void modified(ComponentContext context) {
+ protected void modified(ComponentContext context) {
Dictionary<?, ?> properties = context.getProperties();
int newFlowPollFrequency;
try {
@@ -223,15 +222,21 @@
NewAdaptiveFlowStatsCollector fsc =
new NewAdaptiveFlowStatsCollector(driverService, sw, flowPollFrequency);
fsc.start();
- afsCollectors.put(new Dpid(sw.getId()), fsc);
+ stopCollectorIfNeeded(afsCollectors.put(new Dpid(sw.getId()), fsc));
} else {
FlowStatsCollector fsc = new FlowStatsCollector(timer, sw, flowPollFrequency);
fsc.start();
- simpleCollectors.put(new Dpid(sw.getId()), fsc);
+ stopCollectorIfNeeded(simpleCollectors.put(new Dpid(sw.getId()), fsc));
}
TableStatisticsCollector tsc = new TableStatisticsCollector(timer, sw, flowPollFrequency);
tsc.start();
- tableStatsCollectors.put(new Dpid(sw.getId()), tsc);
+ stopCollectorIfNeeded(tableStatsCollectors.put(new Dpid(sw.getId()), tsc));
+ }
+
+ private void stopCollectorIfNeeded(SwitchDataCollector collector) {
+ if (collector != null) {
+ collector.stop();
+ }
}
private void stopCollectors() {
@@ -398,29 +403,17 @@
@Override
public void switchAdded(Dpid dpid) {
-
- OpenFlowSwitch sw = controller.getSwitch(dpid);
-
createCollector(controller.getSwitch(dpid));
}
@Override
public void switchRemoved(Dpid dpid) {
if (adaptiveFlowSampling) {
- NewAdaptiveFlowStatsCollector collector = afsCollectors.remove(dpid);
- if (collector != null) {
- collector.stop();
- }
+ stopCollectorIfNeeded(afsCollectors.remove(dpid));
} else {
- FlowStatsCollector collector = simpleCollectors.remove(dpid);
- if (collector != null) {
- collector.stop();
- }
+ stopCollectorIfNeeded(simpleCollectors.remove(dpid));
}
- TableStatisticsCollector tsc = tableStatsCollectors.remove(dpid);
- if (tsc != null) {
- tsc.stop();
- }
+ stopCollectorIfNeeded(tableStatsCollectors.remove(dpid));
}
@Override
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/SwitchDataCollector.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/SwitchDataCollector.java
new file mode 100644
index 0000000..62dee29
--- /dev/null
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/SwitchDataCollector.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2016 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.provider.of.flow.impl;
+
+/**
+ * Auxiliary abstraction.
+ */
+interface SwitchDataCollector {
+
+ /**
+ * Starts the collector.
+ */
+ void start();
+
+ /**
+ * Stops the collector.
+ */
+ void stop();
+}
\ No newline at end of file
diff --git a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/TableStatisticsCollector.java b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/TableStatisticsCollector.java
index 4b1ce6f..0890369 100644
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/TableStatisticsCollector.java
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/TableStatisticsCollector.java
@@ -29,7 +29,7 @@
/**
* Collects Table statistics for the specified switch.
*/
-class TableStatisticsCollector {
+class TableStatisticsCollector implements SwitchDataCollector {
private final Logger log = getLogger(getClass());
diff --git a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
index a9f9339..83100f4 100644
--- a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
+++ b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
@@ -363,10 +363,7 @@
if (isGroupSupported(sw)) {
GroupStatsCollector gsc = new GroupStatsCollector(sw, POLL_INTERVAL);
gsc.start();
- GroupStatsCollector prevGsc = collectors.put(dpid, gsc);
- if (prevGsc != null) {
- prevGsc.stop();
- }
+ stopCollectorIfNeeded(collectors.put(dpid, gsc));
}
//figure out race condition
@@ -377,7 +374,10 @@
@Override
public void switchRemoved(Dpid dpid) {
- GroupStatsCollector collector = collectors.remove(dpid);
+ stopCollectorIfNeeded(collectors.remove(dpid));
+ }
+
+ private void stopCollectorIfNeeded(GroupStatsCollector collector) {
if (collector != null) {
collector.stop();
}
diff --git a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java
index 24bbb95..952a47f 100644
--- a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java
+++ b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/impl/OpenFlowMeterProvider.java
@@ -207,7 +207,13 @@
if (isMeterSupported(sw)) {
MeterStatsCollector msc = new MeterStatsCollector(sw, POLL_INTERVAL);
msc.start();
- collectors.put(new Dpid(sw.getId()), msc);
+ stopCollectorIfNeeded(collectors.put(new Dpid(sw.getId()), msc));
+ }
+ }
+
+ private void stopCollectorIfNeeded(MeterStatsCollector collector) {
+ if (collector != null) {
+ collector.stop();
}
}
@@ -370,10 +376,7 @@
@Override
public void switchRemoved(Dpid dpid) {
- MeterStatsCollector msc = collectors.remove(dpid);
- if (msc != null) {
- msc.stop();
- }
+ stopCollectorIfNeeded(collectors.remove(dpid));
}
@Override