From 7b0f145a355a8dbba7d54432299bfc2652062588 Mon Sep 17 00:00:00 2001 From: "K.V Suneelu Verma" Date: Sun, 5 Feb 2017 11:26:07 +0530 Subject: [PATCH] bug 7599 performance improvement for ucast macs Using the getModifiedChildren api on the modification to identify the modified children data under node. updated data and deleted data is stored in operational state for access by all the commands. Added the following accessor methods in operational state. List getUpdatedData(InstanceIdentifier key, Class cls) List getDeletedData(InstanceIdentifier key, Class cls) Change-Id: I76584d7c90323313da35e28a600019c05081f9d0 Signed-off-by: K.V Suneelu Verma --- .../HwvtepConnectionManager.java | 5 +- .../HwvtepDataChangeListener.java | 2 +- .../HwvtepReconciliationTask.java | 1 + .../transact/AbstractTransactCommand.java | 26 +++- .../transact/HwvtepOperationalState.java | 50 +++++++- .../transact/TransactCommandAggregator.java | 117 +++++++++++++++++- 6 files changed, 193 insertions(+), 8 deletions(-) diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java index 835d435d9..9ca92a88b 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java @@ -225,7 +225,10 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo LOG.info("Clients after put: {}", clients); } - public HwvtepConnectionInstance getConnectionInstance(ConnectionInfo key) { + public HwvtepConnectionInstance getConnectionInstance(final ConnectionInfo key) { + if (key == null) { + return null; + } ConnectionInfo connectionInfo = HwvtepSouthboundMapper.suppressLocalIpPort(key); return clients.get(connectionInfo); } diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepDataChangeListener.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepDataChangeListener.java index f5d96e386..ec4e18ad4 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepDataChangeListener.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepDataChangeListener.java @@ -296,7 +296,7 @@ public class HwvtepDataChangeListener implements ClusteredDataTreeChangeListener result.get(connection).add(change); } } else { - LOG.warn("Failed to get the connection of changed node: {}", node); + LOG.warn("Failed to get the connection of changed node: {}", node.getKey().getNodeId().getValue()); } } LOG.trace("Connection Change Map: {}", result); diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/reconciliation/configuration/HwvtepReconciliationTask.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/reconciliation/configuration/HwvtepReconciliationTask.java index 38379f54f..97dfe5b90 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/reconciliation/configuration/HwvtepReconciliationTask.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/reconciliation/configuration/HwvtepReconciliationTask.java @@ -54,6 +54,7 @@ public class HwvtepReconciliationTask extends ReconciliationTask { private void transactChangesToDevice(Collection> changes) { HwvtepOperationalState hwvtepOperationalState = new HwvtepOperationalState(db, connectionInstance, changes); + hwvtepOperationalState.setInReconciliation(true); connectionInstance.transact(new TransactCommandAggregator(hwvtepOperationalState,changes)); } diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java index 349cf3db2..39a543659 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/AbstractTransactCommand.java @@ -169,7 +169,12 @@ public abstract class AbstractTransactCommand change : changes) { final InstanceIdentifier key = change.getRootPath().getRootIdentifier(); - removed = getRemoved(change); + Class classType = (Class) getClassType(); + if (operationalState.isInReconciliation()) { + removed = getRemoved(change); + } else { + removed = (List) operationalState.getDeletedData(key, classType); + } removed.addAll(getCascadeDeleteData(change)); result.put(key, removed); } @@ -184,7 +189,14 @@ public abstract class AbstractTransactCommand change : changes) { InstanceIdentifier key = change.getRootPath().getRootIdentifier(); - result.put(key, getUpdated(change)); + Class classType = (Class) getClassType(); + List updated = null; + if (operationalState.isInReconciliation()) { + updated = getUpdated(change); + } else { + updated = (List) operationalState.getUpdatedData(key, classType); + } + result.put(key, updated); } } return result; @@ -222,10 +234,9 @@ public abstract class AbstractTransactCommand getUpdated(DataTreeModification change) { DataObjectModification mod = change.getRootNode(); - Node created = TransactUtils.getCreated(mod); Node updated = TransactUtils.getUpdated(mod); Node before = mod.getDataBefore(); - return diffOf(created, updated, before, false); + return diffOf(updated, before, false); } List diffOf(Node include, Node a, Node b, boolean compareKeyOnly) { @@ -273,6 +284,13 @@ public abstract class AbstractTransactCommand, Map> currentTxUUIDs = new ConcurrentHashMap<>(); private Map, Map> currentTxDeletedKeys = new ConcurrentHashMap<>(); + /* stores the modified and deleted data for each child type of each node id + Map + each updated/ deleted contains Map < child type, List> + child type is the child of hwvtep Global augmentation + */ + private Map, + Pair, List>, + Map, List>>> modifiedData = new HashMap<>(); + private boolean inReconciliation = false; + public HwvtepOperationalState(DataBroker db, HwvtepConnectionInstance connectionInstance, Collection> changes) { this.connectionInstance = connectionInstance; @@ -364,4 +375,41 @@ public class HwvtepOperationalState { return Collections.EMPTY_SET; } + public List getUpdatedData(final InstanceIdentifier key, + final Class cls) { + List result = null; + if (modifiedData.get(key) != null && modifiedData.get(key).getLeft() != null) { + result = modifiedData.get(key).getLeft().get(cls); + } + if (result == null) { + result = Collections.EMPTY_LIST; + } + return result; + } + + public List getDeletedData(final InstanceIdentifier key, + final Class cls) { + List result = null; + if (modifiedData.get(key) != null && modifiedData.get(key).getRight() != null) { + result = modifiedData.get(key).getRight().get(cls); + } + if (result == null) { + result = Collections.EMPTY_LIST; + } + return result; + } + + public void setModifiedData(final Map, + Pair, List>, + Map, List>>> modifiedData) { + this.modifiedData = modifiedData; + } + + public boolean isInReconciliation() { + return inReconciliation; + } + + public void setInReconciliation(boolean inReconciliation) { + this.inReconciliation = inReconciliation; + } } diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactCommandAggregator.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactCommandAggregator.java index b9b6b0121..0e07f3dac 100644 --- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactCommandAggregator.java +++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactCommandAggregator.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 China Telecom Beijing Research Institute and others. All rights reserved. + * Copyright (c) 2015, 2017 China Telecom Beijing Research Institute 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, @@ -8,21 +8,45 @@ package org.opendaylight.ovsdb.hwvtepsouthbound.transact; +import org.apache.commons.lang3.tuple.Pair; +import org.opendaylight.controller.md.sal.binding.api.DataObjectModification; import org.opendaylight.controller.md.sal.binding.api.DataTreeModification; import org.opendaylight.ovsdb.lib.operations.TransactionBuilder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalAugmentation; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.PhysicalSwitchAugmentation; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; +import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.Identifiable; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; public class TransactCommandAggregator implements TransactCommand { + private static final Logger LOG = LoggerFactory.getLogger(TransactCommandAggregator.class); + private List commands = new ArrayList(); + private final HwvtepOperationalState operationalState; + /* stores the modified and deleted data for each child type of each node id + Map + each updated/ deleted contains Map < child type, List> + child type is the child of hwvtep Global augmentation + */ + private final Map, + Pair, List>, + Map, List>>> modifiedData = new HashMap<>(); + public TransactCommandAggregator(HwvtepOperationalState state, Collection> changes) { + this.operationalState = state; + onDataTreeChanged(changes); commands.add(new PhysicalSwitchUpdateCommand(state,changes)); commands.add(new PhysicalSwitchRemoveCommand(state,changes)); commands.add(new LogicalSwitchUpdateCommand(state,changes)); @@ -59,4 +83,95 @@ public class TransactCommandAggregator implements TransactCommand { InstanceIdentifier key, Object... extraData) { } + + private void onDataTreeChanged(final Collection> changes) { + for (DataTreeModification change : changes) { + final InstanceIdentifier key = change.getRootPath().getRootIdentifier(); + final DataObjectModification mod = change.getRootNode(); + final Map, List> updatedData = new HashMap<>(); + final Map, List> deletedData = new HashMap<>(); + extractDataChanged(key, mod, updatedData, deletedData); + modifiedData.put(key, Pair.of(updatedData, deletedData)); + operationalState.setModifiedData(modifiedData); + } + } + + private void extractDataChanged(final InstanceIdentifier key, + final DataObjectModification mod, + final Map, List> updatedData, + final Map, List> deletedData) { + + extractDataChanged(mod.getModifiedChildren(), updatedData, deletedData); + DataObjectModification aug = mod.getModifiedAugmentation( + HwvtepGlobalAugmentation.class); + if (aug != null && getModificationType(aug) != null) { + extractDataChanged(aug.getModifiedChildren(), updatedData, deletedData); + } + DataObjectModification psAug = mod.getModifiedAugmentation( + PhysicalSwitchAugmentation.class); + if (psAug != null && getModificationType(psAug) != null) { + extractDataChanged(psAug.getModifiedChildren(), updatedData, deletedData); + } + } + + private void extractDataChanged(final Collection> children, + final Map, List> updatedData, + final Map, List> deletedData) { + if (children == null) { + return; + } + for (DataObjectModification child : children) { + DataObjectModification.ModificationType type = getModificationType(child); + if (type == null) { + continue; + } + InstanceIdentifier instanceIdentifier = null; + Class childClass = (Class) child.getDataType(); + InstanceIdentifier.PathArgument pathArgument = child.getIdentifier(); + switch (type) { + case WRITE: + case SUBTREE_MODIFIED: + DataObject dataAfter = child.getDataAfter(); + if (!(dataAfter instanceof Identifiable)) { + continue; + } + DataObject before = child.getDataBefore(); + if (Objects.equals(dataAfter, before)) { + /* + in cluster reboot scenarios, + application rewrites the data tx.put( logicalswitchiid, logicalswitch ) + that time it fires the update again ignoring such updates here + */ + continue; + } + Identifiable identifiable = (Identifiable) dataAfter; + addToUpdatedData(updatedData, childClass, identifiable); + break; + case DELETE: + DataObject dataBefore = child.getDataBefore(); + if (!(dataBefore instanceof Identifiable)) { + continue; + } + addToUpdatedData(deletedData, childClass, (Identifiable)dataBefore); + break; + } + } + } + + private void addToUpdatedData(Map, List> updatedData, + Class childClass, Identifiable identifiable) { + updatedData.computeIfAbsent(childClass, (cls) -> new ArrayList<>()); + updatedData.get(childClass).add(identifiable); + } + + private DataObjectModification.ModificationType getModificationType( + DataObjectModification mod) { + try { + return mod.getModificationType(); + } catch (IllegalStateException e) { + //not sure why this getter throws this exception, could be some mdsal bug + LOG.warn("Failed to get the modification type for mod {}", mod); + } + return null; + } } -- 2.36.6