Re: [VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2023-03-15 Thread Yunze Xu
1:01 AM PengHui Li wrote: > > > > > > > +1(binding) > > > > > > > > Thanks, > > > > Penghui > > > > > > > > On Fri, Dec 23, 2022 at 10:31 AM 丛搏 wrote: > > > > > > > > > +1 (non-

Re: [VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2023-01-10 Thread r...@apache.org
> > > > > > Thanks, > > > Penghui > > > > > > On Fri, Dec 23, 2022 at 10:31 AM 丛搏 wrote: > > > > > > > +1 (non-binding) > > > > > > > > Thanks, > > > > Bo > > > > > > > > Yun

Re: [VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2023-01-10 Thread Enrico Olivelli
n Fri, Dec 23, 2022 at 10:31 AM 丛搏 wrote: > > > > > +1 (non-binding) > > > > > > Thanks, > > > Bo > > > > > > Yunze Xu 于2022年12月22日周四 20:34写道: > > > > > > > > Hi all, > > > > > > > > I'm st

Re: [VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2023-01-10 Thread guo jiwei
> Yunze Xu 于2022年12月22日周四 20:34写道: > > > > > > Hi all, > > > > > > I'm starting the VOTE for PIP-229: Add a common interface to get > > > fields of MessageIdData: https://github.com/apache/pulsar/issues/18950 > > > > > > Discussion thread:

Re: [VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-22 Thread PengHui Li
+1(binding) Thanks, Penghui On Fri, Dec 23, 2022 at 10:31 AM 丛搏 wrote: > +1 (non-binding) > > Thanks, > Bo > > Yunze Xu 于2022年12月22日周四 20:34写道: > > > > Hi all, > > > > I'm starting the VOTE for PIP-229: Add a common interface to get > &g

Re: [VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-22 Thread 丛搏
+1 (non-binding) Thanks, Bo Yunze Xu 于2022年12月22日周四 20:34写道: > > Hi all, > > I'm starting the VOTE for PIP-229: Add a common interface to get > fields of MessageIdData: https://github.com/apache/pulsar/issues/18950 > > Discussion thread: > ht

[VOTE] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-22 Thread Yunze Xu
Hi all, I'm starting the VOTE for PIP-229: Add a common interface to get fields of MessageIdData: https://github.com/apache/pulsar/issues/18950 Discussion thread: https://lists.apache.org/thread/25rzflmkfmvxhf3my0ombnbpv7bvgy32 The vote will be open for at least 3 days. Thanks, Yunze

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-20 Thread 丛搏
Hi, Yunze > This interface is introduced mainly because we > don't have a String parameter in seek and acknowledge before. It's a > tradeoff between compatibility and complexity. I think `MessageIdAdv` and adding `topic name` is more confusing for non-partition-topic and redundant. If we don't c

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-19 Thread Yunze Xu
Hi Bo, > But when `topicName` is not in the `PulsarApiMessageId`, they should use > `messageId instanceof TopicMessageId` to get the `topicName` The scenario only happens for `acknowledge` APIs because I don't want to add more overloads in PIP-224. The new `seek` overloads in PIP-224 just accept

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-19 Thread 丛搏
Hi Yunze : I have no reason to must add the `getTopicName()` into the `PulsarApiMessageId` (or named `MessageIdAdv `). I just thought of a scenario that users would use. For the consumer side, they don't know whether the topic is a partitioned topic or a non-partitioned topic. Maybe they don't car

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-18 Thread Yunze Xu
Hi Penghui, It makes sense to me. I updated the proposal and added the API docs to the `MessageIdAdv`, as well as why will `BatchMessageAcker` be deprecated. Thanks, Yunze On Mon, Dec 19, 2022 at 12:57 PM PengHui Li wrote: > > Hi Yunze, > > Thanks for the explanation. Now I understand why the B

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-18 Thread PengHui Li
Hi Yunze, Thanks for the explanation. Now I understand why the BatchMessageAcker will be deprecated. For the interface face. I just thought they were for different users. For common users, they can only use MessageID. For advanced users (Pulsar ecosystem developers, client developers), they can u

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-18 Thread Yunze Xu
Hi Penghui, > It looks like LedgerHandleAdv to LedgerHandle in the bookkeeper. It's different. IMO, LedgerHandleAdv to LedgerHandle, or MetadataStoreExtended to MetadataStoreExtend, are both added just to avoid adding non-default methods to the original interface. But the PulsarApiMessageId here

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-18 Thread Yunze Xu
Hi Bo, For a single-topic consumer, the MessageId implementations returned by receive() should be a MessageIdImpl or BatchedMessageIdImpl. Is there any reason to add a `getTopicName()` method for them (via extending TopicMessageId)? If yes, we have to use the default implementation of `getTopicNam

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-18 Thread PengHui Li
I agree with the motivation. Just some minor suggestions: Is AdvancedMessageId or MessageIdAdv better for this case to replace PulsarApiMessageId? It looks like LedgerHandleAdv to LedgerHandle in the bookkeeper. > We have to deprecated the BatchMessageAcker, which is just a wrapper of a BitSet a

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-17 Thread 丛搏
< For a single-topic consumer, wrapping the topic name is < redundant and might break the existing behavior. In this case, if < `PulsarApiMessageId` extends `TopicMessageId`, the `getTopicName()` < method should return null, which is not a good design [1][2]. For `TopicMessageIdImpl`, it is an ori

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-17 Thread Yunze Xu
Hi Bo, Because the topic name is not a part of MessageIdData. It's only used to find the correct internal consumer of a multi-topics consumer. > All I can think of is PulsarApiMessageId extend TopicMessageId(PIP-224[1]) > right? No. The `TopicMessageId` could only be used in a multi-topics cons

Re: [DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-16 Thread 丛搏
Abstraction based on MessageIdData is a good solution. I don't have any discussion context. Why don't we put the topic name in it? All I can think of is PulsarApiMessageId extend TopicMessageId(PIP-224[1]) right? Thanks, Bo [1] https://github.com/apache/pulsar/issues/18616 Yunze Xu 于2022年12月16日

[DISCUSS] PIP-229: Add a common interface to get fields of MessageIdData

2022-12-15 Thread Yunze Xu
Hi all, I've opened a PIP to discuss: https://github.com/apache/pulsar/issues/18950 Currently the `MessageId` interface is not friendly to developers of Pulsar core and ecosystems. There is no abstraction of the `MessageIdData` defined in `PulsarApi.proto`. This proposal aims at solving this pro

PIP-229: Add a common interface to get fields of MessageIdData

2022-12-15 Thread Yunze Xu
Hi all, I've opened a PIP to discuss: https://github.com/apache/pulsar/issues/18950 Currently the `MessageId` interface is not friendly to developers of Pulsar core and ecosystems. There is no abstraction of the `MessageIdData` defined in `PulsarApi.proto`. This proposal aims at solving this pro