hangc0276 opened a new issue, #3792:
URL: https://github.com/apache/bookkeeper/issues/3792

   **BP**
   
   > Follow the instructions at 
https://bookkeeper.apache.org/community/bookkeeper-proposals/ to create a 
proposal.
   
   This is the master ticket for tracking BP-61 :
   
   ### Motivation
   
   In BP-59, which was not discussed in the dev mail list and without a vote 
refactored the ByteBuf release method by ReferenceCountUtil.safeRelease() 
instead of ByteBuf.release().
   
   In the `ReferenceCountUtil.safeRelease()`, it catches the release exception. 
This change can make the ByteBuf release without any exceptions and makes the 
code run well, but it will make bugs hide deeper and more challenging to find 
out.
      ```java
      public static void safeRelease(Object msg) {
           try {
               release(msg);
           } catch (Throwable t) {
               logger.warn("Failed to release a message: {}", msg, t);
           }
       }
      ```
   For example, if there is a bug to release the ByteBuf twice, whose refcnt is 
1, we can find out the bug quickly if we use ByteBuf.release(), but the bug 
will be hard to find out if we use `ReferenceCountUtil.safeRelease()`
   
   There are 12 PRs to refactor the ByteBuf release method, and I suggest 
reverting those PRs. 
   
   - https://github.com/apache/bookkeeper/pull/3673
   - https://github.com/apache/bookkeeper/pull/3674
   - https://github.com/apache/bookkeeper/pull/3687
   - https://github.com/apache/bookkeeper/pull/3688
   - https://github.com/apache/bookkeeper/pull/3689
   - https://github.com/apache/bookkeeper/pull/3691
   - https://github.com/apache/bookkeeper/pull/3693
   - https://github.com/apache/bookkeeper/pull/3694
   - https://github.com/apache/bookkeeper/pull/3695
   - https://github.com/apache/bookkeeper/pull/3698
   - https://github.com/apache/bookkeeper/pull/3700
   - https://github.com/apache/bookkeeper/pull/3703
   
   Considering some PRs have been cherry-picked into the release branch but 
others not, it will be hard to cherry-pick if we just push one PR to revert all 
the above PRs. I will push Prs one by one to replace 
`ReferenceCountUtil.safeRelease()` with `ReferenceCountUtil.release()`
   
   <!-- add a proposal PR link below -->
   ### Proposal PR


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