From 9b8c69b490e82fafdd996af5ea06c8f5113a0666 Mon Sep 17 00:00:00 2001 From: Vaclav Demcak Date: Mon, 29 Sep 2014 22:47:24 +0200 Subject: [PATCH] BUG-2078 Stats not being collected from all nodes, dangling nodes left in oper data store after mininet disconnects BUG-2049 DataStore failure in StatisticsManager * fix performance issues * fix occurrence zombie nodes * fix NPE for StatCollector * fix NPE for Meter/Group lists Change-Id: I95a821aaf308bdb6e989c4a740aa014d5aaab4c2 Signed-off-by: Vaclav Demcak --- .../manager/StatisticsManagerActivator.java | 2 +- .../manager/impl/StatListenCommitFlow.java | 38 +++++++++++++--- .../manager/impl/StatListenCommitGroup.java | 8 ++-- .../manager/impl/StatListenCommitMeter.java | 8 ++-- .../manager/impl/StatListenCommitQueue.java | 2 +- .../manager/impl/StatNotifyCommitPort.java | 11 +++-- .../manager/impl/StatNotifyCommitTable.java | 18 +++++--- .../manager/impl/StatRpcMsgManagerImpl.java | 43 ++++++++----------- .../manager/impl/StatisticsManagerImpl.java | 5 ++- 9 files changed, 83 insertions(+), 52 deletions(-) diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsManagerActivator.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsManagerActivator.java index 912a6eda4b..c505af49e6 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsManagerActivator.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsManagerActivator.java @@ -31,7 +31,7 @@ public class StatisticsManagerActivator extends AbstractBindingAwareProvider { /* TODO move it to ConfigSubsystem */ private static final long DEFAULT_MIN_REQUEST_NET_MONITOR_INTERVAL = 3000L; - private static final int MAX_NODES_FOR_COLLECTOR = 8; + private static final int MAX_NODES_FOR_COLLECTOR = 16; private StatisticsManager statsProvider; diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitFlow.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitFlow.java index a19081db25..230425999e 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitFlow.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitFlow.java @@ -35,6 +35,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.no import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.nodes.node.table.FlowHashIdMapBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.nodes.node.table.FlowHashIdMapKey; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.Table; +import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.TableBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.TableKey; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.FlowBuilder; @@ -139,8 +140,9 @@ public class StatListenCommitFlow extends StatAbstractListenCommit fNodeIdent = InstanceIdentifier.create(Nodes.class) .child(Node.class, new NodeKey(nodeId)).augmentation(FlowCapableNode.class); - final InstanceIdentifier tableStatRef = fNodeIdent - .child(Table.class, table.getKey()).augmentation(AggregateFlowStatisticsData.class); + final InstanceIdentifier tableRef = fNodeIdent.child(Table.class, table.getKey()); + final InstanceIdentifier tableStatRef = tableRef + .augmentation(AggregateFlowStatisticsData.class); Optional fNode = Optional.absent(); try { fNode = tx.read(LogicalDatastoreType.OPERATIONAL, fNodeIdent).checkedGet(); @@ -149,7 +151,8 @@ public class StatListenCommitFlow extends StatAbstractListenCommit tableRef) { + final Table tableNew = new TableBuilder().setId(tableId).build(); + tx.merge(LogicalDatastoreType.OPERATIONAL, tableRef, tableNew); + } + @Override public void onFlowsStatisticsUpdate(final FlowsStatisticsUpdate notification) { final TransactionId transId = notification.getTransactionId(); @@ -327,11 +335,12 @@ public class StatListenCommitFlow extends StatAbstractListenCommit emptyList()).build(); - tx.merge(LogicalDatastoreType.OPERATIONAL, tableRef.augmentation(FlowHashIdMapping.class), emptyMapping, true); + tx.merge(LogicalDatastoreType.OPERATIONAL, tableRef.augmentation(FlowHashIdMapping.class), emptyMapping); tableEnsured = true; } } @@ -387,7 +396,7 @@ public class StatListenCommitFlow extends StatAbstractListenCommit nodeIdent = tableRef.firstIdentifierOf(Node.class); + final List> listMissingConfigFlows = notStatReportedConfigFlows(); final Map, Integer> nodeDeleteMap = mapNodesForDelete.get(nodeIdent); final Map listForRemove = getRemovalList(); for (final Entry entryForRemove : listForRemove.entrySet()) { @@ -433,6 +443,10 @@ public class StatListenCommitFlow extends StatAbstractListenCommit flHashIdent = tableRef.augmentation(FlowHashIdMapping.class).child(FlowHashIdMap.class, entryForRemove.getKey()); @@ -440,6 +454,18 @@ public class StatListenCommitFlow extends StatAbstractListenCommit> notStatReportedConfigFlows() { + if (configFlows != null) { + final List> returnList = new ArrayList<>(configFlows.size()); + for (final Flow confFlow : configFlows) { + final InstanceIdentifier confFlowIdent = tableRef.child(Flow.class, confFlow.getKey()); + returnList.add(confFlowIdent); + } + return returnList; + } + return Collections.emptyList(); + } } } diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitGroup.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitGroup.java index 41d97f080a..f351132f7f 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitGroup.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitGroup.java @@ -155,7 +155,7 @@ public class StatListenCommitGroup extends StatAbstractListenCommit existGroups = fNode.get().getGroup().isEmpty() - ? Collections. emptyList() : fNode.get().getGroup(); + final List existGroups = fNode.get().getGroup() != null + ? fNode.get().getGroup() : Collections. emptyList(); /* Add all existed groups paths - no updated paths has to be removed */ for (final Group group : existGroups) { if (deviceGroupKeys.remove(group.getKey())) { diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitMeter.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitMeter.java index e2ae7637c6..9c9de59a6a 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitMeter.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitMeter.java @@ -151,7 +151,7 @@ public class StatListenCommitMeter extends StatAbstractListenCommit existMeters = fNode.get().getMeter().isEmpty() - ? Collections. emptyList() : fNode.get().getMeter(); + final List existMeters = fNode.get().getMeter() != null + ? fNode.get().getMeter() : Collections. emptyList(); /* Add all existed groups paths - no updated paths has to be removed */ for (final Meter meter : existMeters) { if (deviceMeterKeys.remove(meter.getKey())) { diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitQueue.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitQueue.java index 29fe5d8e5e..e336f01874 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitQueue.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatListenCommitQueue.java @@ -142,7 +142,7 @@ public class StatListenCommitQueue extends StatAbstractListenCommit nodeConnectorIdent = nodeIdent.child(NodeConnector.class, key); final InstanceIdentifier nodeConnStatIdent = nodeConnectorIdent .augmentation(FlowCapableNodeConnectorStatisticsData.class); + final InstanceIdentifier flowCapNodeConnStatIdent = + nodeConnStatIdent.child(FlowCapableNodeConnectorStatistics.class); Optional fNodeConector; try { fNodeConector = tx.read(LogicalDatastoreType.OPERATIONAL, nodeConnectorIdent).checkedGet(); @@ -140,7 +143,9 @@ public class StatNotifyCommitPort extends StatAbstractNotifyCommit tStatIdent = fNodeIdent - .child(Table.class, new TableKey(tableStat.getTableId().getValue())) + final InstanceIdentifier
tableIdent = fNodeIdent + .child(Table.class, new TableKey(tableStat.getTableId().getValue())); + final Table table = new TableBuilder().setId(tableStat.getTableId().getValue()).build(); + trans.merge(LogicalDatastoreType.OPERATIONAL, tableIdent, table); + final InstanceIdentifier tableStatIdent = tableIdent .augmentation(FlowTableStatisticsData.class); - trans.put(LogicalDatastoreType.OPERATIONAL, tStatIdent, stats, true); + trans.merge(LogicalDatastoreType.OPERATIONAL, tableStatIdent, new FlowTableStatisticsDataBuilder().build()); + + final FlowTableStatistics stats = new FlowTableStatisticsBuilder(tableStat).build(); + final InstanceIdentifier tStatIdent = tableStatIdent.child(FlowTableStatistics.class); + trans.put(LogicalDatastoreType.OPERATIONAL, tStatIdent, stats); } } } diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatRpcMsgManagerImpl.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatRpcMsgManagerImpl.java index 8058554840..e53f494129 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatRpcMsgManagerImpl.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatRpcMsgManagerImpl.java @@ -15,10 +15,8 @@ import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; -import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; import org.opendaylight.controller.md.statistics.manager.StatRpcMsgManager; import org.opendaylight.controller.md.statistics.manager.StatisticsManager; -import org.opendaylight.controller.md.statistics.manager.StatisticsManager.StatDataStoreOperation; import org.opendaylight.controller.sal.binding.api.RpcConsumerRegistry; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.TableBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.TableKey; @@ -86,7 +84,6 @@ public class StatRpcMsgManagerImpl implements StatRpcMsgManager { private final long maxLifeForRequest = 50; /* 50 second */ private final int queueCapacity = 5000; - private final StatisticsManager manager; private final OpendaylightGroupStatisticsService groupStatsService; private final OpendaylightMeterStatisticsService meterStatsService; @@ -101,7 +98,7 @@ public class StatRpcMsgManagerImpl implements StatRpcMsgManager { public StatRpcMsgManagerImpl (final StatisticsManager manager, final RpcConsumerRegistry rpcRegistry, final long minReqNetMonitInt) { - this.manager = Preconditions.checkNotNull(manager, "StatisticManager can not be null!"); + Preconditions.checkArgument(manager != null, "StatisticManager can not be null!"); Preconditions.checkArgument(rpcRegistry != null, "RpcConsumerRegistry can not be null !"); groupStatsService = Preconditions.checkNotNull( rpcRegistry.getRpcService(OpendaylightGroupStatisticsService.class), @@ -322,30 +319,26 @@ public class StatRpcMsgManagerImpl implements StatRpcMsgManager { @Override public void getAggregateFlowStat(final NodeRef nodeRef, final TableId tableId) { - - manager.enqueue(new StatDataStoreOperation() { + Preconditions.checkArgument(nodeRef != null, "NodeRef can not be null!"); + Preconditions.checkArgument(tableId != null, "TableId can not be null!"); + final RpcJobsQueue getAggregateFlowStat = new RpcJobsQueue() { @Override - public void applyOperation(final ReadWriteTransaction tx) { - final RpcJobsQueue getAggregateFlowStat = new RpcJobsQueue() { - @Override - public Void call() throws Exception { - final GetAggregateFlowStatisticsFromFlowTableForAllFlowsInputBuilder builder = - new GetAggregateFlowStatisticsFromFlowTableForAllFlowsInputBuilder(); - builder.setNode(nodeRef); - builder.setTableId(tableId); - - final TableBuilder tbuilder = new TableBuilder(); - tbuilder.setId(tableId.getValue()); - tbuilder.setKey(new TableKey(tableId.getValue())); - registrationRpcFutureCallBack(flowStatsService - .getAggregateFlowStatisticsFromFlowTableForAllFlows(builder.build()), tbuilder.build(), nodeRef); - return null; - } - }; - addGetAllStatJob(getAggregateFlowStat); + public Void call() throws Exception { + final GetAggregateFlowStatisticsFromFlowTableForAllFlowsInputBuilder builder = + new GetAggregateFlowStatisticsFromFlowTableForAllFlowsInputBuilder(); + builder.setNode(nodeRef); + builder.setTableId(tableId); + + final TableBuilder tbuilder = new TableBuilder(); + tbuilder.setId(tableId.getValue()); + tbuilder.setKey(new TableKey(tableId.getValue())); + registrationRpcFutureCallBack(flowStatsService + .getAggregateFlowStatisticsFromFlowTableForAllFlows(builder.build()), tbuilder.build(), nodeRef); + return null; } - }); + }; + addGetAllStatJob(getAggregateFlowStat); } @Override diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatisticsManagerImpl.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatisticsManagerImpl.java index 247703f8ac..0f8030f620 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatisticsManagerImpl.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatisticsManagerImpl.java @@ -67,8 +67,8 @@ public class StatisticsManagerImpl implements StatisticsManager, Runnable { private final static Logger LOG = LoggerFactory.getLogger(StatisticsManagerImpl.class); - private static final int QUEUE_DEPTH = 1000; - private static final int MAX_BATCH = 1; + private static final int QUEUE_DEPTH = 5000; + private static final int MAX_BATCH = 100; private final BlockingQueue dataStoreOperQueue = new LinkedBlockingDeque<>(QUEUE_DEPTH); @@ -282,6 +282,7 @@ public class StatisticsManagerImpl implements StatisticsManager, Runnable { @Override public void disconnectedNodeUnregistration(final InstanceIdentifier nodeIdent) { flowListeningCommiter.cleanForDisconnect(nodeIdent); + for (final StatPermCollector collector : statCollectors) { if (collector.disconnectedNodeUnregistration(nodeIdent)) { if ( ! collector.hasActiveNodes()) { -- 2.36.6