There are three ways to write release in current pulsar and bookkeeper
projects:
1.ByteBuf.release: Call release directly without handling any exceptions;
2.ReferenceCountUtil.release: return processing status;
3. ReferenceCountUtil.safeRelease: without return value, print exception
information
I don't think it is necessary to revert these PRs. We can discuss what is
the correct way to release, and then modify these three ways into the
correct way.
Hang Chen <chenh...@apache.org> 于2023年1月30日周一 12:28写道：
> Hi guys,
> 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. Do you have any ideas?
>
> 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
>
>
> Thanks,
> Hang
>
