[ONOS-5934] MeterId availability after withdraw.
Change-Id: Ib40b3ca53bae48d16ecbed9665a4061dd2f7eb0c
diff --git a/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java b/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java
index 5a5016e..a63d272 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java
@@ -114,4 +114,14 @@
*/
long getMaxMeters(MeterFeaturesKey key);
+ /**
+ * Returns the first available MeterId from previously removed meter.
+ * This method allows allocating MeterIds below the actual meterIdCounter
+ * value.
+ *
+ * @param deviceId the device id
+ * @return the meter Id or null if none exist
+ */
+ MeterId firstReusableMeterId(DeviceId deviceId);
+
}
diff --git a/core/api/src/test/java/org/onosproject/store/service/TestStorageService.java b/core/api/src/test/java/org/onosproject/store/service/TestStorageService.java
index 4305fbe..f139664 100644
--- a/core/api/src/test/java/org/onosproject/store/service/TestStorageService.java
+++ b/core/api/src/test/java/org/onosproject/store/service/TestStorageService.java
@@ -49,6 +49,11 @@
}
@Override
+ public <K, V> ConsistentMultimapBuilder<K, V> consistentMultimapBuilder() {
+ return TestConsistentMultimap.builder();
+ }
+
+ @Override
public <T> Topic<T> getTopic(String name, Serializer serializer) {
return new TestTopic(name);
}
diff --git a/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java b/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java
index 1777cbc..30adaa9 100644
--- a/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java
+++ b/incubator/net/src/main/java/org/onosproject/incubator/net/meter/impl/MeterManager.java
@@ -189,9 +189,19 @@
}
private MeterId allocateMeterId(DeviceId deviceId) {
+ // We first query the store for any previously removed meterId that could
+ // be reused. Receiving a value (not null) already means that meters
+ // are available for the device.
+ MeterId meterid = store.firstReusableMeterId(deviceId);
+ if (meterid != null) {
+ return meterid;
+ }
+ // If there was no reusable MeterId we have to generate a new value
+ // with an upper limit in maxMeters.
long maxMeters = store.getMaxMeters(MeterFeaturesKey.key(deviceId));
if (maxMeters == 0L) {
- // MeterFeatures couldn't be retrieved, trying with queryMeters
+ // MeterFeatures couldn't be retrieved, trying with queryMeters.
+ // queryMeters is implemented in FullMetersAvailable behaviour.
maxMeters = queryMeters(deviceId);
}
diff --git a/incubator/net/src/test/java/org/onosproject/incubator/net/meter/impl/MeterManagerTest.java b/incubator/net/src/test/java/org/onosproject/incubator/net/meter/impl/MeterManagerTest.java
index a81004f..045f270 100644
--- a/incubator/net/src/test/java/org/onosproject/incubator/net/meter/impl/MeterManagerTest.java
+++ b/incubator/net/src/test/java/org/onosproject/incubator/net/meter/impl/MeterManagerTest.java
@@ -52,6 +52,7 @@
import org.onosproject.net.provider.ProviderId;
import org.onosproject.store.service.TestStorageService;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
@@ -87,6 +88,7 @@
private Meter m2;
private MeterRequest.Builder m1Request;
private MeterRequest.Builder m2Request;
+ private MeterRequest.Builder m3Request;
private Map<MeterId, Meter> meters = Maps.newHashMap();
@@ -166,6 +168,11 @@
.withUnit(Meter.Unit.KB_PER_SEC)
.withBands(Collections.singletonList(band));
+ m3Request = DefaultMeterRequest.builder()
+ .forDevice(did("1"))
+ .fromApp(APP_ID)
+ .withUnit(Meter.Unit.KB_PER_SEC)
+ .withBands(Collections.singletonList(band));
}
@@ -220,6 +227,30 @@
assertEquals(meterStore.getMaxMeters(MeterFeaturesKey.key(did("2"))), 2);
}
+ @Test
+ public void testMeterReuse() {
+ manager.submit(m1Request.add());
+ manager.submit(m3Request.add());
+ Collection<Meter> meters = manager.getMeters(did("1"));
+ Meter m = meters.iterator().next();
+ MeterRequest mr = DefaultMeterRequest.builder()
+ .forDevice(m.deviceId())
+ .fromApp(m.appId())
+ .withBands(m.bands())
+ .withUnit(m.unit())
+ .remove();
+ manager.withdraw(mr, m.id());
+ mr = DefaultMeterRequest.builder()
+ .forDevice(m.deviceId())
+ .fromApp(m.appId())
+ .withBands(m.bands())
+ .withUnit(m.unit())
+ .add();
+ Meter meter = manager.submit(mr);
+ assertTrue("Meter id not reused", m.id().equals(meter.id()));
+
+ }
+
public class TestApplicationId extends DefaultApplicationId {
diff --git a/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java b/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java
index c6c9f1d..8306288 100644
--- a/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java
+++ b/incubator/store/src/main/java/org/onosproject/incubator/store/meter/impl/DistributedMeterStore.java
@@ -30,18 +30,19 @@
import org.onosproject.net.meter.Band;
import org.onosproject.net.meter.DefaultBand;
import org.onosproject.net.meter.DefaultMeter;
+import org.onosproject.net.meter.DefaultMeterFeatures;
import org.onosproject.net.meter.Meter;
import org.onosproject.net.meter.MeterEvent;
import org.onosproject.net.meter.MeterFailReason;
import org.onosproject.net.meter.MeterFeatures;
import org.onosproject.net.meter.MeterFeaturesKey;
+import org.onosproject.net.meter.MeterId;
import org.onosproject.net.meter.MeterKey;
import org.onosproject.net.meter.MeterOperation;
import org.onosproject.net.meter.MeterState;
import org.onosproject.net.meter.MeterStore;
import org.onosproject.net.meter.MeterStoreDelegate;
import org.onosproject.net.meter.MeterStoreResult;
-import org.onosproject.net.meter.DefaultMeterFeatures;
import org.onosproject.store.AbstractStore;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.ConsistentMap;
@@ -54,6 +55,7 @@
import org.slf4j.Logger;
import java.util.Arrays;
+import java.util.BitSet;
import java.util.Collection;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
@@ -74,6 +76,7 @@
private static final String METERSTORE = "onos-meter-store";
private static final String METERFEATURESSTORE = "onos-meter-features-store";
+ private static final String AVAILABLEMETERIDSTORE = "onos-meters-available-store";
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
private StorageService storageService;
@@ -94,6 +97,8 @@
private Map<MeterKey, CompletableFuture<MeterStoreResult>> futures =
Maps.newConcurrentMap();
+ private ConsistentMap<DeviceId, BitSet> availableMeterIds;
+
@Activate
public void activate() {
local = clusterService.getLocalNode().id();
@@ -122,6 +127,12 @@
Meter.Unit.class,
MeterFailReason.class)).build();
+ availableMeterIds = storageService.<DeviceId, BitSet>consistentMapBuilder()
+ .withName(AVAILABLEMETERIDSTORE)
+ .withSerializer(Serializer.using(Arrays.asList(KryoNamespaces.API),
+ DeviceId.class,
+ BitSet.class)).build();
+
log.info("Started");
}
@@ -131,11 +142,33 @@
log.info("Stopped");
}
+ private void updateMeterIdAvailability(DeviceId deviceId, MeterId id,
+ boolean available) {
+ availableMeterIds.compute(deviceId, (k, v) -> {
+ v = v == null ? new BitSet() : v;
+ v.set(id.id().intValue(), available);
+ return v;
+ });
+ }
+
+ public MeterId firstReusableMeterId(DeviceId deviceId) {
+ Versioned<BitSet> bitSetVersioned = availableMeterIds.get(deviceId);
+ if (bitSetVersioned == null) {
+ return null;
+ }
+ BitSet value = bitSetVersioned.value();
+ int nextSetBit = value.nextSetBit(1);
+ if (nextSetBit < 0) {
+ return null;
+ }
+ return MeterId.meterId(nextSetBit);
+ }
@Override
public CompletableFuture<MeterStoreResult> storeMeter(Meter meter) {
CompletableFuture<MeterStoreResult> future = new CompletableFuture<>();
MeterKey key = MeterKey.key(meter.deviceId(), meter.id());
+ updateMeterIdAvailability(meter.deviceId(), meter.id(), false);
futures.put(key, future);
MeterData data = new MeterData(meter, null, local);
@@ -163,6 +196,7 @@
if (meters.computeIfPresent(key, (k, v) -> data) == null) {
future.complete(MeterStoreResult.success());
}
+ updateMeterIdAvailability(meter.deviceId(), meter.id(), true);
} catch (StorageException e) {
future.completeExceptionally(e);
}