Optimization to read the HwvtepNode from the cache.

JIRA: OVSDB-485

Currently in the HwvtepSouthbound plugin reads the HwvtepNode from the datastore.
Reading the HwvtepNode from the datastore ineffecient, even the same info
is present in HwvtepSouthbound plugin cache.

This review is avoid the HwvtepNode reads from the datastore and get it from the cache.
If the node is not present in the cache, then read from datastore.

Signed-off-by: Chandra Shekar S <chandra.shekar.s@ericsson.com>
Change-Id: I1f15ff31b195a79a1725e63882512b87d40c5838

diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepOperGlobalListener.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepOperGlobalListener.java
index 405a519d0893adf3ff878d1f509c7217e4057e1a..593ceb4813b406a299a8aaafc2e6f63ed9a6de2e 100644 (file)
--- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepOperGlobalListener.java
+++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepOperGlobalListener.java
@@ -28,7 +28,6 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,7 +39,7 @@ public class HwvtepOperGlobalListener implements ClusteredDataTreeChangeListener
     private ListenerRegistration<HwvtepOperGlobalListener> registration;
     private final HwvtepConnectionManager hcm;
     private final DataBroker db;
-    private final Map<YangInstanceIdentifier, Node> connectedNodes = new ConcurrentHashMap<>();
+    private static final Map<InstanceIdentifier<Node>, Node> CONNECTED_NODES = new ConcurrentHashMap<>();
 
     HwvtepOperGlobalListener(DataBroker db, HwvtepConnectionManager hcm) {
         LOG.info("Registering HwvtepOperGlobalListener");
@@ -69,15 +68,13 @@ public class HwvtepOperGlobalListener implements ClusteredDataTreeChangeListener
             InstanceIdentifier<Node> key = change.getRootPath().getRootIdentifier();
             DataObjectModification<Node> mod = change.getRootNode();
             InstanceIdentifier<Node> nodeIid = change.getRootPath().getRootIdentifier();
-            YangInstanceIdentifier entityId =
-                    HwvtepSouthboundUtil.getInstanceIdentifierCodec().getYangInstanceIdentifier(nodeIid);
             Node node = getCreated(mod);
             if (node != null) {
-                connectedNodes.put(entityId, node);
+                CONNECTED_NODES.put(key, node);
             }
             node = getRemoved(mod);
             if (node != null) {
-                connectedNodes.remove(entityId);
+                CONNECTED_NODES.remove(key);
                 HwvtepConnectionInstance connectionInstance = hcm.getConnectionInstanceFromNodeIid(nodeIid);
                 if (Objects.equals(connectionInstance.getConnectionInfo().getRemotePort(),
                         HwvtepSouthboundUtil.getRemotePort(node))) {
@@ -107,8 +104,8 @@ public class HwvtepOperGlobalListener implements ClusteredDataTreeChangeListener
         return null;
     }
 
-    public Map<YangInstanceIdentifier, Node> getConnectedNodes() {
-        return Collections.unmodifiableMap(connectedNodes);
+    public Map<InstanceIdentifier<Node>, Node> getConnectedNodes() {
+        return Collections.unmodifiableMap(CONNECTED_NODES);
     }
 
     private InstanceIdentifier<Node> getWildcardPath() {
@@ -118,4 +115,8 @@ public class HwvtepOperGlobalListener implements ClusteredDataTreeChangeListener
                         .child(Node.class);
         return path;
     }
+
+    public static Node getNode(InstanceIdentifier<Node> key) {
+        return CONNECTED_NODES.get(key);
+    }
 }

diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java
index d4463b447ef94d9b7bc4c8ecd0cf9712a7c536c4..bb98aa1df0acfd37021872453004e9417d567f3e 100644 (file)
--- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java
+++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java
@@ -14,6 +14,7 @@ import java.util.Collection;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
@@ -27,6 +28,7 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey;
+import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.Identifiable;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.impl.codec.DeserializationException;
@@ -69,13 +71,25 @@ public final class HwvtepSouthboundUtil {
 
     public static <D extends org.opendaylight.yangtools.yang.binding.DataObject> Optional<D> readNode(
                     ReadWriteTransaction transaction, final InstanceIdentifier<D> connectionIid) {
-        Optional<D> node = Optional.absent();
+        return readNode(transaction, LogicalDatastoreType.OPERATIONAL, connectionIid);
+    }
+
+    public static <D extends DataObject> Optional<D> readNode(ReadTransaction transaction,
+                                                              LogicalDatastoreType logicalDatastoreType,
+                                                              InstanceIdentifier<D> connectionIid) {
+        if (logicalDatastoreType == LogicalDatastoreType.OPERATIONAL) {
+            if (HwvtepOperGlobalListener.getNode((InstanceIdentifier<Node>) connectionIid) != null) {
+                return Optional.of((D)HwvtepOperGlobalListener.getNode((InstanceIdentifier<Node>)connectionIid));
+            } else {
+                LOG.debug("Node not available in cache. Read from datastore - {}", connectionIid);
+            }
+        }
         try {
-            node = transaction.read(LogicalDatastoreType.OPERATIONAL, connectionIid).checkedGet();
-        } catch (final ReadFailedException e) {
-            LOG.warn("Read Operational/DS for Node failed! {}", connectionIid, e);
+            return transaction.read(logicalDatastoreType, connectionIid).checkedGet();
+        } catch (ReadFailedException e) {
+            LOG.error("Read failed from datastore for Node : {}",connectionIid,e);
+            throw new RuntimeException(e);
         }
-        return node;
     }
 
     public static Optional<HwvtepGlobalAugmentation> getManagingNode(DataBroker db,

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 69ca66e4c1fd512aae1c6ca52ca0c043de4e7751..498617c2ef711f34910ef0d743956c21bbc7023b 100644 (file)
--- 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
@@ -10,18 +10,24 @@ package org.opendaylight.ovsdb.hwvtepsouthbound.reconciliation.configuration;
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.CONFIGURATION;
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.OPERATIONAL;
 
+import com.google.common.base.Optional;
+
 import java.util.ArrayList;
 import java.util.Collection;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
+import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepConnectionInstance;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepConnectionManager;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundMapper;
+import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundUtil;
 import org.opendaylight.ovsdb.hwvtepsouthbound.reconciliation.ReconciliationManager;
 import org.opendaylight.ovsdb.hwvtepsouthbound.reconciliation.ReconciliationTask;
 import org.opendaylight.ovsdb.hwvtepsouthbound.transact.HwvtepOperationalState;
 import org.opendaylight.ovsdb.hwvtepsouthbound.transact.TransactCommandAggregator;
-import org.opendaylight.ovsdb.utils.mdsal.utils.ControllerMdsalUtils;
 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.hwvtep.global.attributes.LogicalSwitches;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
@@ -31,7 +37,6 @@ public class HwvtepReconciliationTask extends ReconciliationTask {
     private final HwvtepConnectionInstance connectionInstance;
     private final DataBroker db;
     private final Node psNode;
-    private final ControllerMdsalUtils mdsalUtils;
 
     public HwvtepReconciliationTask(ReconciliationManager reconciliationManager,
                                     HwvtepConnectionManager connectionManager,
@@ -43,7 +48,6 @@ public class HwvtepReconciliationTask extends ReconciliationTask {
         this.db = db;
         this.psNode = psNode;
         this.connectionInstance = connectionInstance;
-        this.mdsalUtils = new ControllerMdsalUtils(db);
     }
 
     private void transactChangesToDevice(final Collection<DataTreeModification<Node>> changes,
@@ -61,9 +65,10 @@ public class HwvtepReconciliationTask extends ReconciliationTask {
         InstanceIdentifier<Node> psNodeIid = HwvtepSouthboundMapper.createInstanceIdentifier(psNode.getNodeId());
         InstanceIdentifier<Node> nodeId = (InstanceIdentifier<Node>)nodeIid;
 
-        Node globalConfigNode = mdsalUtils.read(CONFIGURATION, nodeId);
-        Node globalOpNode = mdsalUtils.read(OPERATIONAL, nodeId);
-        Node psConfigNode = mdsalUtils.read(CONFIGURATION, psNodeIid);
+        ReadOnlyTransaction tx = reconciliationManager.getDb().newReadOnlyTransaction();
+        Node globalConfigNode = readNode(tx, CONFIGURATION, nodeId);
+        Node globalOpNode = readNode(tx, OPERATIONAL, nodeId);
+        Node psConfigNode = readNode(tx, CONFIGURATION, psNodeIid);
 
         DataTreeModification<Node> change = null;
         Collection<DataTreeModification<Node>> changes = new ArrayList<>();
@@ -101,4 +106,13 @@ public class HwvtepReconciliationTask extends ReconciliationTask {
     public long retryDelayInMills() {
         return 0;
     }
+
+    private Node readNode(ReadTransaction transaction,
+                          LogicalDatastoreType logicalDatastoreType, InstanceIdentifier<Node> iid) {
+        Optional<Node> optional = HwvtepSouthboundUtil.readNode(transaction, logicalDatastoreType, iid);
+        if (optional.isPresent()) {
+            return optional.get();
+        }
+        return null;
+    }
 }

diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UnMetDependencyGetter.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UnMetDependencyGetter.java
index 88dc54585a72d142e86b8407f99a9f626a01dcf8..b046f8a4b5bc819dd37df4dd4a82494167041b31 100644 (file)
--- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UnMetDependencyGetter.java
+++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UnMetDependencyGetter.java
@@ -8,15 +8,19 @@
 
 package org.opendaylight.ovsdb.hwvtepsouthbound.transact;
 
+import com.google.common.base.Optional;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepDeviceInfo;
-import org.opendaylight.ovsdb.utils.mdsal.utils.ControllerMdsalUtils;
+import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.global.attributes.LogicalSwitches;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.node.TerminationPoint;
 import org.opendaylight.yangtools.yang.binding.DataObject;
@@ -99,14 +103,22 @@ public abstract class UnMetDependencyGetter<T extends Identifiable> {
     class ConfigDependencyGetter extends DependencyGetter {
         @Override
         boolean isDependencyMet(HwvtepOperationalState opState, HwvtepDeviceInfo deviceInfo,
-                Class<? extends Identifiable> cls, InstanceIdentifier<? extends DataObject> key) {
-            return deviceInfo.isConfigDataAvailable(cls, key) || isConfigDataAvailable(opState, key);
+                                Class<? extends Identifiable> cls, InstanceIdentifier<? extends DataObject> key) {
+            return deviceInfo.isConfigDataAvailable(cls, key) || isConfigDataAvailable(opState, cls, key);
         }
 
-        boolean isConfigDataAvailable(HwvtepOperationalState opState, InstanceIdentifier<? extends DataObject> key) {
+        boolean isConfigDataAvailable(HwvtepOperationalState opState,
+                                      Class<? extends Identifiable> cls,
+                                      InstanceIdentifier<? extends DataObject> key) {
             DataBroker db = opState.getConnectionInstance().getDataBroker();
-            ControllerMdsalUtils mdsalUtils = new ControllerMdsalUtils(db);
-            return mdsalUtils.read(LogicalDatastoreType.CONFIGURATION, key) != null;
+            ReadOnlyTransaction tx = db.newReadOnlyTransaction();
+            Optional data = HwvtepSouthboundUtil.readNode(tx, LogicalDatastoreType.CONFIGURATION, key);
+            tx.close();
+            if (data.isPresent()) {
+                opState.getDeviceInfo().updateConfigData(cls, key, data.get());
+                return true;
+            }
+            return false;
         }
     }