From: Anil Vishnoi <avishnoi@in.ibm.com>
Date: Thu, 21 Aug 2014 13:30:36 +0000 (+0530)
Subject: Bug 1593 - Flow Statistics manager is updating store with incorrect key
X-Git-Tag: release/helium~167^2
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;ds=sidebyside;h=refs%2Fchanges%2F35%2F10135%2F4;p=controller.git

Bug 1593 - Flow Statistics manager is updating store with incorrect key
Statistics manager cache the metadata related to stats data of all the entities (flow,group,
meter,queue) like when was their respective stats were updated in operational data store.
If statistics manager see that stats are not being updated from last two stats fetch cycle,
it consider it stale and remove it. Object that it uses as keys for hash map cache contains
some variant data (counters/durations) that varies in every stats fetch cycle. It creats
duplicate entry in the cache for the same entity. After two fetch cycle statistics manager
find old entry stale and removes it, but in actual that's not the case. This exception is
thrown when it tries to remove the stats related to next duplicate entry (which was already
removed ). Fixed it by using invariant key for caching the metadata to avoid the duplication.

Change-Id: Iad5823ab70938a66fd3d024dc921b74e0852c204
Signed-off-by: Anil Vishnoi <avishnoi@in.ibm.com>
---

diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java
index e23ad0acdb..838aeb7dc3 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/AbstractStatsTracker.java
@@ -97,14 +97,15 @@ abstract class AbstractStatsTracker<I, K> {
     }
     protected abstract void cleanupSingleStat(DataModificationTransaction trans, K item);
     protected abstract K updateSingleStat(DataModificationTransaction trans, I item);
+    protected abstract K createInvariantKey(K item);
     public abstract void request();
 
     public final synchronized void updateStats(List<I> list) {
 
         final DataModificationTransaction trans = startTransaction();
-
         for (final I item : list) {
-            trackedItems.put(updateSingleStat(trans, item), requestCounter);
+            K key = updateSingleStat(trans, item);
+            trackedItems.put(createInvariantKey(key), requestCounter);
         }
 
         trans.commit();
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsEntry.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsEntry.java
index b5b39d94d2..c43c1ebaca 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsEntry.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsEntry.java
@@ -56,4 +56,9 @@ final class FlowStatsEntry {
             return false;
         return true;
     }
+
+    @Override
+    public String toString() {
+        return "FlowStatsEntry [tableId=" + tableId + ", flow=" + flow + "]";
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsTracker.java
index e92d0bd625..d540f115c3 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowStatsTracker.java
@@ -301,4 +301,13 @@ final class FlowStatsTracker extends AbstractListeningStatsTracker<FlowAndStatis
         }
         return flowCookieMap;
     }
+
+    @Override
+    protected FlowStatsEntry createInvariantKey(final FlowStatsEntry item) {
+        FlowBuilder newFlow = new FlowBuilder();
+        newFlow.setId(item.getFlow().getId());
+        newFlow.setKey(item.getFlow().getKey());
+        newFlow.fieldsFrom(item.getFlow());
+        return new FlowStatsEntry(item.getTableId(),newFlow.build());
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowTableStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowTableStatsTracker.java
index a160f6d467..2e85058041 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowTableStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/FlowTableStatsTracker.java
@@ -21,6 +21,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev13
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev131215.GetFlowTablesStatisticsInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev131215.OpendaylightFlowTableStatisticsService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev131215.flow.table.and.statistics.map.FlowTableAndStatisticsMap;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev131215.flow.table.and.statistics.map.FlowTableAndStatisticsMapBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev131215.flow.table.statistics.FlowTableStatistics;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.table.statistics.rev131215.flow.table.statistics.FlowTableStatisticsBuilder;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -70,4 +71,12 @@ final class FlowTableStatsTracker extends AbstractStatsTracker<FlowTableAndStati
             requestHelper(flowTableStatsService.getFlowTablesStatistics(input.build()));
         }
     }
+
+    @Override
+    protected FlowTableAndStatisticsMap createInvariantKey(FlowTableAndStatisticsMap item) {
+        FlowTableAndStatisticsMapBuilder flowTableAndStatisticsMapBuilder = new FlowTableAndStatisticsMapBuilder();
+        flowTableAndStatisticsMapBuilder.setTableId(item.getTableId());
+        flowTableAndStatisticsMapBuilder.setKey(item.getKey());
+        return flowTableAndStatisticsMapBuilder.build();
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupDescStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupDescStatsTracker.java
index a775b73169..11c6d4c22d 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupDescStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupDescStatsTracker.java
@@ -114,4 +114,10 @@ final class GroupDescStatsTracker extends AbstractListeningStatsTracker<GroupDes
 
         super.start(dbs);
     }
+
+    @Override
+    protected GroupDescStats createInvariantKey(GroupDescStats item) {
+        // No invariant data exist in the group description stats.
+        return item;
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupStatsTracker.java
index c3c23c9b58..f3ba45643c 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/GroupStatsTracker.java
@@ -17,6 +17,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.group.statistics.rev131111.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.statistics.rev131111.OpendaylightGroupStatisticsService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.statistics.rev131111.group.statistics.GroupStatisticsBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.statistics.reply.GroupStats;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.statistics.reply.GroupStatsBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.Group;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.GroupBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.GroupKey;
@@ -105,4 +106,12 @@ final class GroupStatsTracker extends AbstractListeningStatsTracker<GroupStats,
 
         super.start(dbs);
     }
+
+    @Override
+    protected GroupStats createInvariantKey(GroupStats item) {
+        GroupStatsBuilder groupStatsBuilder = new GroupStatsBuilder();
+        groupStatsBuilder.setKey(item.getKey());
+        groupStatsBuilder.setGroupId(item.getGroupId());
+        return groupStatsBuilder.build();
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterConfigStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterConfigStatsTracker.java
index ea412a09e1..88139fc606 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterConfigStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterConfigStatsTracker.java
@@ -109,4 +109,10 @@ final class MeterConfigStatsTracker extends AbstractListeningStatsTracker<MeterC
 
         super.start(dbs);
     }
+
+    @Override
+    protected MeterConfigStats createInvariantKey(MeterConfigStats item) {
+        // No invariant data exist in the meter config stats.
+        return item;
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterStatsTracker.java
index be53a38f18..f43ec693cd 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/MeterStatsTracker.java
@@ -20,6 +20,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.statistics.rev131111.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.statistics.rev131111.OpendaylightMeterStatisticsService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.statistics.rev131111.nodes.node.meter.MeterStatisticsBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.types.rev130918.meter.statistics.reply.MeterStats;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.meter.types.rev130918.meter.statistics.reply.MeterStatsBuilder;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
@@ -112,4 +113,12 @@ final class MeterStatsTracker extends AbstractListeningStatsTracker<MeterStats,
 
         super.start(dbs);
     }
+
+    @Override
+    protected MeterStats createInvariantKey(MeterStats item) {
+        MeterStatsBuilder meterStatsBuilder = new MeterStatsBuilder();
+        meterStatsBuilder.setKey(item.getKey());
+        meterStatsBuilder.setMeterId(item.getMeterId());
+        return meterStatsBuilder.build();
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeConnectorStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeConnectorStatsTracker.java
index e4490eaa3c..2e2bf1cd51 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeConnectorStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/NodeConnectorStatsTracker.java
@@ -17,6 +17,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.port.statistics.rev131214.G
 import org.opendaylight.yang.gen.v1.urn.opendaylight.port.statistics.rev131214.OpendaylightPortStatisticsService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.port.statistics.rev131214.flow.capable.node.connector.statistics.FlowCapableNodeConnectorStatisticsBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.port.statistics.rev131214.node.connector.statistics.and.port.number.map.NodeConnectorStatisticsAndPortNumberMap;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.port.statistics.rev131214.node.connector.statistics.and.port.number.map.NodeConnectorStatisticsAndPortNumberMapBuilder;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -83,4 +84,12 @@ final class NodeConnectorStatsTracker extends AbstractStatsTracker<NodeConnector
             requestHelper(portStatsService.getAllNodeConnectorsStatistics(input.build()));
         }
     }
+
+    @Override
+    protected NodeConnectorStatisticsAndPortNumberMap createInvariantKey(NodeConnectorStatisticsAndPortNumberMap item) {
+        NodeConnectorStatisticsAndPortNumberMapBuilder ncStatsBuilder = new NodeConnectorStatisticsAndPortNumberMapBuilder();
+        ncStatsBuilder.setNodeConnectorId(item.getNodeConnectorId());
+        ncStatsBuilder.setKey(item.getKey());
+        return ncStatsBuilder.build();
+    }
 }
diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/QueueStatsTracker.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/QueueStatsTracker.java
index 03f7808c8f..4a589703be 100644
--- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/QueueStatsTracker.java
+++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/QueueStatsTracker.java
@@ -152,4 +152,10 @@ final class QueueStatsTracker extends AbstractListeningStatsTracker<QueueIdAndSt
 
         super.start(dbs);
     }
+
+    @Override
+    protected QueueStatsEntry createInvariantKey(QueueStatsEntry item) {
+        // No invariant data exist in the group description stats.
+        return item;
+    }
 }