[GitHub] merlimat closed pull request #1107: Log only first exception in the connection exception handler

[GitBox <gi...@apache.org>]

merlimat closed pull request #1107: Log only first exception in the connection exception handler
URL: https://github.com/apache/incubator-pulsar/pull/1107
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
index df8e577c7..bd49fc76f 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
@@ -44,6 +44,7 @@
 import org.apache.pulsar.broker.web.RestException;
 import org.apache.pulsar.client.api.PulsarClientException;
 import org.apache.pulsar.client.impl.BatchMessageIdImpl;
+import org.apache.pulsar.client.impl.ClientCnx;
 import org.apache.pulsar.client.impl.MessageIdImpl;
 import org.apache.pulsar.common.api.CommandUtils;
 import org.apache.pulsar.common.api.Commands;
@@ -102,7 +103,7 @@
     private String originalPrincipal;
 
     enum State {
-        Start, Connected
+        Start, Connected, Failed
     }
 
     public ServerCnx(BrokerService service) {
@@ -164,7 +165,18 @@ public void channelWritabilityChanged(ChannelHandlerContext ctx) throws Exceptio
 
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-        log.warn("[{}] Got exception: {}", remoteAddress, cause.getMessage(), cause);
+        if (state != State.Failed) {
+            // No need to report stack trace for known exceptions that happen in disconnections
+            log.warn("[{}] Got exception {} : {}", remoteAddress, cause.getClass().getSimpleName(), cause.getMessage(),
+                    ClientCnx.isKnownException(cause) ? null : cause);
+            state = State.Failed;
+        } else {
+            // At default info level, suppress all subsequent exceptions that are thrown when the connection has already
+            // failed
+            if (log.isDebugEnabled()) {
+                log.debug("[{}] Got exception: {}", remoteAddress, cause.getMessage(), cause);
+            }
+        }
         ctx.close();
     }
 
diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java
index 2eaa1d459..85e286745 100644
--- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java
+++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java
@@ -23,6 +23,7 @@
 
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
+import java.nio.channels.ClosedChannelException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
@@ -57,6 +58,7 @@
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.EventLoopGroup;
+import io.netty.channel.unix.Errors.NativeIoException;
 import io.netty.util.concurrent.Promise;
 
 public class ClientCnx extends PulsarHandler {
@@ -85,7 +87,7 @@
     private String proxyToTargetBrokerAddress = null;
 
     enum State {
-        None, SentConnectFrame, Ready
+        None, SentConnectFrame, Ready, Failed
     }
 
     public ClientCnx(ClientConfiguration conf, EventLoopGroup eventLoopGroup) {
@@ -152,10 +154,26 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception {
 
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-        log.warn("{} Exception caught: {}", ctx.channel(), cause.getMessage(), cause);
+        if (state != State.Failed) {
+            // No need to report stack trace for known exceptions that happen in disconnections
+            log.warn("[{}] Got exception {} : {}", remoteAddress, cause.getClass().getSimpleName(), cause.getMessage(),
+                    isKnownException(cause) ? null : cause);
+            state = State.Failed;
+        } else {
+            // At default info level, suppress all subsequent exceptions that are thrown when the connection has already
+            // failed
+            if (log.isDebugEnabled()) {
+                log.debug("[{}] Got exception: {}", remoteAddress, cause.getMessage(), cause);
+            }
+        }
+
         ctx.close();
     }
 
+    public static boolean isKnownException(Throwable t) {
+        return t instanceof NativeIoException || t instanceof ClosedChannelException;
+    }
+
     @Override
     protected void handleConnected(CommandConnected connected) {
         checkArgument(state == State.SentConnectFrame);


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services