From 8e9240b888d0568b7fb477715698518ca15fa0be Mon Sep 17 00:00:00 2001 From: Vaclav Demcak Date: Thu, 25 Sep 2014 23:03:30 +0200 Subject: [PATCH] BUG 2049 DataStore failure in StatisticsManager BUG 2101 When L2 switch installed, its flows are interpreted as new flows with each stats request * hashCode is not safe in general so we'd like to prevent Collisions how we can * we change HashBuilding to KeyBuilding from toString method * toString is generated in CompileTime * it can not be change during JVM run * every one use same YANG model version and jar files * and the same aproach could by use across claster * add cleaning disconnected Node from StatListeningCommiter implementers Change-Id: I645e9f07382af0b293bc43698446b7b84b95bbde Signed-off-by: Vaclav Demcak --- .../manager/StatListeningCommiter.java | 10 +++ .../impl/StatAbstractListenCommit.java | 5 ++ .../manager/impl/StatListenCommitFlow.java | 86 ++++++++----------- .../manager/impl/StatisticsManagerImpl.java | 30 ++----- 4 files changed, 62 insertions(+), 69 deletions(-) diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatListeningCommiter.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatListeningCommiter.java index 7589c72a45..be3d40246b 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatListeningCommiter.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatListeningCommiter.java @@ -9,7 +9,9 @@ package org.opendaylight.controller.md.statistics.manager; import org.opendaylight.controller.md.sal.binding.api.DataChangeListener; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node; import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.binding.NotificationListener; /** @@ -30,5 +32,13 @@ import org.opendaylight.yangtools.yang.binding.NotificationListener; public interface StatListeningCommiter extends DataChangeListener, StatNotifyCommiter { + /** + * All StatListeningCommiter implementer has to clean its actual state + * for all cached data related to disconnected node. + * Method prevents unwanted dataStore changes. + * + * @param nodeIdent + */ + void cleanForDisconnect(InstanceIdentifier nodeIdent); } diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatAbstractListenCommit.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatAbstractListenCommit.java index 6ebf944b22..6db73d5ddc 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatAbstractListenCommit.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/impl/StatAbstractListenCommit.java @@ -108,6 +108,11 @@ public abstract class StatAbstractListenCommit nodeIdent) { + mapNodesForDelete.remove(nodeIdent); + } + @Override public void close() { if (listenerRegistration != null) { 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 e54fcc6fa2..a19081db25 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 @@ -86,6 +86,8 @@ public class StatListenCommitFlow extends StatAbstractListenCommit fNode = Optional.absent(); try { fNode = tx.read(LogicalDatastoreType.OPERATIONAL, fNodeIdent).checkedGet(); - } - catch (final ReadFailedException e) { + } catch (final ReadFailedException e) { LOG.debug("Read Operational/DS for FlowCapableNode fail! {}", fNodeIdent, e); return; } @@ -192,6 +193,20 @@ public class StatListenCommitFlow extends StatAbstractListenCommit, Integer> listAliens = mapNodesForDelete.get(nodeIdent); + if (listAliens != null) { + for (final Entry, Integer> nodeForDelete : listAliens.entrySet()) { + final Integer lifeIndex = nodeForDelete.getValue(); + if (nodeForDelete.getValue() > 0) { + nodeForDelete.setValue(Integer.valueOf(lifeIndex.intValue() - 1)); + } else { + final InstanceIdentifier flowNodeIdent = nodeForDelete.getKey(); + mapNodesForDelete.get(nodeIdent).remove(flowNodeIdent); + tx.delete(LogicalDatastoreType.OPERATIONAL, flowNodeIdent); + } + } + } /* Notification for continue collecting statistics */ notifyToCollectNextStatistics(nodeIdent); } @@ -246,14 +261,13 @@ public class StatListenCommitFlow extends StatAbstractListenCommit tableRef; final TableKey tableKey; @@ -304,7 +319,7 @@ public class StatListenCommitFlow extends StatAbstractListenCommit {}", flowHashId.getKey(), flowHashId.getFlowId()); } } @@ -338,12 +353,7 @@ public class StatListenCommitFlow extends StatAbstractListenCommit table = readLatestConfiguration(tableRef); - try { - table = trans.read(LogicalDatastoreType.CONFIGURATION, tableRef).checkedGet(); - } catch (final ReadFailedException e) { - table = Optional.absent(); - } + final Optional table = readLatestConfiguration(tableRef); List localList = null; if(table.isPresent()) { localList = table.get().getFlow(); @@ -378,7 +388,7 @@ public class StatListenCommitFlow extends StatAbstractListenCommit nodeIdent = tableRef.firstIdentifierOf(Node.class); + final Map, Integer> nodeDeleteMap = mapNodesForDelete.get(nodeIdent); final Map listForRemove = getRemovalList(); - final Optional
configTable = readLatestConfiguration(tableRef); - List configFlows = Collections.emptyList(); - if (configTable.isPresent() && configTable.get().getFlow() != null) { - configFlows = new ArrayList<>(configTable.get().getFlow()); - } for (final Entry entryForRemove : listForRemove.entrySet()) { final FlowKey flowKey = new FlowKey(entryForRemove.getValue()); final InstanceIdentifier flowRef = tableRef.child(Flow.class, flowKey); - final InstanceIdentifier flowStatIdent = flowRef.augmentation(FlowStatisticsData.class); - if (flowKey.getId().getValue().startsWith(ALIEN_SYSTEM_FLOW_ID)) { - final InstanceIdentifier nodeIdent = tableRef.firstIdentifierOf(Node.class); - final Integer lifeIndex = mapNodesForDelete.get(nodeIdent).remove(flowRef); + if (nodeDeleteMap != null && flowKey.getId().getValue().startsWith(ALIEN_SYSTEM_FLOW_ID)) { + final Integer lifeIndex = nodeDeleteMap.get(flowRef); if (lifeIndex > 0) { - mapNodesForDelete.get(nodeIdent).put(flowRef, Integer.valueOf(lifeIndex.intValue() - 1)); - break; - } - } else { - if (configFlows.remove(flowRef)) { - /* Node is still presented in Config/DataStore - probably lost some multipart msg */ break; + } else { + nodeDeleteMap.remove(flowRef); } } - final Optional flowStatNodeCheck; - try { - flowStatNodeCheck = tx.read(LogicalDatastoreType.OPERATIONAL, flowStatIdent).checkedGet(); - } - catch (final ReadFailedException e) { - LOG.debug("Read FlowStatistics {} in Operational/DS fail! Statisticscan not beupdated.", flowStatIdent, e); - break; - } - if (flowStatNodeCheck.isPresent()) { - /* Node isn't new and it has not been removed yet */ - final InstanceIdentifier flHashIdent = tableRef.augmentation(FlowHashIdMapping.class).child(FlowHashIdMap.class, entryForRemove.getKey()); - tx.delete(LogicalDatastoreType.OPERATIONAL, flowRef); - tx.delete(LogicalDatastoreType.OPERATIONAL, flHashIdent); - } + final InstanceIdentifier flHashIdent = + tableRef.augmentation(FlowHashIdMapping.class).child(FlowHashIdMap.class, entryForRemove.getKey()); + tx.delete(LogicalDatastoreType.OPERATIONAL, flowRef); + tx.delete(LogicalDatastoreType.OPERATIONAL, flHashIdent); } } } 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 8430549be1..247703f8ac 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 @@ -22,7 +22,6 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction; import org.opendaylight.controller.md.sal.common.api.data.TransactionChain; -import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.controller.md.statistics.manager.StatListeningCommiter; import org.opendaylight.controller.md.statistics.manager.StatNodeRegistration; import org.opendaylight.controller.md.statistics.manager.StatNotifyCommiter; @@ -68,7 +67,7 @@ public class StatisticsManagerImpl implements StatisticsManager, Runnable { private final static Logger LOG = LoggerFactory.getLogger(StatisticsManagerImpl.class); - private static final int QUEUE_DEPTH = 500; + private static final int QUEUE_DEPTH = 1000; private static final int MAX_BATCH = 1; private final BlockingQueue dataStoreOperQueue = new LinkedBlockingDeque<>(QUEUE_DEPTH); @@ -205,31 +204,22 @@ public class StatisticsManagerImpl implements StatisticsManager, Runnable { LOG.trace("Processed {} operations, submitting transaction {}", ops, tx.getIdentifier()); - try { tx.submit().checkedGet(); - } catch (final TransactionCommitFailedException e) { - LOG.warn("Stat DataStoreOperation unexpected State!", e); - txChain.close(); - txChain = dataBroker.createTransactionChain(StatisticsManagerImpl.this); - cleanDataStoreOperQueue(); - } - } - catch (final IllegalStateException e) { - LOG.warn("Stat DataStoreOperation unexpected State!", e); - } - catch (final InterruptedException e) { + } catch (final InterruptedException e) { LOG.warn("Stat Manager DS Operation thread interupted!", e); finishing = true; - } - catch (final Exception e) { - LOG.warn("Stat DataStore Operation executor fail!", e); + } catch (final Exception e) { + LOG.warn("Unhandled exception during processing statistics. Restarting transaction chain.", e); + txChain.close(); + txChain = dataBroker.createTransactionChain(StatisticsManagerImpl.this); + cleanDataStoreOperQueue(); } } // Drain all events, making sure any blocked threads are unblocked cleanDataStoreOperQueue(); } - private void cleanDataStoreOperQueue() { + private synchronized void cleanDataStoreOperQueue() { // Drain all events, making sure any blocked threads are unblocked while (! dataStoreOperQueue.isEmpty()) { dataStoreOperQueue.poll(); @@ -240,9 +230,6 @@ public class StatisticsManagerImpl implements StatisticsManager, Runnable { public void onTransactionChainFailed(final TransactionChain chain, final AsyncTransaction transaction, final Throwable cause) { LOG.warn("Failed to export Flow Capable Statistics, Transaction {} failed.",transaction.getIdentifier(),cause); - txChain.close(); - txChain = dataBroker.createTransactionChain(StatisticsManagerImpl.this); - cleanDataStoreOperQueue(); } @Override @@ -294,6 +281,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