Remove isCloseMsg check for each rpc

Performance improvement

The check re-read the message and checked if its close-session rpc.
DelayedClose method was added to netconf server session that replaces the
isCloseSession method with a simple bollean check.

Change-Id: Ic4f6d473b948bd4f63771748dac793ba43693828
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

diff --git a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java
index e1ffcdddffc679e33ccd823322ba2dce8cfdf199..47bdcd8cc854bc82c606d24f82064de532571c53 100644 (file)
--- a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java
+++ b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java
@@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;

+import io.netty.channel.Channel;

 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;


@@ -94,12 +95,15 @@ import org.opendaylight.controller.netconf.confignetconfconnector.operations.run
 import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreContext;
 import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreService;
 import org.opendaylight.controller.netconf.confignetconfconnector.transactions.TransactionProvider;
 import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreContext;
 import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreService;
 import org.opendaylight.controller.netconf.confignetconfconnector.transactions.TransactionProvider;

+import org.opendaylight.controller.netconf.impl.NetconfServerSession;
+import org.opendaylight.controller.netconf.impl.NetconfServerSessionListener;

 import org.opendaylight.controller.netconf.impl.mapping.operations.DefaultCloseSession;
 import org.opendaylight.controller.netconf.impl.osgi.AggregatedNetconfOperationServiceFactory;
 import org.opendaylight.controller.netconf.impl.osgi.NetconfOperationRouter;
 import org.opendaylight.controller.netconf.mapping.api.HandlingPriority;
 import org.opendaylight.controller.netconf.mapping.api.NetconfOperation;
 import org.opendaylight.controller.netconf.mapping.api.NetconfOperationChainedExecution;
 import org.opendaylight.controller.netconf.impl.mapping.operations.DefaultCloseSession;
 import org.opendaylight.controller.netconf.impl.osgi.AggregatedNetconfOperationServiceFactory;
 import org.opendaylight.controller.netconf.impl.osgi.NetconfOperationRouter;
 import org.opendaylight.controller.netconf.mapping.api.HandlingPriority;
 import org.opendaylight.controller.netconf.mapping.api.NetconfOperation;
 import org.opendaylight.controller.netconf.mapping.api.NetconfOperationChainedExecution;

+import org.opendaylight.controller.netconf.util.messages.NetconfHelloMessageAdditionalHeader;

 import org.opendaylight.controller.netconf.util.messages.NetconfMessageUtil;
 import org.opendaylight.controller.netconf.util.test.XmlFileLoader;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.opendaylight.controller.netconf.util.messages.NetconfMessageUtil;
 import org.opendaylight.controller.netconf.util.test.XmlFileLoader;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;


@@ -387,7 +391,14 @@ public class NetconfMappingTest extends AbstractConfigTest {
 
     private void closeSession() throws NetconfDocumentedException, ParserConfigurationException, SAXException,
             IOException {
 
     private void closeSession() throws NetconfDocumentedException, ParserConfigurationException, SAXException,
             IOException {

+        final Channel channel = mock(Channel.class);
+        doReturn("channel").when(channel).toString();
+        final NetconfServerSessionListener listener = mock(NetconfServerSessionListener.class);
+        final NetconfServerSession session =
+                new NetconfServerSession(listener, channel, 1L,
+                        NetconfHelloMessageAdditionalHeader.fromString("[netconf;10.12.0.102:48528;ssh;;;;;;]"));

         DefaultCloseSession closeOp = new DefaultCloseSession(NETCONF_SESSION_ID, sessionCloseable);
         DefaultCloseSession closeOp = new DefaultCloseSession(NETCONF_SESSION_ID, sessionCloseable);

+        closeOp.setNetconfSession(session);

         executeOp(closeOp, "netconfMessages/closeSession.xml");
     }
 
         executeOp(closeOp, "netconfMessages/closeSession.xml");
     }
 

diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java
index 0cf2dbc281e1503e0ff1dd8d362285919c527c9a..4368f7ec7940152ef038a1b4320dbc3c858732cf 100644 (file)
--- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java
+++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java
@@ -10,6 +10,8 @@ package org.opendaylight.controller.netconf.impl;
 
 import com.google.common.base.Preconditions;
 import io.netty.channel.Channel;
 
 import com.google.common.base.Preconditions;
 import io.netty.channel.Channel;

+import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelFutureListener;

 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.MessageToByteEncoder;
 import java.text.SimpleDateFormat;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.MessageToByteEncoder;
 import java.text.SimpleDateFormat;


@@ -45,6 +47,7 @@ public final class NetconfServerSession extends AbstractNetconfSession<NetconfSe
 
     private Date loginTime;
     private long inRpcSuccess, inRpcFail, outRpcError;
 
     private Date loginTime;
     private long inRpcSuccess, inRpcFail, outRpcError;

+    private volatile boolean delayedClose;

 
     public NetconfServerSession(final NetconfServerSessionListener sessionListener, final Channel channel, final long sessionId,
             final NetconfHelloMessageAdditionalHeader header) {
 
     public NetconfServerSession(final NetconfServerSessionListener sessionListener, final Channel channel, final long sessionId,
             final NetconfHelloMessageAdditionalHeader header) {


@@ -60,6 +63,29 @@ public final class NetconfServerSession extends AbstractNetconfSession<NetconfSe
         super.sessionUp();
     }
 
         super.sessionUp();
     }
 

+    /**
+     * Close this session after next message is sent.
+     * Suitable for close rpc that needs to send ok response before the session is closed.
+     */
+    public void delayedClose() {
+        this.delayedClose = true;
+    }
+
+    @Override
+    public ChannelFuture sendMessage(final NetconfMessage netconfMessage) {
+        final ChannelFuture channelFuture = super.sendMessage(netconfMessage);
+        // delayed close was set, close after the message was sent
+        if(delayedClose) {
+            channelFuture.addListener(new ChannelFutureListener() {
+                @Override
+                public void operationComplete(final ChannelFuture future) throws Exception {
+                    close();
+                }
+            });
+        }
+        return channelFuture;
+    }
+

     public void onIncommingRpcSuccess() {
         inRpcSuccess++;
     }
     public void onIncommingRpcSuccess() {
         inRpcSuccess++;
     }

diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
index a6531d3c63d09b6fc64a1c0afd08e3f21041b2df..e83e0b0227e236bf592ad5d2c4b327efd795db80 100644 (file)
--- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
+++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
@@ -17,10 +17,8 @@ import org.opendaylight.controller.netconf.api.NetconfSessionListener;
 import org.opendaylight.controller.netconf.api.NetconfTerminationReason;
 import org.opendaylight.controller.netconf.api.monitoring.NetconfMonitoringService;
 import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants;
 import org.opendaylight.controller.netconf.api.NetconfTerminationReason;
 import org.opendaylight.controller.netconf.api.monitoring.NetconfMonitoringService;
 import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants;

-import org.opendaylight.controller.netconf.impl.mapping.operations.DefaultCloseSession;

 import org.opendaylight.controller.netconf.impl.osgi.NetconfOperationRouter;
 import org.opendaylight.controller.netconf.util.messages.SendErrorExceptionUtil;
 import org.opendaylight.controller.netconf.impl.osgi.NetconfOperationRouter;
 import org.opendaylight.controller.netconf.util.messages.SendErrorExceptionUtil;

-import org.opendaylight.controller.netconf.util.xml.XmlElement;

 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;


@@ -89,11 +87,6 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
                     session);
             LOG.debug("Responding with message {}", message);
             session.sendMessage(message);
                     session);
             LOG.debug("Responding with message {}", message);
             session.sendMessage(message);

-
-            if (isCloseSession(netconfMessage)) {
-                closeNetconfSession(session);
-            }
-

         } catch (final RuntimeException e) {
             // TODO: should send generic error or close session?
             LOG.error("Unexpected exception", e);
         } catch (final RuntimeException e) {
             // TODO: should send generic error or close session?
             LOG.error("Unexpected exception", e);


@@ -107,14 +100,6 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
         }
     }
 
         }
     }
 

-    private void closeNetconfSession(final NetconfServerSession session) {
-        // destroy NetconfOperationService
-        session.close();
-        LOG.info("Session {} closed successfully", session.getSessionId());
-    }
-
-
-

     private NetconfMessage processDocument(final NetconfMessage netconfMessage, final NetconfServerSession session)
             throws NetconfDocumentedException {
 
     private NetconfMessage processDocument(final NetconfMessage netconfMessage, final NetconfServerSession session)
             throws NetconfDocumentedException {
 


@@ -166,14 +151,4 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
                 ImmutableMap.of(NetconfDocumentedException.ErrorTag.missing_attribute.toString(),
                         XmlNetconfConstants.MESSAGE_ID));
     }
                 ImmutableMap.of(NetconfDocumentedException.ErrorTag.missing_attribute.toString(),
                         XmlNetconfConstants.MESSAGE_ID));
     }

-
-    private static boolean isCloseSession(final NetconfMessage incomingDocument) {
-        final Document document = incomingDocument.getDocument();
-        XmlElement rpcElement = XmlElement.fromDomDocument(document);
-        if (rpcElement.getOnlyChildElementOptionally(DefaultCloseSession.CLOSE_SESSION).isPresent()) {
-            return true;
-        }
-
-        return false;
-    }

 }
 }

diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java
index 8c7465b02961197e4a1bc5c836d10d00c0441be5..303352c9c557f111cabde8866e4376698cb96190 100644 (file)
--- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java
+++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java
@@ -9,18 +9,26 @@
 package org.opendaylight.controller.netconf.impl.mapping.operations;
 
 import com.google.common.base.Optional;
 package org.opendaylight.controller.netconf.impl.mapping.operations;
 
 import com.google.common.base.Optional;

+import com.google.common.base.Preconditions;

 import java.util.Collections;
 import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
 import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants;
 import java.util.Collections;
 import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
 import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants;

+import org.opendaylight.controller.netconf.impl.NetconfServerSession;

 import org.opendaylight.controller.netconf.util.mapping.AbstractSingletonNetconfOperation;
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.opendaylight.controller.netconf.util.mapping.AbstractSingletonNetconfOperation;
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;

 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 

-public class DefaultCloseSession extends AbstractSingletonNetconfOperation {
+public class DefaultCloseSession extends AbstractSingletonNetconfOperation implements DefaultNetconfOperation {
+    private static final Logger LOG = LoggerFactory.getLogger(DefaultCloseSession.class);
+

     public static final String CLOSE_SESSION = "close-session";
     public static final String CLOSE_SESSION = "close-session";

+

     private final AutoCloseable sessionResources;
     private final AutoCloseable sessionResources;

+    private NetconfServerSession session;

 
     public DefaultCloseSession(String netconfSessionIdForReporting, AutoCloseable sessionResources) {
         super(netconfSessionIdForReporting);
 
     public DefaultCloseSession(String netconfSessionIdForReporting, AutoCloseable sessionResources) {
         super(netconfSessionIdForReporting);


@@ -42,6 +50,8 @@ public class DefaultCloseSession extends AbstractSingletonNetconfOperation {
             throws NetconfDocumentedException {
         try {
             sessionResources.close();
             throws NetconfDocumentedException {
         try {
             sessionResources.close();

+            Preconditions.checkNotNull(session, "Session was not set").delayedClose();
+            LOG.info("Session {} closing", session.getSessionId());

         } catch (Exception e) {
             throw new NetconfDocumentedException("Unable to properly close session "
                     + getNetconfSessionIdForReporting(), NetconfDocumentedException.ErrorType.application,
         } catch (Exception e) {
             throw new NetconfDocumentedException("Unable to properly close session "
                     + getNetconfSessionIdForReporting(), NetconfDocumentedException.ErrorType.application,


@@ -51,4 +61,9 @@ public class DefaultCloseSession extends AbstractSingletonNetconfOperation {
         }
         return XmlUtil.createElement(document, XmlNetconfConstants.OK, Optional.<String>absent());
     }
         }
         return XmlUtil.createElement(document, XmlNetconfConstants.OK, Optional.<String>absent());
     }

+
+    @Override
+    public void setNetconfSession(final NetconfServerSession s) {
+        this.session = s;
+    }

 }
 }

diff --git a/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java b/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
index d82c1484e49e2f4786c5ac3ee7ddca51d6a3b95d..b45b116b1262cbc67879de52a49482e0f7b43128 100644 (file)
--- a/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
+++ b/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
@@ -8,25 +8,67 @@
 
 package org.opendaylight.controller.netconf.impl.mapping.operations;
 
 
 package org.opendaylight.controller.netconf.impl.mapping.operations;
 

+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.doAnswer;

 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doNothing;

+import static org.mockito.Mockito.doReturn;

 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;

+import static org.mockito.Mockito.verify;

 
 

+import io.netty.channel.Channel;
+import io.netty.channel.ChannelFuture;
+import io.netty.util.concurrent.GenericFutureListener;

 import org.junit.Test;
 import org.junit.Test;

+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;

 import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
 import org.opendaylight.controller.netconf.api.NetconfDocumentedException;

+import org.opendaylight.controller.netconf.api.NetconfMessage;
+import org.opendaylight.controller.netconf.api.NetconfTerminationReason;
+import org.opendaylight.controller.netconf.impl.NetconfServerSession;
+import org.opendaylight.controller.netconf.impl.NetconfServerSessionListener;
+import org.opendaylight.controller.netconf.util.messages.NetconfHelloMessageAdditionalHeader;

 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.w3c.dom.Document;
 
 public class DefaultCloseSessionTest {
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.w3c.dom.Document;
 
 public class DefaultCloseSessionTest {

+

     @Test
     public void testDefaultCloseSession() throws Exception {
         AutoCloseable res = mock(AutoCloseable.class);
         doNothing().when(res).close();
     @Test
     public void testDefaultCloseSession() throws Exception {
         AutoCloseable res = mock(AutoCloseable.class);
         doNothing().when(res).close();

-        DefaultCloseSession session = new DefaultCloseSession("", res);
+        DefaultCloseSession close = new DefaultCloseSession("", res);

         Document doc = XmlUtil.newDocument();
         XmlElement elem = XmlElement.fromDomElement(XmlUtil.readXmlToElement("<elem/>"));
         Document doc = XmlUtil.newDocument();
         XmlElement elem = XmlElement.fromDomElement(XmlUtil.readXmlToElement("<elem/>"));

-        session.handleWithNoSubsequentOperations(doc, elem);
+        final Channel channel = mock(Channel.class);
+        doReturn("channel").when(channel).toString();
+        doReturn(mock(ChannelFuture.class)).when(channel).close();
+
+        final ChannelFuture sendFuture = mock(ChannelFuture.class);
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(final InvocationOnMock invocation) throws Throwable {
+                ((GenericFutureListener) invocation.getArguments()[0]).operationComplete(sendFuture);
+                return null;
+            }
+        }).when(sendFuture).addListener(any(GenericFutureListener.class));
+        doReturn(sendFuture).when(channel).writeAndFlush(anyObject());
+        final NetconfServerSessionListener listener = mock(NetconfServerSessionListener.class);
+        doNothing().when(listener).onSessionTerminated(any(NetconfServerSession.class), any(NetconfTerminationReason.class));
+        final NetconfServerSession session =
+                new NetconfServerSession(listener, channel, 1L,
+                        NetconfHelloMessageAdditionalHeader.fromString("[netconf;10.12.0.102:48528;ssh;;;;;;]"));
+        close.setNetconfSession(session);
+        close.handleWithNoSubsequentOperations(doc, elem);
+        // Fake close response to trigger delayed close
+        session.sendMessage(new NetconfMessage(XmlUtil.readXmlToDocument("<rpc-reply message-id=\"101\"\n" +
+                "xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">\n" +
+                "<ok/>\n" +
+                "</rpc-reply>")));
+        verify(channel).close();
+        verify(listener).onSessionTerminated(any(NetconfServerSession.class), any(NetconfTerminationReason.class));

     }
 
     @Test(expected = NetconfDocumentedException.class)
     }
 
     @Test(expected = NetconfDocumentedException.class)