From: Jozef Bacigal Date: Tue, 28 Mar 2017 06:25:36 +0000 (+0200) Subject: Revert "Fix statistics race condition on big flows" X-Git-Tag: release/boron-sr3~1 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=1537fd31483e3c9f5ec62e882d54acf9e508e556;p=openflowplugin.git Revert "Fix statistics race condition on big flows" This reverts commit 501d4d64c806ad39e90b97def853fa043dda5f30. Change-Id: I9e7c43ac81ec79c12db8ed2203464664d1a6910a Signed-off-by: Jozef Bacigal --- diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/CommonDeviceRegistry.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/CommonDeviceRegistry.java deleted file mode 100644 index afa1d0386d..0000000000 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/CommonDeviceRegistry.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (c) 2017 Pantheon Technologies s.r.o. and others. All rights reserved. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v1.0 which accompanies this distribution, - * and is available at http://www.eclipse.org/legal/epl-v10.html - */ - -package org.opendaylight.openflowplugin.api.openflow.registry; - -import java.util.function.Consumer; - -public interface CommonDeviceRegistry extends AutoCloseable { - - /** - * Store KEY in device registry. - * @param key device registry key - */ - void store(KEY key); - - /** - * Add mark for specified KEY. - * @param key device registry key - */ - void addMark(KEY key); - - /** - * Checks if registry has mark for KEY. - * @param key device registry key - * @return true if device registry has mark for KEY - */ - boolean hasMark(KEY key); - - /** - * Process marked keys. - */ - void processMarks(); - - /** - * Iterate over all keys in device registry. - * @param consumer key consumer - */ - void forEach(Consumer consumer); - - /** - * Get device registry size. - * @return device registry size - */ - int size(); - - @Override - void close(); - -} diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/DeviceFlowRegistry.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/DeviceFlowRegistry.java index d98f56b3f4..e43433f4fb 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/DeviceFlowRegistry.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/DeviceFlowRegistry.java @@ -12,22 +12,30 @@ package org.opendaylight.openflowplugin.api.openflow.registry.flow; import com.google.common.base.Optional; import com.google.common.util.concurrent.ListenableFuture; import java.util.List; -import java.util.function.BiConsumer; -import org.opendaylight.openflowplugin.api.openflow.registry.CommonDeviceRegistry; +import java.util.Map; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode; +import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowId; /** * Registry for mapping composite-key of flow ({@link FlowRegistryKey}) from device view * to flow descriptor ({@link FlowDescriptor}) as the identifier of the same flow in data store. */ -public interface DeviceFlowRegistry extends CommonDeviceRegistry { +public interface DeviceFlowRegistry extends AutoCloseable { ListenableFuture>> fill(); - void storeDescriptor(FlowRegistryKey flowRegistryKey, FlowDescriptor flowDescriptor); + FlowDescriptor retrieveIdForFlow(FlowRegistryKey flowRegistryKey); - FlowDescriptor retrieveDescriptor(FlowRegistryKey flowRegistryKey); + void store(FlowRegistryKey flowRegistryKey, FlowDescriptor flowDescriptor); - void forEachEntry(BiConsumer consumer); + FlowId storeIfNecessary(FlowRegistryKey flowRegistryKey); -} + void removeDescriptor(FlowRegistryKey flowRegistryKey); + + void update(FlowRegistryKey newFlowRegistryKey,FlowDescriptor flowDescriptor); + + Map getAllFlowDescriptors(); + + @Override + void close(); +} \ No newline at end of file diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/FlowDescriptor.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/FlowDescriptor.java index 24fcc945d1..dd9cff527f 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/FlowDescriptor.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/flow/FlowDescriptor.java @@ -11,10 +11,12 @@ package org.opendaylight.openflowplugin.api.openflow.registry.flow; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowId; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.TableKey; +/** + * Created by Martin Bobak <mbobak@cisco.com> on 9.4.2015. + */ public interface FlowDescriptor { FlowId getFlowId(); TableKey getTableKey(); - } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/group/DeviceGroupRegistry.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/group/DeviceGroupRegistry.java index 7414f70f4d..05eb9f6d0d 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/group/DeviceGroupRegistry.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/group/DeviceGroupRegistry.java @@ -8,9 +8,22 @@ package org.opendaylight.openflowplugin.api.openflow.registry.group; -import org.opendaylight.openflowplugin.api.openflow.registry.CommonDeviceRegistry; +import java.util.List; import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.GroupId; -public interface DeviceGroupRegistry extends CommonDeviceRegistry { +/** + * Created by Martin Bobak <mbobak@cisco.com> on 15.4.2015. + */ +public interface DeviceGroupRegistry extends AutoCloseable { + + void store(GroupId groupId); + + void markToBeremoved(GroupId groupId); + + void removeMarked(); + + List getAllGroupIds(); + @Override + void close(); } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/meter/DeviceMeterRegistry.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/meter/DeviceMeterRegistry.java index 25b5bb9bb7..12540cb086 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/meter/DeviceMeterRegistry.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/registry/meter/DeviceMeterRegistry.java @@ -8,9 +8,23 @@ package org.opendaylight.openflowplugin.api.openflow.registry.meter; -import org.opendaylight.openflowplugin.api.openflow.registry.CommonDeviceRegistry; +import java.util.List; import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.types.rev130918.MeterId; -public interface DeviceMeterRegistry extends CommonDeviceRegistry { +/** + * Created by Martin Bobak <mbobak@cisco.com> on 15.4.2015. + */ +public interface DeviceMeterRegistry extends AutoCloseable { + + void store(MeterId meterId); + + void markToBeremoved(MeterId meterId); + + void removeMarked(); + + List getAllMeterIds(); + + @Override + void close(); } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java index b45234307f..f87bbaa810 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java @@ -332,7 +332,7 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi //2. create registry key final FlowRegistryKey flowRegKey = FlowRegistryKeyFactory.create(getDeviceInfo().getVersion(), flowRemovedNotification); //3. lookup flowId - final FlowDescriptor flowDescriptor = deviceFlowRegistry.retrieveDescriptor(flowRegKey); + final FlowDescriptor flowDescriptor = deviceFlowRegistry.retrieveIdForFlow(flowRegKey); //4. if flowId present: if (flowDescriptor != null) { // a) construct flow path diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java index 9492b690fb..4115e8b5aa 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java @@ -11,22 +11,20 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; +import com.google.common.collect.Maps; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.BiConsumer; import java.util.function.Consumer; -import javax.annotation.concurrent.GuardedBy; -import javax.annotation.concurrent.ThreadSafe; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; @@ -44,14 +42,12 @@ import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@ThreadSafe public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { private static final Logger LOG = LoggerFactory.getLogger(DeviceFlowRegistryImpl.class); private static final String ALIEN_SYSTEM_FLOW_ID = "#UF$TABLE*"; private static final AtomicInteger UNACCOUNTED_FLOWS_COUNTER = new AtomicInteger(0); - private final BiMap flowRegistry = HashBiMap.create(); - private final List marks = new ArrayList<>(); + private final BiMap flowRegistry = Maps.synchronizedBiMap(HashBiMap.create()); private final DataBroker dataBroker; private final KeyedInstanceIdentifier instanceIdentifier; private final List>>> lastFillFutures = new ArrayList<>(); @@ -70,14 +66,13 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { if (!flowRegistry.containsKey(key)) { LOG.trace("Found flow with table ID : {} and flow ID : {}", flow.getTableId(), flow.getId().getValue()); final FlowDescriptor descriptor = FlowDescriptorFactory.create(flow.getTableId(), flow.getId()); - storeDescriptor(key, descriptor); + store(key, descriptor); } }; } @Override - @GuardedBy("this") - public synchronized ListenableFuture>> fill() { + public ListenableFuture>> fill() { LOG.debug("Filling flow registry with flows for node: {}", instanceIdentifier.getKey().getId().getValue()); // Prepare path for read transaction @@ -85,12 +80,12 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { final InstanceIdentifier path = instanceIdentifier.augmentation(FlowCapableNode.class); // First, try to fill registry with flows from DS/Configuration - final CheckedFuture, ReadFailedException> configFuture = fillFromDatastore(LogicalDatastoreType.CONFIGURATION, path); + CheckedFuture, ReadFailedException> configFuture = fillFromDatastore(LogicalDatastoreType.CONFIGURATION, path); // Now, try to fill registry with flows from DS/Operational // in case of cluster fail over, when clients are not using DS/Configuration // for adding flows, but only RPCs - final CheckedFuture, ReadFailedException> operationalFuture = fillFromDatastore(LogicalDatastoreType.OPERATIONAL, path); + CheckedFuture, ReadFailedException> operationalFuture = fillFromDatastore(LogicalDatastoreType.OPERATIONAL, path); // And at last, chain and return futures created above. // Also, cache this future, so call to DeviceFlowRegistry.close() will be able @@ -100,7 +95,6 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { return lastFillFuture; } - @GuardedBy("this") private CheckedFuture, ReadFailedException> fillFromDatastore(final LogicalDatastoreType logicalDatastoreType, final InstanceIdentifier path) { // Create new read-only transaction final ReadOnlyTransaction transaction = dataBroker.newReadOnlyTransaction(); @@ -108,32 +102,32 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { // Bail out early if transaction is null if (transaction == null) { return Futures.immediateFailedCheckedFuture( - new ReadFailedException("Read transaction is null")); + new ReadFailedException("Read transaction is null")); } // Prepare read operation from datastore for path final CheckedFuture, ReadFailedException> future = - transaction.read(logicalDatastoreType, path); + transaction.read(logicalDatastoreType, path); // Bail out early if future is null if (future == null) { return Futures.immediateFailedCheckedFuture( - new ReadFailedException("Future from read transaction is null")); + new ReadFailedException("Future from read transaction is null")); } Futures.addCallback(future, new FutureCallback>() { @Override public void onSuccess(Optional result) { result.asSet().stream() - .filter(Objects::nonNull) - .filter(flowCapableNode -> Objects.nonNull(flowCapableNode.getTable())) - .flatMap(flowCapableNode -> flowCapableNode.getTable().stream()) - .filter(Objects::nonNull) - .filter(table -> Objects.nonNull(table.getFlow())) - .flatMap(table -> table.getFlow().stream()) - .filter(Objects::nonNull) - .filter(flow -> Objects.nonNull(flow.getId())) - .forEach(flowConsumer); + .filter(Objects::nonNull) + .filter(flowCapableNode -> Objects.nonNull(flowCapableNode.getTable())) + .flatMap(flowCapableNode -> flowCapableNode.getTable().stream()) + .filter(Objects::nonNull) + .filter(table -> Objects.nonNull(table.getFlow())) + .flatMap(table -> table.getFlow().stream()) + .filter(Objects::nonNull) + .filter(flow -> Objects.nonNull(flow.getId())) + .forEach(flowConsumer); // After we are done with reading from datastore, close the transaction transaction.close(); @@ -150,115 +144,84 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { } @Override - @GuardedBy("this") - public synchronized FlowDescriptor retrieveDescriptor(final FlowRegistryKey flowRegistryKey) { + public FlowDescriptor retrieveIdForFlow(final FlowRegistryKey flowRegistryKey) { LOG.trace("Retrieving flow descriptor for flow hash : {}", flowRegistryKey.hashCode()); - return flowRegistry.get(correctFlowRegistryKey(flowRegistry.keySet(), flowRegistryKey)); + FlowDescriptor flowDescriptor = flowRegistry.get(flowRegistryKey); + // Get FlowDescriptor from flow registry + if(flowDescriptor == null){ + if (LOG.isTraceEnabled()) { + LOG.trace("Failed to retrieve flow descriptor for flow hash : {}, trying with custom equals method", flowRegistryKey.hashCode()); + } + for(Map.Entry fd : flowRegistry.entrySet()) { + if (flowRegistryKey.equals(fd.getKey())) { + flowDescriptor = fd.getValue(); + break; + } + } + } + return flowDescriptor; } @Override - @GuardedBy("this") - public synchronized void storeDescriptor(final FlowRegistryKey flowRegistryKey, final FlowDescriptor flowDescriptor) { - final FlowRegistryKey correctFlowRegistryKey = correctFlowRegistryKey(flowRegistry.keySet(), flowRegistryKey); - + public void store(final FlowRegistryKey flowRegistryKey, final FlowDescriptor flowDescriptor) { try { - if (hasMark(correctFlowRegistryKey)) { - // We are probably doing update of flow ID or table ID, so remove mark for removal of this flow - // and replace it with new value - marks.remove(correctFlowRegistryKey(marks, correctFlowRegistryKey)); - flowRegistry.forcePut(correctFlowRegistryKey, flowDescriptor); - return; - } - - LOG.trace("Storing flowDescriptor with table ID : {} and flow ID : {} for flow hash : {}", - flowDescriptor.getTableKey().getId(), flowDescriptor.getFlowId().getValue(), correctFlowRegistryKey.hashCode()); - - flowRegistry.put(correctFlowRegistryKey, flowDescriptor); + LOG.trace("Storing flowDescriptor with table ID : {} and flow ID : {} for flow hash : {}", + flowDescriptor.getTableKey().getId(), flowDescriptor.getFlowId().getValue(), flowRegistryKey.hashCode()); + flowRegistry.put(flowRegistryKey, flowDescriptor); } catch (IllegalArgumentException ex) { - if (hasMark(flowRegistry.inverse().get(flowDescriptor))) { - // We are probably doing update of flow, but without changing flow ID or table ID, so we need to replace - // old value with new value, but keep the old value marked for removal - flowRegistry.forcePut(correctFlowRegistryKey, flowDescriptor); - return; - } - - // We are trying to store new flow to flow registry, but we already have different flow with same flow ID - // stored in registry, so we need to create alien ID for this new flow here. - LOG.warn("Flow with flow ID {} already exists in table {}, generating alien flow ID", flowDescriptor.getFlowId().getValue(), - flowDescriptor.getTableKey().getId()); - - flowRegistry.put( - correctFlowRegistryKey, - FlowDescriptorFactory.create( - flowDescriptor.getTableKey().getId(), - createAlienFlowId(flowDescriptor.getTableKey().getId()))); + LOG.warn("Flow with flowId {} already exists in table {}", flowDescriptor.getFlowId().getValue(), + flowDescriptor.getTableKey().getId()); + final FlowId newFlowId = createAlienFlowId(flowDescriptor.getTableKey().getId()); + final FlowDescriptor newFlowDescriptor = FlowDescriptorFactory. + create(flowDescriptor.getTableKey().getId(), newFlowId); + flowRegistry.put(flowRegistryKey, newFlowDescriptor); } } @Override - @GuardedBy("this") - public synchronized void forEachEntry(final BiConsumer consumer) { - flowRegistry.forEach(consumer); + public void update(final FlowRegistryKey newFlowRegistryKey, final FlowDescriptor flowDescriptor) { + LOG.trace("Updating the entry with hash: {}", newFlowRegistryKey.hashCode()); + flowRegistry.forcePut(newFlowRegistryKey, flowDescriptor); } @Override - @GuardedBy("this") - public synchronized void store(final FlowRegistryKey flowRegistryKey) { - if (Objects.isNull(retrieveDescriptor(flowRegistryKey))) { - // We do not found flow in flow registry, that means it do not have any ID already assigned, so we need - // to generate new alien flow ID here. - LOG.debug("Flow descriptor for flow hash : {} not found, generating alien flow ID", flowRegistryKey.hashCode()); + public FlowId storeIfNecessary(final FlowRegistryKey flowRegistryKey) { + LOG.trace("Trying to retrieve flow ID for flow hash : {}", flowRegistryKey.hashCode()); + + // First, try to get FlowDescriptor from flow registry + FlowDescriptor flowDescriptor = retrieveIdForFlow(flowRegistryKey); + + // We was not able to retrieve FlowDescriptor, so we will at least try to generate it + if (flowDescriptor == null) { + LOG.trace("Flow descriptor for flow hash : {} not found, generating alien flow ID", flowRegistryKey.hashCode()); final short tableId = flowRegistryKey.getTableId(); final FlowId alienFlowId = createAlienFlowId(tableId); - final FlowDescriptor flowDescriptor = FlowDescriptorFactory.create(tableId, alienFlowId); + flowDescriptor = FlowDescriptorFactory.create(tableId, alienFlowId); // Finally we got flowDescriptor, so now we will store it to registry, // so next time we won't need to generate it again - storeDescriptor(flowRegistryKey, flowDescriptor); + store(flowRegistryKey, flowDescriptor); } - } - @Override - @GuardedBy("this") - public synchronized void addMark(final FlowRegistryKey flowRegistryKey) { - LOG.trace("Removing flow descriptor for flow hash : {}", flowRegistryKey.hashCode()); - marks.add(flowRegistryKey); + return flowDescriptor.getFlowId(); } @Override - @GuardedBy("this") - public synchronized boolean hasMark(final FlowRegistryKey flowRegistryKey) { - return Objects.nonNull(flowRegistryKey) && marks.contains(correctFlowRegistryKey(marks, flowRegistryKey)); - - } - - @Override - @GuardedBy("this") - public synchronized void processMarks() { - // Remove all flows that was marked for removal from flow registry and clear all marks. - marks.forEach(flowRegistry::remove); - marks.clear(); - } - - @Override - @GuardedBy("this") - public synchronized void forEach(final Consumer consumer) { - flowRegistry.keySet().forEach(consumer); + public void removeDescriptor(final FlowRegistryKey flowRegistryKey) { + LOG.trace("Removing flow descriptor for flow hash : {}", flowRegistryKey.hashCode()); + flowRegistry.remove(flowRegistryKey); } @Override - @GuardedBy("this") - public synchronized int size() { - return flowRegistry.size(); + public Map getAllFlowDescriptors() { + return Collections.unmodifiableMap(flowRegistry); } @Override - @GuardedBy("this") - public synchronized void close() { + public void close() { final Iterator>>> iterator = lastFillFutures.iterator(); - // We need to force interrupt and clear all running futures that are trying to read flow IDs from datastore - while (iterator.hasNext()) { + while(iterator.hasNext()) { final ListenableFuture>> next = iterator.next(); boolean success = next.cancel(true); LOG.trace("Cancelling filling flow registry with flows job {} with result: {}", next, success); @@ -266,44 +229,11 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { } flowRegistry.clear(); - marks.clear(); - } - - @GuardedBy("this") - private FlowRegistryKey correctFlowRegistryKey(final Collection flowRegistryKeys, final FlowRegistryKey key) { - if (Objects.isNull(key)) { - return null; - } - - if (!flowRegistryKeys.contains(key)) { - // If we failed to compare FlowRegistryKey by hashCode, try to retrieve correct FlowRegistryKey - // from set of keys using custom comparator method for Match. This case can occur when we have different - // augmentations on extensions, or switch returned things like IP address or port in different format that - // we sent. - if (LOG.isTraceEnabled()) { - LOG.trace("Failed to retrieve flow descriptor for flow hash : {}, trying with custom equals method", key.hashCode()); - } - - for (final FlowRegistryKey flowRegistryKey : flowRegistryKeys) { - if (key.equals(flowRegistryKey)) { - return flowRegistryKey; - } - } - } - - // If we failed to find key at all or key is already present in set of keys, just return original key - return key; } @VisibleForTesting static FlowId createAlienFlowId(final short tableId) { final String alienId = ALIEN_SYSTEM_FLOW_ID + tableId + '-' + UNACCOUNTED_FLOWS_COUNTER.incrementAndGet(); - LOG.debug("Created alien flow id {} for table id {}", alienId, tableId); return new FlowId(alienId); } - - @VisibleForTesting - Map getAllFlowDescriptors() { - return flowRegistry; - } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImpl.java index 6fb6d5c9ec..1698929974 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImpl.java @@ -8,60 +8,49 @@ package org.opendaylight.openflowplugin.impl.registry.group; -import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.function.Consumer; import org.opendaylight.openflowplugin.api.openflow.registry.group.DeviceGroupRegistry; import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.GroupId; +/** + * Created by Martin Bobak <mbobak@cisco.com> on 15.4.2015. + */ public class DeviceGroupRegistryImpl implements DeviceGroupRegistry { - private final List groupIds = Collections.synchronizedList(new ArrayList<>()); - private final List marks = Collections.synchronizedList(new ArrayList<>()); + private final List groupIdList = new ArrayList<>(); + private final List marks = new ArrayList<>(); @Override public void store(final GroupId groupId) { - groupIds.add(groupId); + groupIdList.add(groupId); } @Override - public void addMark(final GroupId groupId) { + public void markToBeremoved(final GroupId groupId) { marks.add(groupId); } @Override - public boolean hasMark(final GroupId groupId) { - return marks.contains(groupId); - } - - @Override - public void processMarks() { - groupIds.removeAll(marks); - marks.clear(); - } - - @Override - public void forEach(final Consumer consumer) { - synchronized (groupIds) { - groupIds.forEach(consumer); + public void removeMarked() { + synchronized (groupIdList) { + groupIdList.removeAll(marks); } + marks.clear(); } @Override - public int size() { - return groupIds.size(); + public List getAllGroupIds() { + return groupIdList; } @Override public void close() { - groupIds.clear(); - marks.clear(); - } - - @VisibleForTesting - List getAllGroupIds() { - return groupIds; + synchronized (groupIdList) { + groupIdList.clear(); + } + synchronized (marks) { + marks.clear(); + } } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImpl.java index a29dc3cb9d..c5e2192a53 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImpl.java @@ -8,18 +8,18 @@ package org.opendaylight.openflowplugin.impl.registry.meter; -import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.function.Consumer; import org.opendaylight.openflowplugin.api.openflow.registry.meter.DeviceMeterRegistry; import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.types.rev130918.MeterId; +/** + * Created by Martin Bobak <mbobak@cisco.com> on 15.4.2015. + */ public class DeviceMeterRegistryImpl implements DeviceMeterRegistry { - private final List meterIds = Collections.synchronizedList(new ArrayList<>()); - private final List marks = Collections.synchronizedList(new ArrayList<>()); + private final List meterIds = new ArrayList<>(); + private final List marks = new ArrayList<>(); @Override public void store(final MeterId meterId) { @@ -27,41 +27,32 @@ public class DeviceMeterRegistryImpl implements DeviceMeterRegistry { } @Override - public void addMark(final MeterId meterId) { + public void markToBeremoved(final MeterId meterId) { marks.add(meterId); } @Override - public boolean hasMark(final MeterId meterId) { - return marks.contains(meterId); - } - - @Override - public void processMarks() { - meterIds.removeAll(marks); - marks.clear(); - } - - @Override - public void forEach(final Consumer consumer) { + public void removeMarked() { synchronized (meterIds) { - meterIds.forEach(consumer); + meterIds.removeAll(marks); + } + synchronized (marks) { + marks.clear(); } } @Override - public int size() { - return meterIds.size(); + public List getAllMeterIds() { + return meterIds; } @Override public void close() { - meterIds.clear(); - marks.clear(); - } - - @VisibleForTesting - List getAllMeterIds() { - return meterIds; + synchronized (meterIds) { + meterIds.clear(); + } + synchronized (marks) { + marks.clear(); + } } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallback.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallback.java index a516bccc7f..da8d74275d 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallback.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallback.java @@ -112,7 +112,6 @@ final class MultipartRequestOnTheFlyCallback extends AbstractRequestCallback flowPath = createFlowPath(flowDescriptor, deviceContext.getDeviceInfo().getNodeInstanceIdentifier()); @@ -228,25 +228,16 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource { final OriginalFlow original = input.getOriginalFlow(); final FlowRegistryKey origFlowRegistryKey = FlowRegistryKeyFactory.create(deviceContext.getDeviceInfo().getVersion(), original); final FlowRegistryKey updatedFlowRegistryKey = FlowRegistryKeyFactory.create(deviceContext.getDeviceInfo().getVersion(), updated); - final FlowDescriptor origFlowDescriptor = deviceFlowRegistry.retrieveDescriptor(origFlowRegistryKey); + final FlowDescriptor origFlowDescriptor = deviceFlowRegistry.retrieveIdForFlow(origFlowRegistryKey); final boolean isUpdate = Objects.nonNull(origFlowDescriptor); - final FlowDescriptor updatedFlowDescriptor; - - if (Objects.nonNull(input.getFlowRef())) { - updatedFlowDescriptor = FlowDescriptorFactory.create(updated.getTableId(), input.getFlowRef().getValue().firstKeyOf(Flow.class).getId()); - } else { - if (isUpdate) { - updatedFlowDescriptor = origFlowDescriptor; - } else { - deviceFlowRegistry.store(updatedFlowRegistryKey); - updatedFlowDescriptor = deviceFlowRegistry.retrieveDescriptor(updatedFlowRegistryKey); - } - } - + final FlowId fLowId = Objects.nonNull(input.getFlowRef()) + ? input.getFlowRef().getValue().firstKeyOf(Flow.class).getId() + : isUpdate ? origFlowDescriptor.getFlowId() : deviceFlowRegistry.storeIfNecessary(updatedFlowRegistryKey); + final FlowDescriptor updatedFlowDescriptor = FlowDescriptorFactory.create(updated.getTableId(), fLowId); if (isUpdate) { - deviceFlowRegistry.addMark(origFlowRegistryKey); - deviceFlowRegistry.storeDescriptor(updatedFlowRegistryKey, updatedFlowDescriptor); + deviceFlowRegistry.removeDescriptor(origFlowRegistryKey); + deviceFlowRegistry.store(updatedFlowRegistryKey, updatedFlowDescriptor); } if (itemLifecycleListener != null) { diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImpl.java index 3bbe45aeef..a6264a1ba7 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImpl.java @@ -126,7 +126,7 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource if (LOG.isDebugEnabled()) { LOG.debug("Group remove with id={} finished without error", input.getGroupId().getValue()); } - removeGroup.getDeviceRegistry().getDeviceGroupRegistry().addMark(input.getGroupId()); + removeGroup.getDeviceRegistry().getDeviceGroupRegistry().markToBeremoved(input.getGroupId()); removeIfNecessaryFromDS(input.getGroupId()); } else { if (LOG.isDebugEnabled()) { diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImpl.java index 1a4ac4c3db..02f72dc6ab 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImpl.java @@ -119,6 +119,7 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource @Override public Future> removeMeter(final RemoveMeterInput input) { + removeMeter.getDeviceRegistry().getDeviceMeterRegistry().markToBeremoved(input.getMeterId()); final ListenableFuture> resultFuture = removeMeter.handleServiceCall(input); Futures.addCallback(resultFuture, new FutureCallback>() { @Override @@ -127,7 +128,6 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource if (LOG.isDebugEnabled()) { LOG.debug("Meter remove with id={} finished without error", input.getMeterId()); } - removeMeter.getDeviceRegistry().getDeviceMeterRegistry().addMark(input.getMeterId()); removeIfNecessaryFromDS(input.getMeterId()); } else { if (LOG.isDebugEnabled()) { @@ -164,4 +164,4 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource private static KeyedInstanceIdentifier createMeterPath(final MeterId meterId, final KeyedInstanceIdentifier nodePath) { return nodePath.augmentation(FlowCapableNode.class).child(org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.meters.Meter.class, new MeterKey(meterId)); } -} +} \ No newline at end of file diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java index 9666080794..bb789b6271 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java @@ -321,7 +321,6 @@ public final class StatisticsGatheringUtils { writeFlowStatistics(data, deviceInfo, flowRegistry, txFacade); txFacade.submitTransaction(); EventsTimeCounter.markEnd(eventIdentifier); - flowRegistry.processMarks(); return Boolean.TRUE; }); } @@ -340,8 +339,7 @@ public final class StatisticsGatheringUtils { final short tableId = flowStat.getTableId(); final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowBuilder.build()); - registry.store(flowRegistryKey); - final FlowId flowId = registry.retrieveDescriptor(flowRegistryKey).getFlowId(); + final FlowId flowId = registry.storeIfNecessary(flowRegistryKey); final FlowKey flowKey = new FlowKey(flowId); flowBuilder.setKey(flowKey); @@ -524,11 +522,11 @@ public final class StatisticsGatheringUtils { final DeviceMeterRegistry meterRegistry, final InstanceIdentifier flowNodeIdent, final TxFacade txFacade) throws TransactionChainClosedException { - meterRegistry.forEach(meterId -> { + for (final MeterId meterId : meterRegistry.getAllMeterIds()) { final InstanceIdentifier meterIdent = flowNodeIdent.child(Meter.class, new MeterKey(meterId)); txFacade.addDeleteToTxChain(LogicalDatastoreType.OPERATIONAL, meterIdent); - }); - meterRegistry.processMarks(); + } + meterRegistry.removeMarked(); } private static void processGroupDescStats( @@ -560,11 +558,11 @@ public final class StatisticsGatheringUtils { final TxFacade txFacade, final InstanceIdentifier flowNodeIdent, final DeviceGroupRegistry groupRegistry) throws TransactionChainClosedException { - groupRegistry.forEach(groupId -> { + for (final GroupId groupId : groupRegistry.getAllGroupIds()) { final InstanceIdentifier groupIdent = flowNodeIdent.child(Group.class, new GroupKey(groupId)); txFacade.addDeleteToTxChain(LogicalDatastoreType.OPERATIONAL, groupIdent); - }); - groupRegistry.processMarks(); + } + groupRegistry.removeMarked(); } private static void processGroupStatistics( diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java index e6d6181fb6..a106ad93e5 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java @@ -20,6 +20,7 @@ import org.opendaylight.openflowplugin.extension.api.path.MatchPath; import org.opendaylight.openflowplugin.impl.registry.flow.FlowRegistryKeyFactory; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.ConvertorExecutor; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.FlowStatsResponseConvertorData; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.VersionDatapathIdConvertorData; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.match.MatchReactor; import org.opendaylight.yang.gen.v1.urn.opendaylight.direct.statistics.rev160511.GetFlowStatisticsInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.direct.statistics.rev160511.GetFlowStatisticsOutput; @@ -167,7 +168,6 @@ public class FlowDirectStatisticsService extends AbstractDirectStatisticsService .addAugmentation(FlowStatisticsData.class, flowStatisticsDataBld.build()); final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(getVersion(), flowBuilder.build()); - getDeviceRegistry().getDeviceFlowRegistry().store(flowRegistryKey); - return getDeviceRegistry().getDeviceFlowRegistry().retrieveDescriptor(flowRegistryKey).getFlowId(); + return getDeviceRegistry().getDeviceFlowRegistry().storeIfNecessary(flowRegistryKey); } } diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java index 95e454a980..e5a3167bc5 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java @@ -478,7 +478,7 @@ public class DeviceContextImplTest { // insert flow+flowId into local registry final FlowRegistryKey flowRegKey = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowRemovedMdsalBld.build()); final FlowDescriptor flowDescriptor = FlowDescriptorFactory.create((short) 0, new FlowId("ut-ofp:f456")); - deviceContext.getDeviceFlowRegistry().storeDescriptor(flowRegKey, flowDescriptor); + deviceContext.getDeviceFlowRegistry().store(flowRegKey, flowDescriptor); // plug in lifecycleListener final ItemLifecycleListener itemLifecycleListener = Mockito.mock(ItemLifecycleListener.class); @@ -531,9 +531,9 @@ public class DeviceContextImplTest { public void testOnDeviceDisconnected() throws Exception { final DeviceTerminationPhaseHandler deviceContextClosedHandler = mock(DeviceTerminationPhaseHandler.class); - assertEquals(0, deviceContext.getDeviceFlowRegistry().size()); - assertEquals(0, deviceContext.getDeviceGroupRegistry().size()); - assertEquals(0, deviceContext.getDeviceMeterRegistry().size()); + assertEquals(0, deviceContext.getDeviceFlowRegistry().getAllFlowDescriptors().size()); + assertEquals(0, deviceContext.getDeviceGroupRegistry().getAllGroupIds().size()); + assertEquals(0, deviceContext.getDeviceMeterRegistry().getAllMeterIds().size()); } diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java index d4f5906426..250d13615c 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java @@ -80,7 +80,7 @@ public class DeviceFlowRegistryImplTest { descriptor = FlowDescriptorFactory.create(key.getTableId(), new FlowId("ut:1")); Assert.assertEquals(0, deviceFlowRegistry.getAllFlowDescriptors().size()); - deviceFlowRegistry.storeDescriptor(key, descriptor); + deviceFlowRegistry.store(key, descriptor); Assert.assertEquals(1, deviceFlowRegistry.getAllFlowDescriptors().size()); } @@ -113,7 +113,7 @@ public class DeviceFlowRegistryImplTest { order.verify(readOnlyTransaction).read(LogicalDatastoreType.OPERATIONAL, path); assertTrue(allFlowDescriptors.containsKey(key)); - deviceFlowRegistry.addMark(key); + deviceFlowRegistry.removeDescriptor(key); } @Test @@ -166,23 +166,23 @@ public class DeviceFlowRegistryImplTest { @Test public void testRetrieveIdForFlow() throws Exception { - Assert.assertEquals(descriptor, deviceFlowRegistry.retrieveDescriptor(key)); + Assert.assertEquals(descriptor, deviceFlowRegistry.retrieveIdForFlow(key)); } @Test public void testStore() throws Exception { //store the same key with different value final FlowDescriptor descriptor2 = FlowDescriptorFactory.create(key.getTableId(), new FlowId("ut:2")); - deviceFlowRegistry.storeDescriptor(key, descriptor2); + deviceFlowRegistry.store(key, descriptor2); Assert.assertEquals(1, deviceFlowRegistry.getAllFlowDescriptors().size()); - Assert.assertEquals("ut:2", deviceFlowRegistry.retrieveDescriptor(key).getFlowId().getValue()); + Assert.assertEquals("ut:2", deviceFlowRegistry.retrieveIdForFlow(key).getFlowId().getValue()); // store new key with old value final FlowAndStatisticsMapList flowStats = TestFlowHelper.createFlowAndStatisticsMapListBuilder(2).build(); final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(OFConstants.OFP_VERSION_1_3, flowStats); - deviceFlowRegistry.storeDescriptor(key2, descriptor); + deviceFlowRegistry.store(key2, descriptor); Assert.assertEquals(2, deviceFlowRegistry.getAllFlowDescriptors().size()); - Assert.assertEquals("ut:1", deviceFlowRegistry.retrieveDescriptor(key2).getFlowId().getValue()); + Assert.assertEquals("ut:1", deviceFlowRegistry.retrieveIdForFlow(key2).getFlowId().getValue()); } @Test @@ -190,28 +190,26 @@ public class DeviceFlowRegistryImplTest { FlowId newFlowId; //store existing key - deviceFlowRegistry.store(key); - newFlowId = deviceFlowRegistry.retrieveDescriptor(key).getFlowId(); + newFlowId = deviceFlowRegistry.storeIfNecessary(key); Assert.assertEquals(1, deviceFlowRegistry.getAllFlowDescriptors().size()); - Assert.assertEquals(descriptor, deviceFlowRegistry.retrieveDescriptor(key)); + Assert.assertEquals(descriptor, deviceFlowRegistry.retrieveIdForFlow(key)); Assert.assertEquals(descriptor.getFlowId(), newFlowId); //store new key final String alienPrefix = "#UF$TABLE*2-"; final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(OFConstants.OFP_VERSION_1_3, TestFlowHelper.createFlowAndStatisticsMapListBuilder(2).build()); - deviceFlowRegistry.store(key2); - newFlowId = deviceFlowRegistry.retrieveDescriptor(key2).getFlowId(); + newFlowId = deviceFlowRegistry.storeIfNecessary(key2); Assert.assertTrue(newFlowId.getValue().startsWith(alienPrefix)); - Assert.assertTrue(deviceFlowRegistry.retrieveDescriptor(key2).getFlowId().getValue().startsWith(alienPrefix)); + Assert.assertTrue(deviceFlowRegistry.retrieveIdForFlow(key2).getFlowId().getValue().startsWith(alienPrefix)); Assert.assertEquals(2, deviceFlowRegistry.getAllFlowDescriptors().size()); } @Test public void testRemoveDescriptor() throws Exception { - deviceFlowRegistry.addMark(key); - Assert.assertEquals(1, deviceFlowRegistry.getAllFlowDescriptors().size()); + deviceFlowRegistry.removeDescriptor(key); + Assert.assertEquals(0, deviceFlowRegistry.getAllFlowDescriptors().size()); } @Test diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImplTest.java index 6b62e9f169..a0d74b0499 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/group/DeviceGroupRegistryImplTest.java @@ -40,29 +40,29 @@ public class DeviceGroupRegistryImplTest { @Test public void testRemoveMarked() throws Exception { - deviceGroupRegistry.addMark(groupId); - deviceGroupRegistry.processMarks(); + deviceGroupRegistry.markToBeremoved(groupId); + deviceGroupRegistry.removeMarked(); Assert.assertEquals(0, deviceGroupRegistry.getAllGroupIds().size()); } @Test public void testRemoveMarkedNegative() throws Exception { - deviceGroupRegistry.addMark(groupId2); - deviceGroupRegistry.processMarks(); + deviceGroupRegistry.markToBeremoved(groupId2); + deviceGroupRegistry.removeMarked(); Assert.assertEquals(1, deviceGroupRegistry.getAllGroupIds().size()); } @Test public void testClose() throws Exception { - deviceGroupRegistry.addMark(groupId); + deviceGroupRegistry.markToBeremoved(groupId); deviceGroupRegistry.close(); Assert.assertEquals(0, deviceGroupRegistry.getAllGroupIds().size()); deviceGroupRegistry.store(groupId); Assert.assertEquals(1, deviceGroupRegistry.getAllGroupIds().size()); - deviceGroupRegistry.processMarks(); + deviceGroupRegistry.removeMarked(); Assert.assertEquals(1, deviceGroupRegistry.getAllGroupIds().size()); } -} +} \ No newline at end of file diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImplTest.java index dbc3e3aedc..08bd755c85 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/meter/DeviceMeterRegistryImplTest.java @@ -40,29 +40,29 @@ public class DeviceMeterRegistryImplTest { @Test public void testRemoveMarked() throws Exception { - deviceMeterRegistry.addMark(meterId); - deviceMeterRegistry.processMarks(); + deviceMeterRegistry.markToBeremoved(meterId); + deviceMeterRegistry.removeMarked(); Assert.assertEquals(0, deviceMeterRegistry.getAllMeterIds().size()); } @Test public void testRemoveMarkedNegative() throws Exception { - deviceMeterRegistry.addMark(meterId2); - deviceMeterRegistry.processMarks(); + deviceMeterRegistry.markToBeremoved(meterId2); + deviceMeterRegistry.removeMarked(); Assert.assertEquals(1, deviceMeterRegistry.getAllMeterIds().size()); } @Test public void testClose() throws Exception { - deviceMeterRegistry.addMark(meterId); + deviceMeterRegistry.markToBeremoved(meterId); deviceMeterRegistry.close(); Assert.assertEquals(0, deviceMeterRegistry.getAllMeterIds().size()); deviceMeterRegistry.store(meterId); Assert.assertEquals(1, deviceMeterRegistry.getAllMeterIds().size()); - deviceMeterRegistry.processMarks(); + deviceMeterRegistry.removeMarked(); Assert.assertEquals(1, deviceMeterRegistry.getAllMeterIds().size()); } -} +} \ No newline at end of file diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java index 692c29feaa..5d15eff5cd 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java @@ -9,6 +9,7 @@ package org.opendaylight.openflowplugin.impl.services; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -137,7 +138,7 @@ public class MultipartRequestOnTheFlyCallbackTest { when(mockedDeviceContext.getDeviceState()).thenReturn(mockedDeviceState); when(mockedDeviceContext.getDeviceInfo()).thenReturn(mockedDeviceInfo); when(mockedDeviceContext.getDeviceFlowRegistry()).thenReturn(mockedFlowRegistry); - when(mockedFlowRegistry.retrieveDescriptor(Matchers.any(FlowRegistryKey.class))).thenReturn(mockedFlowDescriptor); + when(mockedFlowRegistry.retrieveIdForFlow(Matchers.any(FlowRegistryKey.class))).thenReturn(mockedFlowDescriptor); final InstanceIdentifier nodePath = mockedDeviceInfo.getNodeInstanceIdentifier().augmentation(FlowCapableNode.class); final FlowCapableNodeBuilder flowNodeBuilder = new FlowCapableNodeBuilder(); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImplTest.java index 8418e13ac8..89cbef6d7e 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImplTest.java @@ -285,7 +285,8 @@ public class SalFlowServiceImplTest extends TestCase { when(mockedFlowDescriptor.getFlowId()).thenReturn(flowId); when(mockedFlowDescriptor.getTableKey()).thenReturn(new TableKey(DUMMY_TABLE_ID)); - when(deviceFlowRegistry.retrieveDescriptor(Matchers.any(FlowRegistryKey.class))).thenReturn(mockedFlowDescriptor); + when(deviceFlowRegistry.storeIfNecessary(Matchers.any(FlowRegistryKey.class))).thenReturn(flowId); + when(deviceFlowRegistry.retrieveIdForFlow(Matchers.any(FlowRegistryKey.class))).thenReturn(mockedFlowDescriptor); } private void verifyOutput(Future> rpcResultFuture) throws ExecutionException, InterruptedException { diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImplTest.java index bca07192fb..f0814d8962 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalGroupServiceImplTest.java @@ -129,7 +129,7 @@ public class SalGroupServiceImplTest extends ServiceMocking { salGroupService.removeGroup(removeGroupInput); verify(mockedRequestContextStack).createRequestContext(); - verify(mockedDeviceGroupRegistry).addMark(eq(dummyGroupId)); + verify(mockedDeviceGroupRegistry).markToBeremoved(eq(dummyGroupId)); if (itemLifecycleListener != null) { verify(itemLifecycleListener).onRemoved(Matchers.>any()); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImplTest.java index 0cb47df026..ea20a88647 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/SalMeterServiceImplTest.java @@ -130,7 +130,7 @@ public class SalMeterServiceImplTest extends ServiceMocking { salMeterService.removeMeter(removeMeterInput); verify(mockedRequestContextStack).createRequestContext(); - verify(mockedDeviceMeterRegistry).addMark(eq(dummyMeterId)); + verify(mockedDeviceMeterRegistry).markToBeremoved(eq(dummyMeterId)); if (itemLifecycleListener != null) { verify(itemLifecycleListener).onRemoved(Matchers.>any()); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtilsTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtilsTest.java index 3d6add0f1f..607adc0627 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtilsTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtilsTest.java @@ -180,7 +180,7 @@ public class StatisticsGatheringUtilsTest { when(deviceContext.getDeviceFlowRegistry()).thenReturn(deviceFlowRegistry); when(deviceContext.getDeviceGroupRegistry()).thenReturn(deviceGroupRegistry); when(deviceContext.getDeviceMeterRegistry()).thenReturn(deviceMeterRegistry); - when(deviceFlowRegistry.retrieveDescriptor(Matchers.any(FlowRegistryKey.class))).thenReturn(flowDescriptor); + when(deviceFlowRegistry.retrieveIdForFlow(Matchers.any(FlowRegistryKey.class))).thenReturn(flowDescriptor); when(deviceContext.getReadTransaction()).thenReturn(readTx); when(txFacade.getReadTransaction()).thenReturn(readTx); when(deviceContext.getPrimaryConnectionContext()).thenReturn(connectionAdapter); @@ -284,7 +284,7 @@ public class StatisticsGatheringUtilsTest { verify(deviceContext, Mockito.never()).addDeleteToTxChain( Matchers.eq(LogicalDatastoreType.OPERATIONAL), Matchers.>any()); - verify(deviceGroupRegistry).processMarks(); + verify(deviceGroupRegistry).removeMarked(); verify(deviceGroupRegistry).store(storedGroupId); verify(txFacade).writeToTransaction( Matchers.eq(LogicalDatastoreType.OPERATIONAL), Matchers.eq(groupPath), Matchers.any(Group.class)); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsServiceTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsServiceTest.java index e2a1869af9..9dd7d6aaf9 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsServiceTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsServiceTest.java @@ -22,8 +22,6 @@ import java.util.Collections; import java.util.List; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.openflowplugin.api.openflow.registry.flow.DeviceFlowRegistry; -import org.opendaylight.openflowplugin.impl.registry.flow.FlowDescriptorFactory; -import org.opendaylight.openflowplugin.impl.statistics.services.direct.AbstractDirectStatisticsServiceTest; import org.opendaylight.yang.gen.v1.urn.opendaylight.direct.statistics.rev160511.GetFlowStatisticsInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.direct.statistics.rev160511.GetFlowStatisticsOutput; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowId; @@ -47,7 +45,7 @@ public class FlowDirectStatisticsServiceTest extends AbstractDirectStatisticsSer public void setUp() throws Exception { service = new FlowDirectStatisticsService(requestContextStack, deviceContext, convertorManager); final DeviceFlowRegistry registry = mock(DeviceFlowRegistry.class); - when(registry.retrieveDescriptor(any())).thenReturn(FlowDescriptorFactory.create(TABLE_NO, new FlowId("1"))); + when(registry.storeIfNecessary(any())).thenReturn(new FlowId("1")); when(deviceContext.getDeviceFlowRegistry()).thenReturn(registry); } @@ -109,4 +107,4 @@ public class FlowDirectStatisticsServiceTest extends AbstractDirectStatisticsSer service.storeStatistics(output); verify(deviceContext).writeToTransactionWithParentsSlow(eq(LogicalDatastoreType.OPERATIONAL), any(), any()); } -} +} \ No newline at end of file