hangc0276 commented on code in PR #3528:
URL: https://github.com/apache/bookkeeper/pull/3528#discussion_r994704420


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java:
##########
@@ -432,10 +433,7 @@ public String toString() {
                                  opCode, ledgerId, entryId, errorCode);
         }
 
-        void retain() {
-        }
-
-        void release() {

Review Comment:
   It has conflict with the release() interface in `ReferenceCounted`



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java:
##########
@@ -445,7 +443,7 @@ void recycle() {
     /**
      * A request that reads data.
      */
-    class ReadResponse extends Response {
+    class ReadResponse extends Response implements ReferenceCounted {

Review Comment:
   https://github.com/apache/bookkeeper/issues/3527 will give more context.



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -145,12 +152,12 @@ protected void sendResponseAndWait(int rc, Object 
response, OpStatsLogger statsL
         try {
             ChannelFuture future = channel.writeAndFlush(response);
             if (!channel.eventLoop().inEventLoop()) {
-                future.await();

Review Comment:
   `await()` won't throw the exception out. We'd better catch the exception and 
log it in debug mode to help debug issues.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to