Github user eolivelli commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/669#discussion_r226839114
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
---
@@ -116,170 +115,94 @@ 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
- {
- if (LOG.isTraceEnabled()) {
- LOG.trace("Channel disconnected " + e);
- }
- NettyServerCnxn cnxn = (NettyServerCnxn) ctx.getAttachment();
+ public void channelInactive(ChannelHandlerContext ctx) throws
Exception {
+ LOG.trace("Channel inactive {}", ctx.channel());
--- End diff --
It is better to add the 'if' because in general you will skip calling the
logger method, with all what is comes with it: evaluating expressions for
parameters, passing parameters for the method call, and calling the method.
You will trade a single cheap method call with a potential expense of
resources and useless allocations.
IMHO it is better to have this pattern consistently in the whole code
---
