Optimization to read the OvsdbNode from the cache.

JIRA: OVSDB-488

Currently in the OvsdbSouthboudn plugin reads the OvsdbNode from the datastore.
Reading the OvsdbNode from the datastore ineffecient.

This review is avoid the OvsdbNode 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: I1e26fa83d0edea45b39c44267488335eea75eb37

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
index d0ddd88d404775be842ed9ec92c577eaa53187ea..00f8c2df519ce6bec85547d374c03046ba4f18b1 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
@@ -399,4 +399,5 @@ public class OvsdbConnectionInstance {
     public OvsdbClient getOvsdbClient() {
         return client;
     }
+
 }

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbOperGlobalListener.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbOperGlobalListener.java
new file mode 100644 (file)
index 0000000..0f06697
--- /dev/null
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbOperGlobalListener.java
@@ -0,0 +1,146 @@
+/*
+ * Copyright (c) 2019 Ericsson India Global Services Pvt Ltd. 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,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.ovsdb.southbound;
+
+import java.util.Collection;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener;
+import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.DataObjectModification;
+import org.opendaylight.controller.md.sal.binding.api.DataTreeIdentifier;
+import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.ovsdb.southbound.transactions.md.TransactionInvoker;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
+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.TopologyKey;
+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.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class OvsdbOperGlobalListener implements ClusteredDataTreeChangeListener<Node>, AutoCloseable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(OvsdbOperGlobalListener.class);
+    private ListenerRegistration<OvsdbOperGlobalListener> registration;
+    private DataBroker db;
+    public static final ConcurrentMap<InstanceIdentifier<Node>, Node> OPER_NODE_CACHE = new ConcurrentHashMap<>();
+    private final OvsdbConnectionManager ovsdbConnectionManager;
+    private final TransactionInvoker txInvoker;
+
+
+    OvsdbOperGlobalListener(DataBroker db, OvsdbConnectionManager ovsdbConnectionManager,
+                            TransactionInvoker txInvoker) {
+        LOG.info("Registering OvsdbOperGlobalListener");
+        this.db = db;
+        this.ovsdbConnectionManager = ovsdbConnectionManager;
+        this.txInvoker = txInvoker;
+        registerListener();
+    }
+
+    public void registerListener() {
+        DataTreeIdentifier<Node> treeId =
+                new DataTreeIdentifier<Node>(LogicalDatastoreType.OPERATIONAL, getWildcardPath());
+        registration = db.registerDataTreeChangeListener(treeId, this);
+    }
+
+    @Override
+    public void close() {
+        if (registration != null) {
+            registration.close();
+            LOG.info("OVSDB Oper Node listener has been closed.");
+        }
+    }
+
+    @Override
+    @SuppressWarnings("checkstyle:IllegalCatch")
+    public void onDataTreeChanged(Collection<DataTreeModification<Node>> changes) {
+        changes.forEach((change) -> {
+            try {
+                InstanceIdentifier<Node> key = change.getRootPath().getRootIdentifier();
+                DataObjectModification<Node> mod = change.getRootNode();
+                Node addNode = getCreated(mod);
+                if (addNode != null) {
+                    OPER_NODE_CACHE.put(key, addNode);
+                    LOG.info("Node added to oper {}", SouthboundUtil.getOvsdbNodeId(key));
+                }
+                Node removedNode = getRemoved(mod);
+                if (removedNode != null) {
+                    OPER_NODE_CACHE.remove(key);
+                    LOG.info("Node deleted from oper {}", SouthboundUtil.getOvsdbNodeId(key));
+
+                    OvsdbConnectionInstance connectionInstance = ovsdbConnectionManager.getConnectionInstance(key);
+                    if (connectionInstance != null && connectionInstance.isActive()
+                            && connectionInstance.getHasDeviceOwnership() != null
+                            && connectionInstance.getHasDeviceOwnership()) {
+                        //Oops some one deleted the node held by me This should never happen.
+                        //put the node back in oper
+                        txInvoker.invoke(transaction -> {
+                            transaction.put(LogicalDatastoreType.OPERATIONAL, key, removedNode);
+                        });
+
+                    }
+                }
+
+                Node modifiedNode = getUpdated(mod);
+                if (modifiedNode != null) {
+                    OPER_NODE_CACHE.put(key, modifiedNode);
+                }
+            } catch (Exception e) {
+                LOG.error("Failed to handle oper node ", e);
+            }
+        });
+    }
+
+
+
+    private Node getCreated(DataObjectModification<Node> mod) {
+        if ((mod.getModificationType() == DataObjectModification.ModificationType.WRITE)
+                && (mod.getDataBefore() == null)) {
+            return mod.getDataAfter();
+        }
+        return null;
+    }
+
+    private Node getRemoved(DataObjectModification<Node> mod) {
+        if (mod.getModificationType() == DataObjectModification.ModificationType.DELETE) {
+            return mod.getDataBefore();
+        }
+        return null;
+    }
+
+    private Node getUpdated(DataObjectModification<Node> mod) {
+        Node node = null;
+        switch (mod.getModificationType()) {
+            case SUBTREE_MODIFIED:
+                node = mod.getDataAfter();
+                break;
+            case WRITE:
+                if (mod.getDataBefore() !=  null) {
+                    node = mod.getDataAfter();
+                }
+                break;
+            default:
+                break;
+        }
+        return node;
+    }
+
+    private InstanceIdentifier<Node> getWildcardPath() {
+        InstanceIdentifier<Node> path = InstanceIdentifier
+                .create(NetworkTopology.class)
+                .child(Topology.class, new TopologyKey(SouthboundConstants.OVSDB_TOPOLOGY_ID))
+                .child(Node.class);
+        return path;
+    }
+
+}

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java
index f197731011a60e192e692b47dd4a8ba8835aee35..38707cf9c88fa6fea231500e6a9144c4574fa604 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java
@@ -71,6 +71,7 @@ public class SouthboundProvider implements ClusteredDataTreeChangeListener<Topol
     private OvsdbConnectionManager cm;
     private TransactionInvoker txInvoker;
     private OvsdbDataTreeChangeListener ovsdbDataTreeChangeListener;
+    private OvsdbOperGlobalListener ovsdbOperGlobalListener;
     private final EntityOwnershipService entityOwnershipService;
     private EntityOwnershipCandidateRegistration registration;
     private SouthboundPluginInstanceEntityOwnershipListener providerOwnershipChangeListener;
@@ -117,6 +118,7 @@ public class SouthboundProvider implements ClusteredDataTreeChangeListener<Topol
         cm = new OvsdbConnectionManager(db, txInvoker, entityOwnershipService, ovsdbConnection,
                 instanceIdentifierCodec, upgradeState);
         ovsdbDataTreeChangeListener = new OvsdbDataTreeChangeListener(db, cm, instanceIdentifierCodec);
+        ovsdbOperGlobalListener = new OvsdbOperGlobalListener(db, cm, txInvoker);
 
         //Register listener for entityOnwership changes
         providerOwnershipChangeListener =
@@ -153,6 +155,7 @@ public class SouthboundProvider implements ClusteredDataTreeChangeListener<Topol
         }
         cm.close();
         ovsdbDataTreeChangeListener.close();
+        ovsdbOperGlobalListener.close();
         registration.close();
         providerOwnershipChangeListener.close();
         if (operTopologyRegistration != null) {

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundUtil.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundUtil.java
index 2a73c006aea2a190e8044a4ef785ad2a52db0f2c..34fd897e005bd1c3500900ce252debe3306e7b8a 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundUtil.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundUtil.java
@@ -19,7 +19,7 @@ import java.util.concurrent.ExecutionException;
 
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
-import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
+import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.ovsdb.lib.error.SchemaVersionMismatchException;
@@ -32,6 +32,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.re
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.port._interface.attributes.PortExternalIds;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.port._interface.attributes.PortExternalIdsBuilder;
 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.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -89,15 +90,21 @@ public final class SouthboundUtil {
         }
     }
 
-    public static <D extends org.opendaylight.yangtools.yang.binding.DataObject> Optional<D> readNode(
-            ReadWriteTransaction transaction, final InstanceIdentifier<D> connectionIid) {
-        Optional<D> node = Optional.absent();
+    public static <D extends DataObject> Optional<D> readNode(ReadTransaction transaction,
+                                                              InstanceIdentifier<D> connectionIid) {
+        Optional<D> node;
         try {
-            node = transaction.read(LogicalDatastoreType.OPERATIONAL, connectionIid).checkedGet();
-        } catch (final ReadFailedException e) {
+            if (OvsdbOperGlobalListener.OPER_NODE_CACHE.containsKey(connectionIid)) {
+                node = Optional.of((D)OvsdbOperGlobalListener.OPER_NODE_CACHE.get(connectionIid));
+            } else {
+                node = transaction.read(LogicalDatastoreType.OPERATIONAL, connectionIid).checkedGet();
+            }
+        } catch (ReadFailedException e) {
             LOG.warn("Read Operational/DS for Node failed! {}", connectionIid, e);
+            throw new RuntimeException(e);
         }
         return node;
+
     }
 
     @VisibleForTesting
@@ -166,4 +173,18 @@ public final class SouthboundUtil {
             .setExternalIdKey(key)
             .setExternalIdValue(value).build();
     }
+
+    @SuppressWarnings("checkstyle:IllegalCatch")
+    public static String getOvsdbNodeId(InstanceIdentifier<Node> nodeIid) {
+        String nodeId = "";
+        if (nodeIid != null) {
+            try {
+                nodeId = nodeIid.toString();
+                nodeId = nodeIid.firstKeyOf(Node.class).getNodeId().getValue();
+            } catch (Exception exp) {
+                LOG.debug("Exception in getting the value from {} ", nodeIid);
+            }
+        }
+        return nodeId;
+    }
 }

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/AutoAttachRemovedCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/AutoAttachRemovedCommand.java
index f266ef714af7a8fe86a7af10bc2e5f20ee585c32..b944c1a283d40990b06eb74ede73040e9cc01e85 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/AutoAttachRemovedCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/AutoAttachRemovedCommand.java
@@ -26,6 +26,7 @@ import org.opendaylight.ovsdb.schema.openvswitch.AutoAttach;
 import org.opendaylight.ovsdb.schema.openvswitch.Bridge;
 import org.opendaylight.ovsdb.southbound.InstanceIdentifierCodec;
 import org.opendaylight.ovsdb.southbound.SouthboundProvider;
+import org.opendaylight.ovsdb.southbound.SouthboundUtil;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAugmentation;
@@ -139,7 +140,7 @@ public class AutoAttachRemovedCommand implements TransactCommand {
         OvsdbBridgeAugmentation bridge = null;
         final InstanceIdentifier<Node> nodeIid = key.firstIdentifierOf(Node.class);
         try (ReadOnlyTransaction transaction = SouthboundProvider.getDb().newReadOnlyTransaction()) {
-            final Optional<Node> nodeOptional = transaction.read(LogicalDatastoreType.OPERATIONAL, nodeIid).get();
+            final Optional<Node> nodeOptional = SouthboundUtil.readNode(transaction, nodeIid);
             if (nodeOptional.isPresent()) {
                 final List<ManagedNodeEntry> managedNodes =
                         nodeOptional.get().augmentation(OvsdbNodeAugmentation.class).getManagedNodeEntry();

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalState.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalState.java
index 271740cd2750dcb6f00c7e33d7eb61b48bc2d260..b11febf3ae15bdf51a79862f67c6f87fb17bbead 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalState.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalState.java
@@ -9,17 +9,11 @@
 package org.opendaylight.ovsdb.southbound.ovsdb.transact;
 
 import com.google.common.base.Optional;
-import com.google.common.util.concurrent.CheckedFuture;
 import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.concurrent.ExecutionException;
 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.common.api.data.LogicalDatastoreType;
-import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
+import org.opendaylight.ovsdb.southbound.SouthboundUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbTerminationPointAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.bridge.attributes.ControllerEntry;
@@ -35,48 +29,26 @@ import org.slf4j.LoggerFactory;
 
 public class BridgeOperationalState {
     private static final Logger LOG = LoggerFactory.getLogger(BridgeOperationalState.class);
-    private final Map<InstanceIdentifier<Node>, Node> operationalNodes = new HashMap<>();
+    private DataBroker db;
 
     public BridgeOperationalState(DataBroker db, DataChangeEvent changes) {
-        try (ReadOnlyTransaction transaction = db.newReadOnlyTransaction()) {
-            Map<InstanceIdentifier<Node>, Node> nodeCreateOrUpdate =
-                    TransactUtils.extractCreatedOrUpdatedOrRemoved(changes, Node.class);
-            for (Entry<InstanceIdentifier<Node>, Node> entry: nodeCreateOrUpdate.entrySet()) {
-                CheckedFuture<Optional<Node>, ReadFailedException> nodeFuture =
-                        transaction.read(LogicalDatastoreType.OPERATIONAL, entry.getKey());
-                try {
-                    Optional<Node> nodeOptional = nodeFuture.get();
-                    if (nodeOptional.isPresent()) {
-                        operationalNodes.put(entry.getKey(), nodeOptional.get());
-                    }
-                } catch (InterruptedException | ExecutionException e) {
-                    LOG.warn("Error reading from datastore",e);
-                }
-            }
-        }
+        this.db = db;
     }
 
     public BridgeOperationalState(DataBroker db, Collection<DataTreeModification<Node>> changes) {
-        try (ReadOnlyTransaction transaction = db.newReadOnlyTransaction()) {
-            Map<InstanceIdentifier<Node>, Node> nodeCreateOrUpdateOrRemove =
-                    TransactUtils.extractCreatedOrUpdatedOrRemoved(changes, Node.class);
-            for (Entry<InstanceIdentifier<Node>, Node> entry : nodeCreateOrUpdateOrRemove.entrySet()) {
-                try {
-                    Optional<Node> nodeOptional =
-                            transaction.read(LogicalDatastoreType.OPERATIONAL, entry.getKey()).checkedGet();
-                    if (nodeOptional.isPresent()) {
-                        operationalNodes.put(entry.getKey(), nodeOptional.get());
-                    }
-                } catch (ReadFailedException e) {
-                    LOG.warn("Error reading from datastore", e);
-                }
-            }
-        }
+        this.db = db;
     }
 
+    @SuppressWarnings("IllegalCatch")
     public Optional<Node> getBridgeNode(InstanceIdentifier<?> iid) {
         InstanceIdentifier<Node> nodeIid = iid.firstIdentifierOf(Node.class);
-        return Optional.fromNullable(operationalNodes.get(nodeIid));
+        Optional<Node> bridgeNode = Optional.absent();
+        try (ReadOnlyTransaction transaction = db.newReadOnlyTransaction()) {
+            bridgeNode = SouthboundUtil.readNode(transaction, nodeIid);
+        } catch (Exception exp) {
+            LOG.error("Error in getting the brideNode for {}", iid, exp);
+        }
+        return bridgeNode;
     }
 
     public Optional<OvsdbBridgeAugmentation> getOvsdbBridgeAugmentation(InstanceIdentifier<?> iid) {

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointCreateCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointCreateCommand.java
index 0f10c8dbbdbb175ff223404290239d9a95691c05..0310f969f5fdc8f52564f22a032b2a790dd267a5 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointCreateCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointCreateCommand.java
@@ -11,7 +11,6 @@ import static org.opendaylight.ovsdb.lib.operations.Operations.op;
 import static org.opendaylight.ovsdb.southbound.SouthboundUtil.schemaMismatchLog;
 
 import com.google.common.base.Optional;
-import com.google.common.util.concurrent.CheckedFuture;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -22,11 +21,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.concurrent.ExecutionException;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
-import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
-import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.ovsdb.lib.error.SchemaVersionMismatchException;
 import org.opendaylight.ovsdb.lib.notation.Mutator;
 import org.opendaylight.ovsdb.lib.notation.UUID;
@@ -367,15 +363,9 @@ public class TerminationPointCreateCommand implements TransactCommand {
             bridge = node.augmentation(OvsdbBridgeAugmentation.class);
             if (bridge == null) {
                 ReadOnlyTransaction transaction = SouthboundProvider.getDb().newReadOnlyTransaction();
-                CheckedFuture<Optional<Node>, ReadFailedException> future =
-                        transaction.read(LogicalDatastoreType.OPERATIONAL, nodeIid);
-                try {
-                    Optional<Node> nodeOptional = future.get();
-                    if (nodeOptional.isPresent()) {
-                        bridge = nodeOptional.get().augmentation(OvsdbBridgeAugmentation.class);
-                    }
-                } catch (InterruptedException | ExecutionException e) {
-                    LOG.warn("Error reading from datastore",e);
+                Optional<Node> nodeOptional = SouthboundUtil.readNode(transaction, nodeIid);
+                if (nodeOptional.isPresent()) {
+                    bridge = nodeOptional.get().augmentation(OvsdbBridgeAugmentation.class);
                 }
                 transaction.close();
             }

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointUpdateCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointUpdateCommand.java
index c880a76f4dc44eeb7aec8e65f9e1fd4e5dc2b3d7..fc8ce442240d89412482ce360a56878b4d41767d 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointUpdateCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointUpdateCommand.java
@@ -12,7 +12,6 @@ import static org.opendaylight.ovsdb.lib.operations.Operations.op;
 import static org.opendaylight.ovsdb.southbound.SouthboundUtil.schemaMismatchLog;
 
 import com.google.common.base.Optional;
-import com.google.common.util.concurrent.CheckedFuture;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -22,11 +21,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.concurrent.ExecutionException;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
-import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
-import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.ovsdb.lib.error.SchemaVersionMismatchException;
 import org.opendaylight.ovsdb.lib.notation.UUID;
 import org.opendaylight.ovsdb.lib.operations.TransactionBuilder;
@@ -183,20 +179,18 @@ public class TerminationPointUpdateCommand implements TransactCommand {
         port.setQos(uuidSet);
     }
 
+    @SuppressWarnings("IllegalCatch")
     private static OvsdbNodeAugmentation getOperNode(final OvsdbBridgeAugmentation operBridge) {
         @SuppressWarnings("unchecked")
         InstanceIdentifier<Node> iidNode = (InstanceIdentifier<Node>)operBridge.getManagedBy().getValue();
         OvsdbNodeAugmentation operNode = null;
-        ReadOnlyTransaction transaction = SouthboundProvider.getDb().newReadOnlyTransaction();
-        CheckedFuture<Optional<Node>, ReadFailedException> future =
-                transaction.read(LogicalDatastoreType.OPERATIONAL, iidNode);
-        try {
-            Optional<Node> nodeOptional = future.get();
+        try (ReadOnlyTransaction transaction = SouthboundProvider.getDb().newReadOnlyTransaction()) {
+            Optional<Node> nodeOptional = SouthboundUtil.readNode(transaction, iidNode);
             if (nodeOptional.isPresent()) {
                 operNode = nodeOptional.get().augmentation(OvsdbNodeAugmentation.class);
             }
-        } catch (InterruptedException | ExecutionException e) {
-            LOG.warn("Error reading from datastore", e);
+        } catch (Exception exp) {
+            LOG.error("Error in getting the brideNode for {}", iidNode, exp);
         }
         return operNode;
     }

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java
index 236e7fddbe12e68d568201a6d5c7a8073bdfee8a..6ae80b2ebe4eb789b2101da86b893f034b3a3f11 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java
@@ -19,9 +19,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+
 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;
 import org.opendaylight.ovsdb.lib.error.ColumnSchemaNotFoundException;
 import org.opendaylight.ovsdb.lib.error.SchemaVersionMismatchException;
 import org.opendaylight.ovsdb.lib.message.TableUpdates;
@@ -38,6 +38,7 @@ import org.opendaylight.ovsdb.southbound.InstanceIdentifierCodec;
 import org.opendaylight.ovsdb.southbound.OvsdbConnectionInstance;
 import org.opendaylight.ovsdb.southbound.SouthboundConstants;
 import org.opendaylight.ovsdb.southbound.SouthboundMapper;
+import org.opendaylight.ovsdb.southbound.SouthboundUtil;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;
@@ -209,15 +210,13 @@ public class OvsdbPortUpdateCommand extends AbstractTransactionCommand {
         updateInterfaces(interfaceUpdate, tpAugmentationBuilder);
     }
 
+    @SuppressWarnings("IllegalCatch")
     private Optional<Node> readNode(final ReadWriteTransaction transaction, final InstanceIdentifier<Node> nodePath) {
         Optional<Node> node = Optional.absent();
         try {
-            node = transaction.read(
-                    LogicalDatastoreType.OPERATIONAL, nodePath)
-                    .checkedGet();
-        } catch (final ReadFailedException e) {
-            LOG.warn("Read Operational/DS for Node fail! {}",
-                    nodePath, e);
+            node = SouthboundUtil.readNode(transaction, nodePath);
+        } catch (Exception exp) {
+            LOG.error("Error in getting the Node for {}", nodePath, exp);
         }
         return node;
     }
@@ -242,7 +241,7 @@ public class OvsdbPortUpdateCommand extends AbstractTransactionCommand {
         TpId tpId = new TpId(tpName);
 
         for (ManagedNodeEntry managedNodeEntry : managedNodes) {
-            Optional<Node> optManagedNode = readNode(transaction,
+            Optional<Node> optManagedNode = SouthboundUtil.readNode(transaction,
                     (InstanceIdentifier<Node>)managedNodeEntry.getBridgeRef().getValue());
             if (optManagedNode.isPresent()) {
                 Node managedNode = optManagedNode.get();

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalStateTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalStateTest.java
index 979558a5a262867f3e6df7250fd599b786c40fe7..e05a470a654463fe7bd6d680b0c0beee9e319555 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalStateTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/BridgeOperationalStateTest.java
@@ -21,7 +21,6 @@ import static org.powermock.reflect.Whitebox.getField;
 
 import com.google.common.base.Optional;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import org.junit.Before;
@@ -30,6 +29,9 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
+import org.opendaylight.ovsdb.southbound.OvsdbOperGlobalListener;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbTerminationPointAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.bridge.attributes.ControllerEntry;
@@ -51,7 +53,13 @@ public class BridgeOperationalStateTest {
     private final Node nd = new NodeBuilder().setNodeId(new NodeId("foo")).build();
     private final InstanceIdentifier<?> iid = InstanceIdentifier.create(Topology.class);
 
+    private final InstanceIdentifier<Node> nodeIid = InstanceIdentifier.create(NetworkTopology.class)
+            .child(Topology.class).child(Node.class);
+    private final Node brNode = new NodeBuilder().build();
+
     @Mock private BridgeOperationalState briOperationState;
+    @Mock private DataBroker db;
+    @Mock ReadOnlyTransaction mockReadTx;
     private InstanceIdentifier<ProtocolEntry> protocolEntry;
     private InstanceIdentifier<Node> iidNode;
     private Map<InstanceIdentifier<Node>, Node> operationalNodes;
@@ -66,20 +74,20 @@ public class BridgeOperationalStateTest {
 
         briOperationState = mock(BridgeOperationalState.class, Mockito.CALLS_REAL_METHODS);
 
-        operationalNodes = new HashMap<>();
-        operationalNodes.put(iidNode, nd);
-        getField(BridgeOperationalState.class,"operationalNodes").set(briOperationState, operationalNodes);
+        getField(BridgeOperationalState.class,"db").set(briOperationState, db);
+        doReturn(mockReadTx).when(db).newReadOnlyTransaction();
+        OvsdbOperGlobalListener.OPER_NODE_CACHE.put(nodeIid, brNode);
     }
 
     @Test
     public void testGetBridgeNode() {
-        Optional<Node> optNodes = briOperationState.getBridgeNode(iid);
-        assertEquals(Optional.absent(), optNodes);
+        Optional<Node> optNodes = briOperationState.getBridgeNode(nodeIid);
+        assertEquals(brNode, optNodes.get());
     }
 
     @Test
     public void testGetOvsdbBridgeAugmentation() throws Exception {
-        Optional<OvsdbBridgeAugmentation> optOvsdbBri = briOperationState.getOvsdbBridgeAugmentation(iid);
+        Optional<OvsdbBridgeAugmentation> optOvsdbBri = briOperationState.getOvsdbBridgeAugmentation(nodeIid);
         verify(briOperationState, times(1)).getBridgeNode(any(InstanceIdentifier.class));
         assertNotNull(optOvsdbBri);
         assertTrue(optOvsdbBri.equals(Optional.absent()));
@@ -96,7 +104,7 @@ public class BridgeOperationalStateTest {
 
     @Test
     public void testGetBridgeTerminationPoint() throws Exception {
-        Optional<TerminationPoint> optTerm = briOperationState.getBridgeTerminationPoint(iid);
+        Optional<TerminationPoint> optTerm = briOperationState.getBridgeTerminationPoint(nodeIid);
         verify(briOperationState, times(1)).getBridgeNode(any(InstanceIdentifier.class));
         assertNotNull(optTerm);
         assertTrue(optTerm.equals(Optional.absent()));
@@ -120,7 +128,7 @@ public class BridgeOperationalStateTest {
     @Test
     public void testGetOvsdbTerminationPointAugmentation() {
         Optional<OvsdbTerminationPointAugmentation> optOvsdbTermPoint = briOperationState
-                .getOvsdbTerminationPointAugmentation(iid);
+                .getOvsdbTerminationPointAugmentation(nodeIid);
         assertNotNull(optOvsdbTermPoint);
         verify(briOperationState, times(1)).getBridgeTerminationPoint(any(InstanceIdentifier.class));
         verify(briOperationState, times(1)).getBridgeNode(any(InstanceIdentifier.class));
@@ -139,7 +147,7 @@ public class BridgeOperationalStateTest {
 
     @Test
     public void testGetControllerEntry() {
-        Optional<ControllerEntry> optController = briOperationState.getControllerEntry(iid);
+        Optional<ControllerEntry> optController = briOperationState.getControllerEntry(nodeIid);
         verify(briOperationState, times(1)).getOvsdbBridgeAugmentation(any(InstanceIdentifier.class));
         verify(briOperationState, times(1)).getBridgeNode(any(InstanceIdentifier.class));
         assertNotNull(optController);

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java
index 3ce8d6671773043e599402c6be7694eecf7350bb..3584b65839cd1a469cefc1d421009018143ddf61 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java
@@ -35,7 +35,9 @@ import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Matchers;
 import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
@@ -75,8 +77,10 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.re
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.port._interface.attributes.PortOtherConfigs;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.port._interface.attributes.PortOtherConfigsBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.port._interface.attributes.TrunksBuilder;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TpId;
+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.TopologyKey;
 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;
@@ -177,7 +181,11 @@ public class OvsdbPortUpdateCommandTest {
         when(port.getNameColumn()).thenReturn(bridgeColumn);
         when(bridgeColumn.getData()).thenReturn(TERMINATION_POINT_NAME);
 
-        Optional<InstanceIdentifier<Node>> bridgeIid = Optional.of(mock(InstanceIdentifier.class));
+        InstanceIdentifier<Node> nodeIid = InstanceIdentifier.create(NetworkTopology.class)
+                .child(Topology.class, new TopologyKey(SouthboundConstants.OVSDB_TOPOLOGY_ID))
+                .child(Node.class, new NodeKey(new NodeId("nodeId")));
+
+        Optional<InstanceIdentifier<Node>> bridgeIid = Optional.of(nodeIid);
         PowerMockito.doReturn(bridgeIid).when(ovsdbPortUpdateCommand, "getTerminationPointBridge", any(UUID.class));
 
         NodeId bridgeId = mock(NodeId.class);
@@ -342,6 +350,10 @@ public class OvsdbPortUpdateCommandTest {
         PowerMockito.doReturn(optionalNode).when(ovsdbPortUpdateCommand, "readNode", any(ReadWriteTransaction.class),
                 any(InstanceIdentifier.class));
 
+        PowerMockito.mockStatic(SouthboundUtil.class);
+        PowerMockito.when(SouthboundUtil.readNode(Matchers.any(ReadWriteTransaction.class),
+                Matchers.any(InstanceIdentifier.class)))
+                .thenReturn(optionalNode);
         TerminationPointBuilder tpBuilder = mock(TerminationPointBuilder.class);
         PowerMockito.whenNew(TerminationPointBuilder.class).withNoArguments().thenReturn(tpBuilder);
         PowerMockito.whenNew(TpId.class).withAnyArguments().thenReturn(mock(TpId.class));
@@ -395,8 +407,12 @@ public class OvsdbPortUpdateCommandTest {
         PowerMockito.whenNew(Uuid.class).withAnyArguments().thenReturn(mock(Uuid.class));
         when(ovsdbTerminationPointBuilder.setInterfaceUuid(any(Uuid.class))).thenReturn(ovsdbTerminationPointBuilder);
         PowerMockito.mockStatic(SouthboundMapper.class);
-        PowerMockito.when(SouthboundMapper.createInterfaceType(anyString()))
-                .thenAnswer((Answer<Class<? extends InterfaceTypeBase>>) invocation -> InterfaceTypeInternal.class);
+        PowerMockito.when(SouthboundMapper.createInterfaceType(Matchers.anyString()))
+                .thenAnswer(new Answer<Class<? extends InterfaceTypeBase>>() {
+                    public Class<? extends InterfaceTypeBase> answer(InvocationOnMock invocation) throws Exception {
+                        return InterfaceTypeInternal.class;
+                    }
+                });
         when(ovsdbTerminationPointBuilder.setInterfaceType(any(Class.class))).thenReturn(ovsdbTerminationPointBuilder);
         suppress(method(OvsdbPortUpdateCommand.class, "updateOfPort", Interface.class,
                 OvsdbTerminationPointAugmentationBuilder.class));