RATIS-2412. NettyClient LoggingHandler not working.

[slfan1989]

What changes were proposed in this pull request?

Root Cause

The Bootstrap.handler() method stores the handler in a single field. Each call overwrites the previous value instead of appending. As a result, only the last handler (initializer) is applied, and LoggingHandler is completely ignored.

• Bootstrap.handler()

public B handler(ChannelHandler handler) {
    this.handler = ObjectUtil.checkNotNull(handler, "handler");
    return self();
}

Solution

Combine both handlers into a single ChannelInitializer. This combined initializer first adds the LoggingHandler to the pipeline, then adds the original initializer which will be automatically processed by Netty to initialize the remaining handlers.

What is the link to the Apache JIRA

RATIS-2412. NettyClient LoggingHandler not working.

How was this patch tested?

• Exist Junit Test.

[slfan1989]

@szetszwo Could you please review this PR? Thanks so much!

[szetszwo]

@slfan1989 , thanks for working on this! Good catch on the bug.

• Let's add it in the initializer, see below.
• BTW, it currently uses LogLevel.INFO. Would it print a lot messages? Since this is client, how about changing it to WARN?

diff --git a/ratis-netty/src/main/java/org/apache/ratis/netty/NettyClient.java b/ratis-netty/src/main/java/org/apache/ratis/netty/NettyClient.java
index a42ddaca8..91f970dd3 100644
--- a/ratis-netty/src/main/java/org/apache/ratis/netty/NettyClient.java
+++ b/ratis-netty/src/main/java/org/apache/ratis/netty/NettyClient.java
@@ -52,7 +52,6 @@ public class NettyClient implements Closeable {
         () -> channel = new Bootstrap()
             .group(group)
             .channel(NettyUtils.getSocketChannelClass(group))
-            .handler(new LoggingHandler(LogLevel.INFO))
             .handler(initializer)
             .connect(address)
             .sync()
diff --git a/ratis-netty/src/main/java/org/apache/ratis/netty/NettyRpcProxy.java b/ratis-netty/src/main/java/org/apache/ratis/netty/NettyRpcProxy.java
index f77096e18..fca0834a0 100644
--- a/ratis-netty/src/main/java/org/apache/ratis/netty/NettyRpcProxy.java
+++ b/ratis-netty/src/main/java/org/apache/ratis/netty/NettyRpcProxy.java
@@ -31,6 +31,8 @@ import org.apache.ratis.proto.RaftProtos.RaftRpcRequestProto;
 import org.apache.ratis.proto.netty.NettyProtos.RaftNettyServerReplyProto;
 import org.apache.ratis.proto.netty.NettyProtos.RaftNettyServerRequestProto;
 import org.apache.ratis.protocol.exceptions.AlreadyClosedException;
+import org.apache.ratis.thirdparty.io.netty.handler.logging.LogLevel;
+import org.apache.ratis.thirdparty.io.netty.handler.logging.LoggingHandler;
 import org.apache.ratis.util.IOUtils;
 import org.apache.ratis.util.PeerProxyMap;
 import org.apache.ratis.util.ProtoUtils;
@@ -141,9 +143,10 @@ public class NettyRpcProxy implements Closeable {
       final ChannelInitializer<SocketChannel> initializer
           = new ChannelInitializer<SocketChannel>() {
         @Override
-        protected void initChannel(SocketChannel ch) throws Exception {
+        protected void initChannel(SocketChannel ch) {
           final ChannelPipeline p = ch.pipeline();
 
+          p.addLast(new LoggingHandler(LogLevel.WARN));
           p.addLast(new ProtobufVarint32FrameDecoder());
           p.addLast(new ProtobufDecoder(RaftNettyServerReplyProto.getDefaultInstance()));
           p.addLast(new ProtobufVarint32LengthFieldPrepender());