AnonHxy commented on code in PR #4023:
URL: https://github.com/apache/bookkeeper/pull/4023#discussion_r1274289307


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java:
##########
@@ -968,12 +968,18 @@ public void scanEntryLog(long entryLogId, EntryLogScanner 
scanner) throws IOExce
         // Buffer where to read the entrySize (4 bytes) and the ledgerId (8 
bytes)
         ByteBuf headerBuffer = Unpooled.buffer(4 + 8);
         BufferedReadChannel bc;
+        boolean throwExceptionWhenGetChannel = false;
         // Get the BufferedChannel for the current entry log file
         try {
             bc = getChannelForLogId(entryLogId);
         } catch (IOException e) {
             LOG.warn("Failed to get channel to scan entry log: " + entryLogId 
+ ".log");
+            throwExceptionWhenGetChannel = true;

Review Comment:
   Why not call `headerBuffer.release()` here?
   
   And I think that it will be better if we move line969 to just before the 
second `try catch` block, like:
   ```
   ByteBuf data = allocator.directBuffer(1024 * 1024);
   ByteBuf headerBuffer = Unpooled.buffer(4 + 8);  // line969
    try {
              
               // Read through the entry log file and extract the ledger ID's.
               while (true) {
                   // Check if we'`
               // ...
              // ...
    finally {
              ReferenceCountUtil.release(headerBuffer);
               ReferenceCountUtil.release(data);
      }
   ```



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