You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/01/15 12:44:03 UTC

[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2538: Fix BookKeeper shell "list bookies" in case of downbookie and reduce noisy log during re-replication in case of down bookie

eolivelli commented on a change in pull request #2538:
URL: https://github.com/apache/bookkeeper/pull/2538#discussion_r558281325



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultBookieAddressResolver.java
##########
@@ -59,13 +59,13 @@ public BookieSocketAddress resolve(BookieId bookieId) {
                 log.debug("Resolving dummy bookie Id {} using legacy bookie resolver", bookieId);
                 return BookieSocketAddress.resolveDummyBookieId(bookieId);
             }
-            log.info("Cannot resolve {}, bookie is unknown", bookieId, ex);
+            log.debug("Cannot resolve {}, bookie is unknown", bookieId, ex.toString());

Review comment:
       that was my thought when I first wrote this code, but actually we are appending the exception to the chain of the thrown BookieIdNotResolvedException, so we are going to report this error twice (here and in the caller).
   I can leave it at "info" level, but the stacktrace is redundant this is case

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultBookieAddressResolver.java
##########
@@ -59,13 +59,13 @@ public BookieSocketAddress resolve(BookieId bookieId) {
                 log.debug("Resolving dummy bookie Id {} using legacy bookie resolver", bookieId);
                 return BookieSocketAddress.resolveDummyBookieId(bookieId);
             }
-            log.info("Cannot resolve {}, bookie is unknown", bookieId, ex);
+            log.debug("Cannot resolve {}, bookie is unknown", bookieId, ex.toString());
             throw new BookieIdNotResolvedException(bookieId, ex);
         } catch (Exception ex) {
             if (ex instanceof InterruptedException) {
                 Thread.currentThread().interrupt();
             }
-            log.info("Cannot resolve {} ", bookieId, ex);
+            log.debug("Cannot resolve {} ", bookieId, ex);

Review comment:
        we are appending the exception to the chain of the thrown BookieIdNotResolvedException, so we are going to report this error twice (here and in the caller).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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