zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ivmaykov <...@git.apache.org>
Subject [GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4
Date Wed, 14 Nov 2018 22:57:39 GMT
Github user ivmaykov commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/669#discussion_r233649908
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
---
    @@ -116,170 +116,104 @@ public void channelConnected(ChannelHandlerContext ctx,
     
                 NettyServerCnxn cnxn = new NettyServerCnxn(channel,
                         zkServer, NettyServerCnxnFactory.this);
    -            ctx.setAttachment(cnxn);
    +            ctx.channel().attr(CONNECTION_ATTRIBUTE).set(cnxn);
     
                 if (secure) {
    -                SslHandler sslHandler = ctx.getPipeline().get(SslHandler.class);
    -                ChannelFuture handshakeFuture = sslHandler.handshake();
    +                SslHandler sslHandler = ctx.pipeline().get(SslHandler.class);
    +                Future<Channel> handshakeFuture = sslHandler.handshakeFuture();
                     handshakeFuture.addListener(new CertificateVerifier(sslHandler, cnxn));
                 } else {
    -                allChannels.add(ctx.getChannel());
    +                allChannels.add(ctx.channel());
                     addCnxn(cnxn);
                 }
             }
     
             @Override
    -        public void channelDisconnected(ChannelHandlerContext ctx,
    -                ChannelStateEvent e) throws Exception
    -        {
    +        public void channelInactive(ChannelHandlerContext ctx) throws Exception {
                 if (LOG.isTraceEnabled()) {
    -                LOG.trace("Channel disconnected " + e);
    +                LOG.trace("Channel inactive {}", ctx.channel());
                 }
    -            NettyServerCnxn cnxn = (NettyServerCnxn) ctx.getAttachment();
    +            allChannels.remove(ctx.channel());
    +            NettyServerCnxn cnxn = ctx.channel().attr(CONNECTION_ATTRIBUTE).getAndSet(null);
                 if (cnxn != null) {
                     if (LOG.isTraceEnabled()) {
    -                    LOG.trace("Channel disconnect caused close " + e);
    +                    LOG.trace("Channel inactive caused close {}", cnxn);
                     }
                     cnxn.close();
                 }
             }
     
             @Override
    -        public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent e)
    -            throws Exception
    -        {
    -            LOG.warn("Exception caught " + e, e.getCause());
    -            NettyServerCnxn cnxn = (NettyServerCnxn) ctx.getAttachment();
    +        public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws
Exception {
    --- End diff --
    
    We call `cnxn.close()` at the end of `exceptionCaught()`, which will end up closing the
channel so `channelInactive()` will get called, so I think it would be redundant to remove
from `allChannels` here.


---

Mime
View raw message