NETCONF scalability: share instances

Scalability testing shows that the netconf session code:
- retains XML document reference
- does not reuse QName instances

Change-Id: I6b58355e3fa74d67872d665d0f8dc2d16abe156c
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java
index 1a7d90e9c0ad1d9424da229b29eef9ebf7c9b6a4..0999efff0f6aed20bba510d2de207e316166c3df 100644 (file)
--- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java
+++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java
@@ -7,8 +7,8 @@ import com.google.common.base.Predicate;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-
 import com.google.common.collect.Sets;
+
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
@@ -158,7 +158,7 @@ public final class NetconfSessionCapabilities {
             }
 
             // FIXME: do we really want to continue here?
-            moduleBasedCaps.add(QName.create(namespace, revision, moduleName));
+            moduleBasedCaps.add(QName.cachedReference(QName.create(namespace, revision, moduleName)));
             nonModuleCaps.remove(capability);
         }
 

diff --git a/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSession.java b/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSession.java
index 004a22f6948c570e96fc880ea764b366c8cbfd94..bd91af5bcc95ba7f691d6880bbabe9483a865d2a 100644 (file)
--- a/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSession.java
+++ b/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSession.java
@@ -9,7 +9,9 @@
 package org.opendaylight.controller.netconf.client;
 
 import io.netty.channel.Channel;
+
 import java.util.Collection;
+
 import org.opendaylight.controller.netconf.nettyutil.AbstractNetconfSession;
 import org.opendaylight.controller.netconf.nettyutil.handler.NetconfEXICodec;
 import org.opendaylight.controller.netconf.nettyutil.handler.NetconfEXIToMessageDecoder;
@@ -24,8 +26,16 @@ public class NetconfClientSession extends AbstractNetconfSession<NetconfClientSe
     private static final Logger logger = LoggerFactory.getLogger(NetconfClientSession.class);
     private final Collection<String> capabilities;
 
-    public NetconfClientSession(NetconfClientSessionListener sessionListener, Channel channel, long sessionId,
-            Collection<String> capabilities) {
+    /**
+     * Construct a new session.
+     *
+     * @param sessionListener
+     * @param channel
+     * @param sessionId
+     * @param capabilities set of advertised capabilities. Expected to be immutable.
+     */
+    public NetconfClientSession(final NetconfClientSessionListener sessionListener, final Channel channel, final long sessionId,
+            final Collection<String> capabilities) {
         super(sessionListener, channel, sessionId);
         this.capabilities = capabilities;
         logger.debug("Client Session {} created", toString());
@@ -41,7 +51,7 @@ public class NetconfClientSession extends AbstractNetconfSession<NetconfClientSe
     }
 
     @Override
-    protected void addExiHandlers(NetconfEXICodec exiCodec) {
+    protected void addExiHandlers(final NetconfEXICodec exiCodec) {
         // TODO used only in negotiator, client supports only auto start-exi
         replaceMessageDecoder(new NetconfEXIToMessageDecoder(exiCodec));
         replaceMessageEncoder(new NetconfMessageToEXIEncoder(exiCodec));

diff --git a/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSessionNegotiator.java b/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSessionNegotiator.java
index 971ea39ade5b6fb57d07a041f999ec18890528df..e2ac49c3efed823b3899fa952e6742eb2bcade95 100644 (file)
--- a/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSessionNegotiator.java
+++ b/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSessionNegotiator.java
@@ -8,6 +8,8 @@
 
 package org.opendaylight.controller.netconf.client;
 
+import com.google.common.collect.ImmutableList;
+
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelFutureListener;
@@ -48,17 +50,17 @@ public class NetconfClientSessionNegotiator extends
 
     private static final String EXI_1_0_CAPABILITY_MARKER = "exi:1.0";
 
-    protected NetconfClientSessionNegotiator(NetconfClientSessionPreferences sessionPreferences,
-                                             Promise<NetconfClientSession> promise,
-                                             Channel channel,
-                                             Timer timer,
-                                             NetconfClientSessionListener sessionListener,
-                                             long connectionTimeoutMillis) {
+    protected NetconfClientSessionNegotiator(final NetconfClientSessionPreferences sessionPreferences,
+                                             final Promise<NetconfClientSession> promise,
+                                             final Channel channel,
+                                             final Timer timer,
+                                             final NetconfClientSessionListener sessionListener,
+                                             final long connectionTimeoutMillis) {
         super(sessionPreferences, promise, channel, timer, sessionListener, connectionTimeoutMillis);
     }
 
     @Override
-    protected void handleMessage(NetconfHelloMessage netconfMessage) throws NetconfDocumentedException {
+    protected void handleMessage(final NetconfHelloMessage netconfMessage) throws NetconfDocumentedException {
         final NetconfClientSession session = getSessionForHelloMessage(netconfMessage);
         replaceHelloMessageInboundHandler(session);
 
@@ -98,7 +100,7 @@ public class NetconfClientSessionNegotiator extends
         });
     }
 
-    private boolean shouldUseExi(NetconfHelloMessage helloMsg) {
+    private boolean shouldUseExi(final NetconfHelloMessage helloMsg) {
         return containsExi10Capability(helloMsg.getDocument())
                 && containsExi10Capability(sessionPreferences.getHelloMessage().getDocument());
     }
@@ -113,7 +115,7 @@ public class NetconfClientSessionNegotiator extends
         return false;
     }
 
-    private long extractSessionId(Document doc) {
+    private long extractSessionId(final Document doc) {
         final Node sessionIdNode = (Node) XmlUtil.evaluateXPath(sessionIdXPath, doc, XPathConstants.NODE);
         String textContent = sessionIdNode.getTextContent();
         if (textContent == null || textContent.equals("")) {
@@ -124,10 +126,14 @@ public class NetconfClientSessionNegotiator extends
     }
 
     @Override
-    protected NetconfClientSession getSession(NetconfClientSessionListener sessionListener, Channel channel,
-            NetconfHelloMessage message) throws NetconfDocumentedException {
+    protected NetconfClientSession getSession(final NetconfClientSessionListener sessionListener, final Channel channel,
+            final NetconfHelloMessage message) throws NetconfDocumentedException {
         long sessionId = extractSessionId(message.getDocument());
-        Collection<String> capabilities = NetconfMessageUtil.extractCapabilitiesFromHello(message.getDocument());
+
+        // Copy here is important: it disconnects the strings from the document
+        Collection<String> capabilities = ImmutableList.copyOf(NetconfMessageUtil.extractCapabilitiesFromHello(message.getDocument()));
+
+        // FIXME: scalability: we could instantiate a cache to share the same collections
         return new NetconfClientSession(sessionListener, channel, sessionId, capabilities);
     }
 
@@ -138,15 +144,15 @@ public class NetconfClientSessionNegotiator extends
         private static final String EXI_CONFIRMED_HANDLER = "exiConfirmedHandler";
 
         private final NetconfClientSession session;
-        private NetconfStartExiMessage startExiMessage;
+        private final NetconfStartExiMessage startExiMessage;
 
-        ExiConfirmationInboundHandler(NetconfClientSession session, final NetconfStartExiMessage startExiMessage) {
+        ExiConfirmationInboundHandler(final NetconfClientSession session, final NetconfStartExiMessage startExiMessage) {
             this.session = session;
             this.startExiMessage = startExiMessage;
         }
 
         @Override
-        public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
+        public void channelRead(final ChannelHandlerContext ctx, final Object msg) throws Exception {
             ctx.pipeline().remove(ExiConfirmationInboundHandler.EXI_CONFIRMED_HANDLER);
 
             NetconfMessage netconfMessage = (NetconfMessage) msg;