[VOL-2525] Fixing the OLT app meter removal and optimizing the bandwith profile to meter map with a Multimap
Change-Id: I62b1fe58770df8f1a9e73a4b5bc1dec751d47ad9
diff --git a/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java b/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java
index 630ae4a..bf00b92 100644
--- a/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java
+++ b/app/src/main/java/org/opencord/olt/cli/ShowBpMeterMappingsCommand.java
@@ -22,7 +22,9 @@
import org.opencord.olt.internalapi.AccessDeviceMeterService;
import java.util.Map;
-import java.util.Set;
+import java.util.Collection;
+
+
@Command(scope = "onos", name = "volt-bpmeter-mappings",
description = "Shows information about bandwidthProfile-meterKey (device / meter) mappings")
@@ -31,11 +33,11 @@
@Override
protected void execute() {
AccessDeviceMeterService service = AbstractShellCommand.get(AccessDeviceMeterService.class);
- Map<String, Set<MeterKey>> bpMeterMappings = service.getBpMeterMappings();
+ Map<String, Collection<MeterKey>> bpMeterMappings = service.getBpMeterMappings();
bpMeterMappings.forEach(this::display);
}
- private void display(String bpInfo, Set<MeterKey> meterKeyList) {
+ private void display(String bpInfo, Collection<MeterKey> meterKeyList) {
meterKeyList.forEach(meterKey ->
print("bpInfo=%s deviceId=%s meterId=%s",
bpInfo, meterKey.deviceId(), meterKey.meterId()));
diff --git a/app/src/main/java/org/opencord/olt/impl/OltMeterService.java b/app/src/main/java/org/opencord/olt/impl/OltMeterService.java
index df434eb..3fff30c 100644
--- a/app/src/main/java/org/opencord/olt/impl/OltMeterService.java
+++ b/app/src/main/java/org/opencord/olt/impl/OltMeterService.java
@@ -15,9 +15,12 @@
*/
package org.opencord.olt.impl;
+import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
+import com.google.common.collect.Multimaps;
+import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
@@ -51,7 +54,7 @@
import org.slf4j.Logger;
import java.util.ArrayList;
-
+import java.util.Collection;
import java.util.Dictionary;
import java.util.Iterator;
import java.util.List;
@@ -68,6 +71,9 @@
import static org.onlab.util.Tools.groupedThreads;
import static org.slf4j.LoggerFactory.getLogger;
+/**
+ * Provisions Meters on access devices.
+ */
@Service
@Component(immediate = true)
public class OltMeterService implements AccessDeviceMeterService {
@@ -85,7 +91,8 @@
label = "Deleting Meters based on flow count statistics")
protected boolean deleteMeters = true;
- protected Map<String, Set<MeterKey>> bpInfoToMeter;
+ protected SetMultimap<String, MeterKey> bpInfoToMeter =
+ Multimaps.synchronizedSetMultimap(HashMultimap.create());
protected Set<MeterKey> programmedMeters;
private ApplicationId appId;
private static final String APP_NAME = "org.opencord.olt";
@@ -101,7 +108,6 @@
eventExecutor = Executors.newFixedThreadPool(5, groupedThreads("onos/olt",
"events-%d", log));
appId = coreService.registerApplication(APP_NAME);
- bpInfoToMeter = Maps.newConcurrentMap();
programmedMeters = Sets.newConcurrentHashSet();
meterService.addListener(meterListener);
componentConfigService.registerProperties(getClass());
@@ -125,20 +131,13 @@
}
@Override
- public ImmutableMap<String, Set<MeterKey>> getBpMeterMappings() {
- return ImmutableMap.copyOf(bpInfoToMeter);
+ public ImmutableMap<String, Collection<MeterKey>> getBpMeterMappings() {
+ return ImmutableMap.copyOf(bpInfoToMeter.asMap());
}
@Override
public void addMeterIdToBpMapping(DeviceId deviceId, MeterId meterId, String bandwidthProfile) {
- bpInfoToMeter.compute(bandwidthProfile, (k, v) -> {
- if (v == null) {
- return Sets.newHashSet(MeterKey.key(deviceId, meterId));
- } else {
- v.add(MeterKey.key(deviceId, meterId));
- return v;
- }
- });
+ bpInfoToMeter.put(bandwidthProfile, MeterKey.key(deviceId, meterId));
}
@Override
@@ -199,7 +198,8 @@
@Override
public void onError(MeterRequest op, MeterFailReason reason) {
- bpInfoToMeter.remove(MeterKey.key(deviceId, meterIdRef.get()));
+ bpInfoToMeter.remove(bpInfo.id(),
+ MeterKey.key(deviceId, meterIdRef.get()));
meterFuture.complete(reason);
}
})
@@ -242,12 +242,16 @@
public void event(MeterEvent meterEvent) {
eventExecutor.execute(() -> {
Meter meter = meterEvent.subject();
+ if (meter == null) {
+ log.error("Meter in event {} is null", meterEvent);
+ return;
+ }
MeterKey key = MeterKey.key(meter.deviceId(), meter.id());
if (deleteMeters && MeterEvent.Type.METER_REFERENCE_COUNT_ZERO.equals(meterEvent.type())) {
log.info("Zero Count Meter Event is received. Meter is {}", meter.id());
incrementMeterCount(key);
- if (meter != null && appId.equals(meter.appId()) && pendingRemoveMeters.get(key).get() == 3) {
+ if (appId.equals(meter.appId()) && pendingRemoveMeters.get(key).get() == 3) {
log.info("Deleting unreferenced, no longer programmed Meter {}", meter.id());
deleteMeter(meter.deviceId(), meter.id());
}
@@ -256,7 +260,7 @@
log.info("Meter Removed Event is received for {}", meter.id());
programmedMeters.remove(key);
pendingRemoveMeters.remove(key);
- removeMeterFromBpMapping(meter);
+ removeMeterFromBpMapping(key);
}
});
}
@@ -290,12 +294,11 @@
}
}
- private void removeMeterFromBpMapping(Meter meter) {
- MeterKey meterKey = MeterKey.key(meter.deviceId(), meter.id());
- Iterator<Map.Entry<String, Set<MeterKey>>> iterator = bpInfoToMeter.entrySet().iterator();
+ private void removeMeterFromBpMapping(MeterKey meterKey) {
+ Iterator<Map.Entry<String, MeterKey>> iterator = bpInfoToMeter.entries().iterator();
while (iterator.hasNext()) {
- Map.Entry<String, Set<MeterKey>> entry = iterator.next();
- if (entry.getValue().contains(meterKey)) {
+ Map.Entry<String, MeterKey> entry = iterator.next();
+ if (entry.getValue().equals(meterKey)) {
iterator.remove();
log.info("Deleted meter for MeterKey {} - Last prog meters {}", meterKey, programmedMeters);
break;
diff --git a/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java b/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java
index d2d51b2..4f07131 100644
--- a/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java
+++ b/app/src/main/java/org/opencord/olt/internalapi/AccessDeviceMeterService.java
@@ -22,7 +22,7 @@
import org.onosproject.net.meter.MeterKey;
import org.opencord.sadis.BandwidthProfileInformation;
-import java.util.Set;
+import java.util.Collection;
import java.util.concurrent.CompletableFuture;
/**
@@ -36,7 +36,7 @@
*
* @return an immutable map of bandwidthProfile-meterKey (device / meter) mappings
*/
- ImmutableMap<String, Set<MeterKey>> getBpMeterMappings();
+ ImmutableMap<String, Collection<MeterKey>> getBpMeterMappings();
/**
* Adds a bandwidthProfile-meterKey (device / meter) mapping that have been programmed
diff --git a/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java b/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
index 8a80ee0..16b407b 100644
--- a/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
+++ b/app/src/test/java/org/opencord/olt/impl/OltFlowTest.java
@@ -183,7 +183,7 @@
private class MockOltMeterService implements org.opencord.olt.internalapi.AccessDeviceMeterService {
@Override
- public ImmutableMap<String, Set<MeterKey>> getBpMeterMappings() {
+ public ImmutableMap<String, Collection<MeterKey>> getBpMeterMappings() {
return null;
}
diff --git a/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java b/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java
index 7100b0f..da7e3e1 100644
--- a/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java
+++ b/app/src/test/java/org/opencord/olt/impl/OltMeterTest.java
@@ -15,8 +15,9 @@
*/
package org.opencord.olt.impl;
+import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
+import com.google.common.collect.Multimaps;
import com.google.common.collect.Sets;
import org.junit.Before;
import org.junit.Test;
@@ -30,7 +31,6 @@
import org.opencord.sadis.BandwidthProfileInformation;
import java.util.Collection;
-import java.util.Set;
import java.util.concurrent.CompletableFuture;
public class OltMeterTest extends TestBase {
@@ -41,7 +41,7 @@
@Before
public void setUp() {
oltMeterService = new OltMeterService();
- oltMeterService.bpInfoToMeter = Maps.newConcurrentMap();
+ oltMeterService.bpInfoToMeter = Multimaps.synchronizedSetMultimap(HashMultimap.create());
oltMeterService.programmedMeters = Sets.newConcurrentHashSet();
oltMeterService.meterService = new MockMeterService();
}
@@ -56,7 +56,7 @@
MeterId dsMeterId = oltMeterService.getMeterIdFromBpMapping(DEVICE_ID_1, dsBpId);
assert dsMeterId.equals(this.dsMeterId);
- ImmutableMap<String, Set<MeterKey>> meterMappings = oltMeterService.getBpMeterMappings();
+ ImmutableMap<String, Collection<MeterKey>> meterMappings = oltMeterService.getBpMeterMappings();
assert meterMappings.size() == 2;
}