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
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()`.
>
