You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Yunze Xu <yz...@streamnative.io.INVALID> on 2023/01/20 02:26:52 UTC

Re: [VOTE] PIP-224: Introduce TopicMessageId for consumer's MessageId related APIs

Hi guys,

Please help review https://github.com/apache/pulsar/pull/19158, which
has been pending for a while. I see 2.12.0 (or 3.0.0?) is coming, I
hope we can include this PIP in the next major release.

Thanks,
Yunze

On Wed, Dec 28, 2022 at 10:40 PM Yunze Xu <yz...@streamnative.io> wrote:
>
> Hi Enrico,
>
> Yeah, I've discussed it privately with Hang and he had the same
> concern with you and. So I changed the `Schema<TopicMessageId>` to a
> more simple class:
>
> ```java
>
> class TopicMessageIdSerDes {
>     public static byte[] serialize(TopicMessageId topicMessageId) {/* ... */}
>     public static TopicMessageId deserialize(byte[] bytes) {/* ... */}
> }
> ```
>
> You can see it and the updated Alternatives section in
> https://github.com/apache/pulsar/issues/18616
>
> Thanks,
> Yunze
>
> On Wed, Dec 28, 2022 at 4:22 PM Enrico Olivelli <eo...@gmail.com> wrote:
> >
> > Schema<TopicMessageId> doesn't make much sense to me.
> >
> > Schema is a Pulsar Client API to deal with Message serialisation, it
> > is not a general purpose serde framework.
> >
> > I still approve the PIP overall but I don't think we should expose
> > such things to the user facing APIs
> >
> > Il giorno mer 28 dic 2022 alle ore 09:15 Hang Chen
> > <ch...@apache.org> ha scritto:
> > >
> > > +1 (binding)
> > >
> > > Thanks,
> > > Hang
> > >
> > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年12月28日周三 11:53写道:
> > > >
> > > > After discussing with @hangc0276 offline, I decided to add a
> > > > `Schema<TopicMessageId>` implementation in this proposal to serialize
> > > > both the owner topic and the base `MessageId`. You can see the latest
> > > > update on GitHub. If any of you have any concern, feel free to let me
> > > > know.
> > > >
> > > > Thanks,
> > > > Yunze
> > > >
> > > > On Mon, Dec 26, 2022 at 1:22 AM Enrico Olivelli <eo...@gmail.com> wrote:
> > > > >
> > > > > +1 (binding)
> > > > >
> > > > > Enrico
> > > > >
> > > > > Il Ven 23 Dic 2022, 05:46 Yunze Xu <yz...@streamnative.io.invalid> ha
> > > > > scritto:
> > > > >
> > > > > > It needs the 3rd binding +1 yet. Could anyone else take a look?
> > > > > >
> > > > > > Thanks,
> > > > > > Yunze
> > > > > >
> > > > > > On Wed, Dec 21, 2022 at 3:21 PM 丛搏 <bo...@apache.org> wrote:
> > > > > > >
> > > > > > > Hi Yunze,
> > > > > > >
> > > > > > > add `TopicMessageId ` will couple messageID and `topic name` together,
> > > > > > > which is very unclear for non-partition-topic.
> > > > > > >
> > > > > > > ```
> > > > > > > void seek(String topicName, MessageId messageId) throws
> > > > > > PulsarClientException;
> > > > > > > List<Map<String, MessageId>> getLastTopicMessageId() throws
> > > > > > > PulsarClientException;
> > > > > > > ```
> > > > > > > If the interface is designed in this way, it may be simpler, easier to
> > > > > > > understand, and more intuitive for users, and MessageID will not be
> > > > > > > coupled with TopicName.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Bo
> > > > > > >
> > > > > > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年12月16日周五 15:31写道:
> > > > > > > >
> > > > > > > > Yeah, it's an implementation detail and I will keep the same semantics
> > > > > > > > with the latest master when I push my PR.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Yunze
> > > > > > > >
> > > > > > > > On Fri, Dec 16, 2022 at 3:03 PM 丛搏 <bo...@apache.org> wrote:
> > > > > > > > >
> > > > > > > > > if you don't change this in PIP-229 or PIP-224, I will create a new
> > > > > > > > > PIP to handle the `BatchMessageIdImpl` and `MessageIdImpl`
> > > > > > > > > `compareTo()` method, now I have no problem with this PIP
> > > > > > > > > +1 (non-binding)
> > > > > > > > > Sorry to bother this PIP vote.
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Bo
> > > > > > > > >
> > > > > > > > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年12月16日周五 11:58写道:
> > > > > > > > > >
> > > > > > > > > > If this breaking change can pass the PMC votes, I will keep the new
> > > > > > > > > > semantics in PIP-229. Otherwise, it would not make sense to adopt
> > > > > > the
> > > > > > > > > > new semantics in PIP-229.
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Yunze
> > > > > > > > > >
> > > > > > > > > > On Fri, Dec 16, 2022 at 11:46 AM Yunze Xu <yz...@streamnative.io>
> > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > I cannot find any confusing code from the proposal itself. Could
> > > > > > you
> > > > > > > > > > > point it out? If you are mentioning the `legacyCompare` and
> > > > > > `compare`
> > > > > > > > > > > methods in #18890 [1], it's not related to this proposal. And I
> > > > > > have
> > > > > > > > > > > opened PIP-229 [2] for discussion.
> > > > > > > > > > >
> > > > > > > > > > > BTW, the PIP-229 itself doesn't mention the compare logic. But
> > > > > > I'm not
> > > > > > > > > > > going to adopt the new semantics because it's actually a breaking
> > > > > > > > > > > change, just as I've replied. You might think it's a bug, but
> > > > > > it's a
> > > > > > > > > > > public API. Any change of the semantics in the public API is a
> > > > > > > > > > > breaking change.
> > > > > > > > > > >
> > > > > > > > > > > [1] https://github.com/apache/pulsar/pull/18890/files
> > > > > > > > > > > [2]
> > > > > > https://lists.apache.org/thread/x52zpwlo8pxzp81oxllh5vw82kyrzgpk
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Dec 16, 2022 at 11:34 AM 丛搏 <co...@gmail.com>
> > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Although unrelated, it adds a lot of confusing code.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Bo
> > > > > > > > > > > >
> > > > > > > > > > > > Yunze Xu <yz...@streamnative.io.invalid> 于2022年12月16日周五
> > > > > > 08:05写道:
> > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > This proposal is not related to the comparison logic between
> > > > > > > > > > > > > BatchMessageIdImpl and MessageIdImpl.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Yunze
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, Dec 15, 2022 at 12:58 PM 丛搏 <bo...@apache.org>
> > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > -1 (non-binding)
> > > > > > > > > > > > > > sorry, I have one question about the BatchMessageId
> > > > > > compareTo()
> > > > > > > > > > > > > > method. the discussion mail :
> > > > > > > > > > > > > >
> > > > > > https://lists.apache.org/thread/8n3oyk2hdsskkotnj4lnlvfnndctpqbg.
> > > > > > > > > > > > > > I hope it can be this issue can be discussed clearly.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I hope it can be this issue can be discussed clearly. I
> > > > > > will retry to
> > > > > > > > > > > > > > vote until this issue clearly :
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > Bo
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 丛搏 <co...@gmail.com> 于2022年12月14日周三 22:56写道:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > +1 (non-binding)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > Bo
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > PengHui Li <pe...@apache.org> 于2022年12月14日周三 19:12写道:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > +1 (binding)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > - Penghui
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Sun, Dec 11, 2022 at 6:36 AM Enrico Olivelli <
> > > > > > eolivelli@gmail.com> wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > +1 (binding)
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Enrico
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Il Ven 9 Dic 2022, 10:41 Jiaqi Shen <
> > > > > > gleiphir2769@gmail.com> ha scritto:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > +1(non-binding)
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > > > Jiaqi Shen
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > <ma...@gmail.com> 于2022年12月5日周一 15:23写道:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > +1(non-binding)
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > > > > Mattison
> > > > > > > > > > > > > > > > > > > On Dec 5, 2022, 15:09 +0800, Zike Yang <
> > > > > > zike@apache.org>, wrote:
> > > > > > > > > > > > > > > > > > > > +1(non-binding)
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > > > > > Zike Yang
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > On Mon, Dec 5, 2022 at 2:41 PM Baodi Shi
> > > > > > > > > > > > > > > > > <baodi.shi@icloud.com.invalid
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > +1(non-binding)
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > > > > > > Baodi Shi
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > 2022年12月5日 12:51,Yunze Xu
> > > > > > <yz...@streamnative.io.INVALID> 写道:
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Hi all,
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > I'm starting the vote for PIP-224:
> > > > > > Introduce TopicMessageId for
> > > > > > > > > > > > > > > > > > > > > > > consumer's MessageId related APIs:
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > https://github.com/apache/pulsar/issues/18616
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Here is the discussion thread:
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > https://lists.apache.org/thread/jhqy65cdyxzmmxnfsjm8rv9pbk76noxy
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > The vote will be open for at least 3
> > > > > > days.
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > > > > > > > > Yunze
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > >