You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/10/31 08:19:33 UTC

[bookkeeper] branch branch-4.7 updated: Reduce stack traces in logs for common cases

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch branch-4.7
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/branch-4.7 by this push:
     new e765c42  Reduce stack traces in logs for common cases
e765c42 is described below

commit e765c426dbced50a479e7fb1d409fe20ddfe09b5
Author: Matteo Merli <mm...@apache.org>
AuthorDate: Wed Oct 31 01:18:57 2018 -0700

    Reduce stack traces in logs for common cases
    
    ### Motivation
    
    For common "error" conditions the exception message is the only important bit while the stack traces are just adding clutter the logs files.
    
    Reviewers: Andrey Yegorov <None>, Sijie Guo <si...@apache.org>
    
    This closes #1776 from merlimat/master
    
    (cherry picked from commit b17b1657e938aa41d747fd49537559c4f29860de)
    Signed-off-by: Sijie Guo <si...@apache.org>
---
 .../java/org/apache/bookkeeper/bookie/Journal.java |  4 ++--
 .../bookkeeper/proto/PerChannelBookieClient.java   | 23 +++++++++++++++++++---
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
index 94aafad..40d31fb 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
@@ -495,7 +495,7 @@ public class Journal extends BookieCriticalThread implements CheckpointSource {
                     running = false;
                 } catch (InterruptedException e) {
                     Thread.currentThread().interrupt();
-                    LOG.error("ForceWrite thread interrupted", e);
+                    LOG.info("ForceWrite thread interrupted");
                     // close is idempotent
                     if (null != req) {
                         req.shouldClose = true;
@@ -1097,7 +1097,7 @@ public class Journal extends BookieCriticalThread implements CheckpointSource {
             LOG.error("I/O exception in Journal thread!", ioe);
         } catch (InterruptedException ie) {
             Thread.currentThread().interrupt();
-            LOG.warn("Journal exits when shutting down", ie);
+            LOG.info("Journal exits when shutting down");
         } finally {
             // There could be packets queued for forceWrite on this logFile
             // That is fine as this exception is going to anyway take down the
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
index b03b77a..6b356f4 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
@@ -49,6 +49,7 @@ import io.netty.channel.epoll.EpollEventLoopGroup;
 import io.netty.channel.epoll.EpollSocketChannel;
 import io.netty.channel.local.LocalChannel;
 import io.netty.channel.socket.nio.NioSocketChannel;
+import io.netty.channel.unix.Errors.NativeIoException;
 import io.netty.handler.codec.CorruptedFrameException;
 import io.netty.handler.codec.DecoderException;
 import io.netty.handler.codec.LengthFieldBasedFrameDecoder;
@@ -62,6 +63,7 @@ import io.netty.util.concurrent.GenericFutureListener;
 
 import java.io.IOException;
 import java.net.SocketAddress;
+import java.net.UnknownHostException;
 import java.security.cert.Certificate;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -1042,7 +1044,13 @@ public class PerChannelBookieClient extends ChannelInboundHandlerAdapter {
         }
 
         if (cause instanceof IOException) {
-            LOG.warn("Exception caught on:{} cause:", ctx.channel(), cause);
+            if (cause instanceof NativeIoException) {
+                // Stack trace is not very interesting for native IO exceptio, the important part is in
+                // the exception message
+                LOG.warn("Exception caught on:{} cause: {}", ctx.channel(), cause.getMessage());
+            } else {
+                LOG.warn("Exception caught on:{} cause:", ctx.channel(), cause);
+            }
             ctx.close();
             return;
         }
@@ -2039,8 +2047,17 @@ public class PerChannelBookieClient extends ChannelInboundHandlerAdapter {
                     closeChannel(future.channel());
                     return; // pendingOps should have been completed when other channel connected
                 } else {
-                    LOG.error("Could not connect to bookie: {}/{}, current state {} : ",
-                            future.channel(), addr, state, future.cause());
+                    Throwable cause = future.cause();
+                    if (cause instanceof UnknownHostException || cause instanceof NativeIoException) {
+                        // Don't log stack trace for common errors
+                        LOG.warn("Could not connect to bookie: {}/{}, current state {} : {}",
+                                future.channel(), addr, state, future.cause().getMessage());
+                    } else {
+                        // Regular exceptions, include stack trace
+                        LOG.error("Could not connect to bookie: {}/{}, current state {} : ",
+                                future.channel(), addr, state, future.cause());
+                    }
+
                     rc = BKException.Code.BookieHandleNotAvailableException;
                     closeChannel(future.channel());
                     channel = null;