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 2020/07/29 17:43:21 UTC

[GitHub] [bookkeeper] nicoloboschi opened a new pull request #2393: Spammy log when one bookie of ensemble is down #2285

nicoloboschi opened a new pull request #2393:
URL: https://github.com/apache/bookkeeper/pull/2393


   Descriptions of the changes in this PR:
   
   
   
   ### Motivation
   
   Resolves #2285
   
   ### Changes
   
   We write "bookie server unavailable log" every 30 seconds per bookie client


----------------------------------------------------------------
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



[GitHub] [bookkeeper] eolivelli merged pull request #2393: Spammy log when one bookie of ensemble is down #2285

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #2393:
URL: https://github.com/apache/bookkeeper/pull/2393


   


----------------------------------------------------------------
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



[GitHub] [bookkeeper] eolivelli commented on pull request #2393: Spammy log when one bookie of ensemble is down #2285

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2393:
URL: https://github.com/apache/bookkeeper/pull/2393#issuecomment-696010766


   committed to master branch, thank you @nicoloboschi 
   
   If you want  please create a separate patch for branch-4.11


----------------------------------------------------------------
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



[GitHub] [bookkeeper] eolivelli merged pull request #2393: Spammy log when one bookie of ensemble is down #2285

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #2393:
URL: https://github.com/apache/bookkeeper/pull/2393


   


----------------------------------------------------------------
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



[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2393: Spammy log when one bookie of ensemble is down #2285

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2393:
URL: https://github.com/apache/bookkeeper/pull/2393#discussion_r462748122



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
##########
@@ -2488,6 +2490,14 @@ public void operationComplete(ChannelFuture future) throws Exception {
 
             makeWritable();
         }
+
+        private void logBookieUnavailable(Consumer<Void> log) {

Review comment:
       why `Consumer<Void> log` and not `Runnable` ?

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
##########
@@ -2488,6 +2490,14 @@ public void operationComplete(ChannelFuture future) throws Exception {
 
             makeWritable();
         }
+
+        private void logBookieUnavailable(Consumer<Void> log) {
+            final long now = System.currentTimeMillis();
+            if ((now - lastBookieUnavailableLog) > 30_000) {

Review comment:
       we should make this 30_000 configurable or calculate it from other timeout configuration parameters
   

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
##########
@@ -331,6 +332,7 @@
     private final ExtensionRegistry extRegistry;
     private final SecurityHandlerFactory shFactory;
     private volatile boolean isWritable = true;
+    private long lastBookieUnavailableLog = 0;

Review comment:
       what about renaming it to `lastBookieUnavailableLogTimestamp` ?




----------------------------------------------------------------
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



[GitHub] [bookkeeper] eolivelli commented on pull request #2393: Spammy log when one bookie of ensemble is down #2285

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2393:
URL: https://github.com/apache/bookkeeper/pull/2393#issuecomment-696010766


   committed to master branch, thank you @nicoloboschi 
   
   If you want  please create a separate patch for branch-4.11


----------------------------------------------------------------
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