Getallnodes should read from cache instead of DS 42/82442/13
authorGobinath <gobinath@ericsson.com>
Thu, 4 Apr 2019 06:43:38 +0000 (12:13 +0530)
committerShweta Chaturvedi <shweta.chaturvedi@ericsson.com>
Thu, 14 Nov 2019 06:22:24 +0000 (11:52 +0530)
Problem
=======
 Get All DPNs query takes long time to respond, as it reads from database.

Solution
========
Fix is to provide cache based response instead of Data store read.

Change-Id: Ida535c986713b25440b86c057d94c7b6bf060ede
Signed-off-by: Gobinath <gobinath@ericsson.com>
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/NodeListener.java [new file with mode: 0644]
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/ReconciliationServiceImpl.java
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/cli/GetAllNodesCommandProvider.java
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/util/ShellUtil.java
applications/southbound-cli/src/main/resources/OSGI-INF/blueprint/southbound-cli.xml

diff --git a/applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/NodeListener.java b/applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/NodeListener.java
new file mode 100644 (file)
index 0000000..fd22171
--- /dev/null
@@ -0,0 +1,147 @@
+/*
+ * 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.openflowplugin.applications.southboundcli;
+
+import static java.util.Objects.requireNonNull;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.mdsal.binding.api.ClusteredDataTreeChangeListener;
+import org.opendaylight.mdsal.binding.api.DataBroker;
+import org.opendaylight.mdsal.binding.api.DataObjectModification;
+import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
+import org.opendaylight.mdsal.binding.api.DataTreeModification;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
+import org.opendaylight.yangtools.concepts.ListenerRegistration;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeListener implements ClusteredDataTreeChangeListener<FlowCapableNode>, AutoCloseable {
+    private static final Logger LOG = LoggerFactory.getLogger(NodeListener.class);
+    public static final String DEFAULT_DPN_NAME = "UNKNOWN";
+    public static final String SEPARATOR = ":";
+
+    private final DataBroker dataBroker;
+    private ListenerRegistration<?> listenerReg;
+    private Map<Long, String> dpnIdToNameCache;
+
+    public NodeListener(DataBroker broker) {
+        this.dataBroker = broker;
+    }
+
+    public void start() {
+        final InstanceIdentifier<FlowCapableNode> path = InstanceIdentifier.create(Nodes.class).child(Node.class)
+                .augmentation(FlowCapableNode.class);
+        final DataTreeIdentifier<FlowCapableNode> identifier =
+                DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, path);
+        listenerReg = dataBroker.registerDataTreeChangeListener(identifier, NodeListener.this);
+        dpnIdToNameCache = new ConcurrentHashMap<>();
+    }
+
+    @Override
+    public void close() {
+        if (listenerReg != null) {
+            listenerReg.close();
+        }
+    }
+
+    @Override
+    public void onDataTreeChanged(@NonNull Collection<DataTreeModification<FlowCapableNode>> changes) {
+        requireNonNull(changes, "Changes may not be null!");
+        for (DataTreeModification<FlowCapableNode> change : changes) {
+            final InstanceIdentifier<FlowCapableNode> key = change.getRootPath().getRootIdentifier();
+            final DataObjectModification<FlowCapableNode> mod = change.getRootNode();
+            final InstanceIdentifier<FlowCapableNode> nodeIdent = key.firstIdentifierOf(FlowCapableNode.class);
+            switch (mod.getModificationType()) {
+                case DELETE:
+                    remove(nodeIdent, mod.getDataBefore());
+                    break;
+                case SUBTREE_MODIFIED:
+                    update(nodeIdent, mod.getDataBefore(), mod.getDataAfter());
+                    break;
+                case WRITE:
+                    if (mod.getDataBefore() == null) {
+                        add(nodeIdent, mod.getDataAfter());
+                    } else {
+                        update(nodeIdent, mod.getDataBefore(), mod.getDataAfter());
+                    }
+                    break;
+                default:
+                    throw new IllegalArgumentException("Unhandled modification type " + mod.getModificationType());
+            }
+        }
+    }
+
+    private void remove(InstanceIdentifier<FlowCapableNode> instId, FlowCapableNode delNode) {
+        LOG.trace("Received remove notification for {}", delNode);
+        String[] node = instId.firstKeyOf(Node.class).getId().getValue().split(SEPARATOR);
+        if (node.length < 2) {
+            LOG.error("Failed to remove Unexpected nodeId {}", instId.firstKeyOf(Node.class).getId()
+                    .getValue());
+            return;
+        }
+        long dpnId = Long.parseLong(node[1]);
+        dpnIdToNameCache.remove(dpnId);
+    }
+
+    private void update(InstanceIdentifier<FlowCapableNode> instId, FlowCapableNode dataObjectModificationBefore,
+                          FlowCapableNode dataObjectModificationAfter) {
+
+        LOG.trace("Received update notification {}", instId);
+        String[] node = instId.firstKeyOf(Node.class).getId().getValue().split(SEPARATOR);
+        if (node.length < 2) {
+            LOG.error("Failed to add Unexpected nodeId {}", instId.firstKeyOf(Node.class).getId()
+                    .getValue());
+            return;
+        }
+        long dpnId = Long.parseLong(node[1]);
+        try {
+            String nodeName = dataObjectModificationAfter.getDescription();
+            if (nodeName != null) {
+                dpnIdToNameCache.put(dpnId, nodeName);
+            } else {
+                dpnIdToNameCache.put(dpnId , DEFAULT_DPN_NAME);
+            }
+        } catch (NullPointerException e) {
+            LOG.error("Error while converting Node:{} to FlowCapableNode: ", dpnId, e);
+        }
+    }
+
+    private void add(InstanceIdentifier<FlowCapableNode> instId, FlowCapableNode addNode) {
+        LOG.trace("Received ADD notification for {}", instId);
+        String[] node = instId.firstKeyOf(Node.class).getId().getValue().split(SEPARATOR);
+        if (node.length < 2) {
+            LOG.error("Failed to add Unexpected nodeId {}", instId.firstKeyOf(Node.class).getId()
+                    .getValue());
+            return;
+        }
+        long dpnId = Long.parseLong(node[1]);
+        String dpnName = null;
+        try {
+            dpnName = addNode.getDescription();
+        } catch (NullPointerException e) {
+            LOG.error("Error while converting Node:{} to FlowCapableNode: ", dpnId, e);
+        }
+        LOG.trace("Adding DPNID {} to cache", dpnId);
+        if (dpnName == null) {
+            dpnName = DEFAULT_DPN_NAME;
+        }
+        dpnIdToNameCache.put(dpnId, dpnName);
+    }
+
+    public Map<Long, String> getDpnIdToNameCache() {
+        return dpnIdToNameCache;
+    }
+}
\ No newline at end of file
index 31daf4235fb0a83ccaa072e922dfca3bc04de00b..d1a0ede63e260891140ebdf20e3250516433d732 100644 (file)
@@ -8,6 +8,7 @@
 
 package org.opendaylight.openflowplugin.applications.southboundcli;
 
+import static java.util.Objects.requireNonNull;
 import static org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.NodeReconcileState.State.COMPLETED;
 import static org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.NodeReconcileState.State.FAILED;
 import static org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.NodeReconcileState.State.INPROGRESS;
@@ -70,15 +71,17 @@ public class ReconciliationServiceImpl implements ReconciliationService, AutoClo
     private final DataBroker broker;
     private final FrmReconciliationService frmReconciliationService;
     private final AlarmAgent alarmAgent;
+    private final NodeListener nodeListener;
     private final Long startCount = 1L;
     private final int threadPoolSize = 10;
     private final ExecutorService executor = Executors.newWorkStealingPool(threadPoolSize);
 
     public ReconciliationServiceImpl(final DataBroker broker, final FrmReconciliationService frmReconciliationService,
-                                     final AlarmAgent alarmAgent) {
+                                     final AlarmAgent alarmAgent, final NodeListener nodeListener) {
         this.broker = broker;
         this.frmReconciliationService = frmReconciliationService;
         this.alarmAgent = alarmAgent;
+        this.nodeListener = requireNonNull(nodeListener, "NodeListener cannot be null!");
     }
 
     @Override
@@ -160,8 +163,8 @@ public class ReconciliationServiceImpl implements ReconciliationService, AutoClo
     }
 
     private List<Long> getAllNodes() {
-        List<OFNode> nodeList = ShellUtil.getAllNodes(broker);
-        List<Long> nodes = nodeList.stream().distinct().map(OFNode::getNodeId).collect(Collectors.toList());
+        List<OFNode> nodeList = ShellUtil.getAllNodes(nodeListener);
+        List<Long> nodes = nodeList.stream().distinct().map(node -> node.getNodeId()).collect(Collectors.toList());
         return nodes;
     }
 
@@ -240,7 +243,7 @@ public class ReconciliationServiceImpl implements ReconciliationService, AutoClo
         }
 
         private Optional<ReconcileCounter> getReconciliationCount(ReadWriteTransaction tx,
-                                InstanceIdentifier<ReconcileCounter> instanceIdentifier) {
+                                                             InstanceIdentifier<ReconcileCounter> instanceIdentifier) {
             try {
                 return tx.read(LogicalDatastoreType.OPERATIONAL, instanceIdentifier).get();
             } catch (InterruptedException | ExecutionException e) {
index da25c25535bd4df686a33496f775ec7529cbf335..da8688defc1d19fe066165fbeae9218fd73193ad 100644 (file)
@@ -7,11 +7,13 @@
  */
 
 package org.opendaylight.openflowplugin.applications.southboundcli.cli;
+
 import java.util.Formatter;
 import java.util.List;
 import org.apache.felix.gogo.commands.Command;
 import org.apache.karaf.shell.console.OsgiCommandSupport;
 import org.opendaylight.mdsal.binding.api.DataBroker;
+import org.opendaylight.openflowplugin.applications.southboundcli.NodeListener;
 import org.opendaylight.openflowplugin.applications.southboundcli.util.OFNode;
 import org.opendaylight.openflowplugin.applications.southboundcli.util.ShellUtil;
 import org.slf4j.Logger;
@@ -22,14 +24,19 @@ public class GetAllNodesCommandProvider extends OsgiCommandSupport {
     private static final Logger LOG = LoggerFactory.getLogger(GetAllNodesCommandProvider.class);
 
     private DataBroker dataBroker;
+    private NodeListener nodeListener;
 
     public void setDataBroker(final DataBroker dataBroker) {
         this.dataBroker = dataBroker;
     }
 
+    public void setNodeListener(final NodeListener nodeListener) {
+        this.nodeListener = nodeListener;
+    }
+
     @Override
-    protected Object doExecute() {
-        List<OFNode> ofNodeList = ShellUtil.getAllNodes(dataBroker);
+    protected Object doExecute() throws Exception {
+        List<OFNode> ofNodeList = ShellUtil.getAllNodes(nodeListener);
         if (ofNodeList.isEmpty()) {
             session.getConsole().println("No node is connected yet");
         } else {
index fc43de58bdc7c2279d67b39e37f8b41ea2b47b81..4eb6735ec9cefda426016097e859210820857bb9 100644 (file)
@@ -11,12 +11,13 @@ package org.opendaylight.openflowplugin.applications.southboundcli.util;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
-import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.binding.api.DataBroker;
 import org.opendaylight.mdsal.binding.api.ReadTransaction;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
+import org.opendaylight.openflowplugin.applications.southboundcli.NodeListener;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNodeConnector;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
@@ -38,38 +39,15 @@ public final class ShellUtil {
     private ShellUtil() {
     }
 
-    @NonNull
-    public static List<OFNode> getAllNodes(final DataBroker broker) {
-        List<Node> nodes = null;
-        InstanceIdentifier<Nodes> path = InstanceIdentifier.builder(Nodes.class).build();
-        try (ReadTransaction tx = broker.newReadOnlyTransaction()) {
-            Optional<Nodes> result = tx.read(LogicalDatastoreType.OPERATIONAL, path).get();
-            if (result.isPresent()) {
-                nodes = result.get().getNode();
-            }
-        } catch (ExecutionException | InterruptedException | NullPointerException e) {
-            LOG.error("Error reading nodes from Inventory DS", e);
-        }
-        if (nodes != null) {
-            List<OFNode> nodeList = new ArrayList<>();
-            for (Node node : nodes) {
-                String[] nodeId = node.getId().getValue().split(":");
-                String name = null;
-                FlowCapableNode flowCapableNode = node.<FlowCapableNode>augmentation(FlowCapableNode.class);
-                if (flowCapableNode != null) {
-                    name = node.<FlowCapableNode>augmentation(FlowCapableNode.class).getDescription();
-                } else {
-                    LOG.error("Error while converting OFNode: {} to FlowCapableNode", node.getId());
-                    return Collections.emptyList();
-                }
-                OFNode ofNode = new OFNode(Long.parseLong(nodeId[1]), name);
-                LOG.trace("Added OFNode: {} to the list", ofNode.getNodeId());
-                nodeList.add(ofNode);
-            }
-            Collections.sort(nodeList);
-            return nodeList;
+    public static List<OFNode> getAllNodes(final NodeListener nodeListener) {
+        List<OFNode> dpnList = new ArrayList<>();
+        for (Map.Entry<Long, String> entry : nodeListener.getDpnIdToNameCache().entrySet()) {
+            OFNode dpn = new OFNode(entry.getKey(), entry.getValue());
+            dpnList.add(dpn);
+            LOG.trace("Added OFNode: {} to the list", dpn.getNodeId());
         }
-        return Collections.emptyList();
+        Collections.sort(dpnList);
+        return dpnList;
     }
 
     public static OFNode getNode(final long nodeId, final DataBroker broker) {
index 658cc12355bedc2f8501136fe176677daf7343fa..635c5319e6948cd8a7e6450f7f87e48989232d48 100644 (file)
         <argument ref="dataBroker"/>
         <argument ref="frmReconciliationService"/>
         <argument ref="alarmAgent"/>
+        <argument ref="nodeListener"/>
+    </bean>
+    <bean id="nodeListener"
+          class="org.opendaylight.openflowplugin.applications.southboundcli.NodeListener"
+          init-method="start"
+          destroy-method="close">
+        <argument ref="dataBroker"/>
     </bean>
 
     <odl:rpc-implementation ref="reconciliationService"/>
@@ -28,6 +35,7 @@
         <command name="openflow/getallnodes">
             <action class="org.opendaylight.openflowplugin.applications.southboundcli.cli.GetAllNodesCommandProvider">
                 <property name="dataBroker" ref="dataBroker" />
+                <property name="nodeListener" ref="nodeListener"/>
             </action>
         </command>
         <command name="openflow/shownode">