jvrao commented on a change in pull request #1088: ISSUE #1086 (@bug 
W-4146427@) Client-side backpressure in netty (Fixes: 
io.netty.util.internal.OutOfDirectMemoryError under continuous heavy load)
URL: https://github.com/apache/bookkeeper/pull/1088#discussion_r168385250
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
 ##########
 @@ -863,17 +902,44 @@ private ChannelFuture closeChannel(Channel c) {
         if (LOG.isDebugEnabled()) {
             LOG.debug("Closing channel {}", c);
         }
-        return c.close();
+        return c.close().addListener(x -> makeWritable());
+    }
+
+    @Override
+    public void channelWritabilityChanged(ChannelHandlerContext ctx) throws 
Exception {
+        final Channel c = channel;
+        if (c == null || c.isWritable()) {
+            makeWritable();
+        }
+        super.channelWritabilityChanged(ctx);
     }
 
     private void writeAndFlush(final Channel channel,
                                final CompletionKey key,
                                final Object request) {
+        writeAndFlush(channel, key, request, false);
+    }
+
+    private void writeAndFlush(final Channel channel,
+                           final CompletionKey key,
+                           final Object request,
+                           final boolean allowFastFail) {
         if (channel == null) {
             errorOut(key);
             return;
         }
 
+        if (isWritable != channel.isWritable()) {
+            // isWritable is volatile so simple "isWritable = 
channel.isWritable()" would be slower
+            isWritable = !isWritable;
+        }
+
+        if (allowFastFail && !isWritable) {
+            errorOut(key);
 
 Review comment:
   This returns BookieHandleNotAvailableException. It is usually interpreted as 
not able to connect to bookie basically cannelPool returning NULL. I think we 
should return a different error here (busy??). Not sure if we can handle it
   differently but we can at least bubble it back to the application where the 
application will know the reason. 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to