Sent from my iPhone
> On Jan 30, 2023, at 1:21 AM, Shiji Lu <lush...@apache.org> wrote:
>
> Good suggestions,
> 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 think one specification can be used at present, all of which are released
> in the way of ReferenceCountUtil.release, replacing these three ways
These methods may be logically the same. They will not perform the same. Pulsar
is highly performant and able to process millions of messages per second.
Please be sure you understand performance impacts of your proposal under heavy
load.
Best,
Dave
>
>
>
>> On 2023/01/30 08:11:39 Enrico Olivelli wrote:
>> There is an open discussion on the Apache BooKeeper mailing list about
>> this kind of refactoring.
>>
>> I appreciate this initiative because I see it as a tentative to reduce
>> the tech debt.
>>
>> I think that it is pretty dangerous to do this kind of change.
>> Pulsar uses refcounting in a very advanced way and there are many
>> subtle cases that deserve careful handling.
>>
>> I like the motto "Don't break things that aren't broken", this is one
>> of the guiding principles while dealing with big open source projects
>> and communities as Apache Pulsar
>>
>>
>> Enrico
>>
>>> Il giorno lun 30 gen 2023 alle ore 03:28 Yunze Xu
>>> <y...@streamnative.io.invalid> ha scritto:
>>>
>>> I disagree with using `ReferenceCountUtil.safeRelease()` everywhere.
>>> The implementation is throwing any Throwable:
>>>
>>> ```java
>>>
>>> public static void safeRelease(Object msg) {
>>> try {
>>> release(msg);
>>> } catch (Throwable var2) {
>>> logger.warn("Failed to release a message: {}", msg, var2);
>>> }
>>> }
>>> ```
>>>
>>> When `release` throws an exception, it means the logic is wrong. For
>>> example, you have released a ByteBuf whose refcnt is 1 twice. We
>>> should make it clear where fast-fail is not allowed and catch the
>>> exception in these places.
>>>
>>> Thanks,
>>> Yunze
>>>
>>> On Sun, Jan 29, 2023 at 7:57 PM steven lu <lushiji2...@gmail.com> wrote:
>>>>
>>>> [DISCUSS] PIP-244: Refactor ByteBuf release method
>>>> Hello everyone. I hope you guys are all doing well. I would like to start
>>>> the discussion for PIP-244 https://github.com/apache/pulsar/issues/19350,
>>>> Please let me know if you have any concerns or questions.
>>>> ------- Paste original PIP content to help quote ------
>>>>
>>>> ### Motivation
>>>>
>>>> It may throw an exception when release a `ByteBuf` object. so the exception
>>>> in `ByteBuf.release` should be checked.
>>>>
>>>> ### Goal
>>>>
>>>> Use `ReferenceCountUtil.safeRelease()` instead of `ByteBuf.release()` in
>>>> all Pulsar's classes.
>>>>
>>>> ### API Changes
>>>>
>>>> _No response_
>>>>
>>>> ### Implementation
>>>>
>>>> 1. Use `ReferenceCountUtil.safeRelease()` instead of `ByteBuf.release()`.
>>
