Re: [DISCUSS] PIP-279: Reformat property in generateResponseWithEntry

[Shiji Lu]

the DISCUSS is passed,I have send the VOTE for this PIP
https://lists.apache.org/thread/g354684m9h495o3p0kmzb7fh7vfxhddx


On 2023/06/21 03:22:52 steven lu wrote:
> # Motivation
> 
> reformat property,for a http header name cannot contain the following
> prohibited characters: =,;: \t\r\n\v\f
> 
> for example:
> {"city=shanghai":"tag"}
> when we run `bin/pulsar-admin topics get-message-by-id `, it will
> throw exception, the exception is:
> `Reason: java.util.concurrent.CompletionException:
> org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector$RetryException:
> Could not complete the operation. Number of retries has been
> exhausted. Failed reason: a header name cannot contain the following
> prohibited characters: =,;: \t\r\n\v\f: =`
> 
>  src="https://github.com/StevenLuMT/pulsar/assets/42990025/973d95b9-4ac2-4977-b160-162c4b53a613;>
> 
> # High Level Design
> 
> In master branch,
> in an http 
> request:getMessageById("/{tenant}/{namespace}/{topic}/ledger/{ledgerId}/entry/{entryId}"),
> replace `"X-Pulsar-PROPERTY-" + msgProperties.getKey()` with
> `"X-Pulsar-PROPERTY"`
> 
> After release-3.1.0, this feature begins to take effect.
> 

Re: [DISCUSS] PIP-244: Refactor ByteBuf release method

[Shiji Lu]

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