Bug5822:Ensuring that the flowId, is unique for a flow

in a table

* also optimising map/list accresses
*reducing addFlow rpc latency

Change-Id: Ida607fc7134c0488c7c3bcc82e38966be2325e71
Signed-off-by: Shuva Kar <shuva.jyoti.kar@ericsson.com>

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 1e574f64592dd25dc7b439bca7a08777bf40548f..9506c96d512bd35d0b52a9221a2a817dbe26b427 100644 (file)
--- 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
@@ -40,4 +40,4 @@ public interface DeviceFlowRegistry extends AutoCloseable {
 
     @Override
     void close();
-}
+}
\ No newline at end of file

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 da83a6e20337f233bb2e7b7ba131aafa20de5146..ae072563ae28807d392f2015b2def15d0f0af403 100644 (file)
--- 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
@@ -9,6 +9,9 @@ package org.opendaylight.openflowplugin.impl.registry.flow;
 
 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;
@@ -51,7 +54,8 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
     private static final String ALIEN_SYSTEM_FLOW_ID = "#UF$TABLE*";
     private static final AtomicInteger UNACCOUNTED_FLOWS_COUNTER = new AtomicInteger(0);
 
-    private final ConcurrentMap<FlowRegistryKey, FlowDescriptor> flowRegistry = new TrieMap<>();
+
+    private final BiMap<FlowRegistryKey, FlowDescriptor> flowRegistry = HashBiMap.create();
     @GuardedBy("marks")
     private final Collection<FlowRegistryKey> marks = new HashSet<>();
     private final DataBroker dataBroker;
@@ -147,6 +151,7 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
     public FlowDescriptor retrieveIdForFlow(final FlowRegistryKey flowRegistryKey) {
         LOG.trace("Retrieving flowDescriptor for flow hash: {}", flowRegistryKey.hashCode());
         FlowDescriptor flowDescriptor = flowRegistry.get(flowRegistryKey);
+        // Get FlowDescriptor from flow registry
         if(flowDescriptor == null){
             for(Map.Entry<FlowRegistryKey, FlowDescriptor> fd : flowRegistry.entrySet()) {
                 if (fd.getKey().equals(flowRegistryKey)) {
@@ -155,14 +160,24 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
                 }
             }
         }
-        // Get FlowDescriptor from flow registry
         return flowDescriptor;
     }
 
+
     @Override
     public void store(final FlowRegistryKey flowRegistryKey, final FlowDescriptor 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);
+        LOG.trace("Storing flowDescriptor with table ID : {} and flow ID : {} for flow hash : {}",
+                flowDescriptor.getTableKey().getId(), flowDescriptor.getFlowId().getValue(), flowRegistryKey.hashCode());
+        try {
+            flowRegistry.put(flowRegistryKey, flowDescriptor);
+        } catch (IllegalArgumentException ex) {
+            LOG.error("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
@@ -232,4 +247,4 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
         final String alienId = ALIEN_SYSTEM_FLOW_ID + tableId + '-' + UNACCOUNTED_FLOWS_COUNTER.incrementAndGet();
         return new FlowId(alienId);
     }
-}
+}
\ No newline at end of file

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowDescriptorFactory.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowDescriptorFactory.java
index a751be2e5fb5df1675b66e29998af05aa367a375..68ad18ee5e6fe2fafba2cece66dbe0db8899bd4d 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowDescriptorFactory.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowDescriptorFactory.java
@@ -8,6 +8,7 @@
 
 package org.opendaylight.openflowplugin.impl.registry.flow;
 
+import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowDescriptor;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowId;
@@ -39,6 +40,20 @@ public class FlowDescriptorFactory {
             this.tableKey = tableKey;
         }
 
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            FlowDescriptorDto that = (FlowDescriptorDto) o;
+            return Objects.equal(flowId, that.flowId) &&
+                    Objects.equal(tableKey, that.tableKey);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(flowId, tableKey);
+        }
+
         @Override
         public FlowId getFlowId() {
             return flowId;
@@ -49,4 +64,4 @@ public class FlowDescriptorFactory {
             return tableKey;
         }
     }
-}
+}
\ No newline at end of file

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java
index 5eb2457688c6a6de4115cafccd19fb28e3beae63..8000e8e1245e8bf30e50079081b342872b48b3a0 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java
@@ -78,25 +78,26 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource {
     @Override
     public Future<RpcResult<AddFlowOutput>> addFlow(final AddFlowInput input) {
         final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(input);
-        final FlowId flowId;
-        final FlowDescriptor flowDescriptor;
 
-        if (Objects.nonNull(input.getFlowRef())) {
-            flowId = input.getFlowRef().getValue().firstKeyOf(Flow.class, FlowKey.class).getId();
-            flowDescriptor = FlowDescriptorFactory.create(input.getTableId(), flowId);
-            deviceContext.getDeviceFlowRegistry().store(flowRegistryKey, flowDescriptor);
-        } else {
-            flowId = deviceContext.getDeviceFlowRegistry().storeIfNecessary(flowRegistryKey);
-            flowDescriptor = FlowDescriptorFactory.create(input.getTableId(), flowId);
-        }
-
-        LOG.trace("Calling add flow for flow with ID ={}.", flowId);
         final ListenableFuture<RpcResult<AddFlowOutput>> future =
                 flowAdd.processFlowModInputBuilders(flowAdd.toFlowModInputs(input));
         Futures.addCallback(future, new FutureCallback<RpcResult<AddFlowOutput>>() {
             @Override
             public void onSuccess(final RpcResult<AddFlowOutput> rpcResult) {
                 if (rpcResult.isSuccessful()) {
+                    final FlowId flowId;
+                    final FlowDescriptor flowDescriptor;
+
+                    if (Objects.nonNull(input.getFlowRef())) {
+                        flowId = input.getFlowRef().getValue().firstKeyOf(Flow.class, FlowKey.class).getId();
+                        flowDescriptor = FlowDescriptorFactory.create(input.getTableId(), flowId);
+                        deviceContext.getDeviceFlowRegistry().store(flowRegistryKey, flowDescriptor);
+
+                    } else {
+                        flowId = deviceContext.getDeviceFlowRegistry().storeIfNecessary(flowRegistryKey);
+                        flowDescriptor = FlowDescriptorFactory.create(input.getTableId(), flowId);
+                    }
+
                     if(LOG.isDebugEnabled()) {
                         LOG.debug("flow add with id={},finished without error,", flowId.getValue());
                     }
@@ -108,15 +109,14 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource {
                     }
                 } else {
                     deviceContext.getDeviceFlowRegistry().markToBeremoved(flowRegistryKey);
-                    LOG.error("flow add failed for id={}, errors={}", flowId.getValue(),
+                    LOG.error("flow add failed for flow={}, errors={}", input.toString(),
                             errorsToString(rpcResult.getErrors()));
                 }
             }
 
             @Override
             public void onFailure(final Throwable throwable) {
-                deviceContext.getDeviceFlowRegistry().markToBeremoved(flowRegistryKey);
-                LOG.error("Service call for adding flow with  id={} failed, reason {} .", flowId.getValue(), throwable);
+               LOG.error("Service call for adding flow={} failed, reason {} .", input.toString(), throwable);
             }
         });
 
@@ -234,8 +234,6 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource {
                             itemLifecycleListener.onAdded(flowPath, flowBuilder.build());
                         }
                     }
-
-
                 }
             }
 
@@ -254,4 +252,4 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource {
                 .child(Table.class, flowDescriptor.getTableKey())
                 .child(Flow.class, new FlowKey(flowDescriptor.getFlowId()));
     }
-}
+}
\ No newline at end of file

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 2d8e7e246af7fa1573ebd42e029093e2d8579c88..1e1ff0e9f5e53f2adb583902da28d68f488b4779 100644 (file)
--- 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
@@ -60,7 +60,6 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource
 
     @Override
     public Future<RpcResult<AddGroupOutput>> addGroup(final AddGroupInput input) {
-        deviceContext.getDeviceGroupRegistry().store(input.getGroupId());
         final ListenableFuture<RpcResult<AddGroupOutput>> resultFuture = addGroup.handleServiceCall(input);
         Futures.addCallback(resultFuture, new FutureCallback<RpcResult<AddGroupOutput>>() {
             @Override
@@ -69,9 +68,9 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource
                     if(LOG.isDebugEnabled()) {
                         LOG.debug("group add with id={} finished without error", input.getGroupId().getValue());
                     }
+                    deviceContext.getDeviceGroupRegistry().store(input.getGroupId());
                     addIfNecessaryToDS(input.getGroupId(), input);
                 } else {
-                    deviceContext.getDeviceGroupRegistry().markToBeremoved(input.getGroupId());
                     LOG.error("group add with id={} failed, errors={}", input.getGroupId().getValue(),
                             errorsToString(result.getErrors()));
                 }
@@ -79,7 +78,6 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource
 
             @Override
             public void onFailure(Throwable t) {
-                deviceContext.getDeviceGroupRegistry().markToBeremoved(input.getGroupId());
                 LOG.error("Service call for group add failed for id={}. Exception: {}", input.getGroupId().getValue(), t);
             }
         });
@@ -117,7 +115,6 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource
 
     @Override
     public Future<RpcResult<RemoveGroupOutput>> removeGroup(final RemoveGroupInput input) {
-        removeGroup.getDeviceRegistry().getDeviceGroupRegistry().markToBeremoved(input.getGroupId());
         final ListenableFuture<RpcResult<RemoveGroupOutput>> resultFuture = removeGroup.handleServiceCall(input);
         Futures.addCallback(resultFuture, new FutureCallback<RpcResult<RemoveGroupOutput>>() {
             @Override
@@ -126,6 +123,7 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource
                     if(LOG.isDebugEnabled()) {
                         LOG.debug("Group remove for id {} succeded", input.getGroupId().getValue());
                     }
+                    removeGroup.getDeviceRegistry().getDeviceGroupRegistry().markToBeremoved(input.getGroupId());
                     removeIfNecessaryFromDS(input.getGroupId());
                 }else{
                     LOG.error("group remove failed with id={}, errors={}", input.getGroupId().getValue(),
@@ -175,4 +173,4 @@ public class SalGroupServiceImpl implements SalGroupService, ItemLifeCycleSource
         }
         return errors.toString();
     }
-}
+}
\ No newline at end of file

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 c605d3690f33d884bf27df8168d27ef7870e4799..d96e2190120bf5812059096991907e7e8f8683a1 100644 (file)
--- 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
@@ -60,18 +60,18 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource
 
     @Override
     public Future<RpcResult<AddMeterOutput>> addMeter(final AddMeterInput input) {
-        deviceContext.getDeviceMeterRegistry().store(input.getMeterId());
+
         final ListenableFuture<RpcResult<AddMeterOutput>> resultFuture = addMeter.handleServiceCall(input);
         Futures.addCallback(resultFuture, new FutureCallback<RpcResult<AddMeterOutput>>() {
             @Override
             public void onSuccess(@Nullable RpcResult<AddMeterOutput> result) {
                 if (result.isSuccessful()) {
-                    if(LOG.isDebugEnabled()) {
+                   if(LOG.isDebugEnabled()) {
                         LOG.debug("Meter add finished without error, id={}", input.getMeterId());
                     }
+                    deviceContext.getDeviceMeterRegistry().store(input.getMeterId());
                     addIfNecessaryToDS(input.getMeterId(),input);
                 } else {
-                    deviceContext.getDeviceMeterRegistry().markToBeremoved(input.getMeterId());
                     LOG.error("Meter add with id {} failed with error {}", input.getMeterId(),
                             errorsToString(result.getErrors()));
                 }
@@ -79,8 +79,7 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource
 
             @Override
             public void onFailure(Throwable t) {
-                deviceContext.getDeviceMeterRegistry().markToBeremoved(input.getMeterId());
-                LOG.error("Meter add failed for id={}. Exception {}", input.getMeterId(), t);
+                 LOG.error("Meter add failed for id={}. Exception {}", input.getMeterId(), t);
             }
         });
 
@@ -90,7 +89,6 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource
     @Override
     public Future<RpcResult<UpdateMeterOutput>> updateMeter(final UpdateMeterInput input) {
         final ListenableFuture<RpcResult<UpdateMeterOutput>> resultFuture = updateMeter.handleServiceCall(input.getUpdatedMeter());
-
         Futures.addCallback(resultFuture, new FutureCallback<RpcResult<UpdateMeterOutput>>() {
 
             @Override
@@ -177,4 +175,4 @@ public class SalMeterServiceImpl implements SalMeterService, ItemLifeCycleSource
         }
         return errors.toString();
     }
-}
+}
\ No newline at end of file