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 care about it. But when `topicName` is not in the `PulsarApiMessageId`, they should use `messageId instanceof TopicMessageId` to get the `topicName`, if this `topicName` is in the `PulsarApiMessageId`, they only use the unified logic to get the field from the messageId `PulsarApiMessageId pulsarApiMessageId = (PulsarApiMessageId) messageId`
These are just some of my views, and it will not block this PIP. Thanks, Bo Yunze Xu <y...@streamnative.io.invalid> 于2022年12月19日周一 10:41写道： > > 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 > `getTopicName()` that returns null without modifying any existing > code. If you don't want to return a null value, you have to add new > logic that sets the topic explicitly when creating the received > message. > > Thanks, > Yunze > > On Sun, Dec 18, 2022 at 3:44 PM 丛搏 <bog...@apache.org> wrote: > > > > < 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 original method. for > > `PulsarApiMessageId` if extend `TopicMessageId` it is a new method for > > any `MessageId` extend `PulsarApiMessageId`, why do we have to return > > null? I think it just reduces the transmission of useless fields at > > the network layer and not added to MessageIdData. LedgerId and EntryId > > are in PulsarApiMessageId, why shouldn't `topicName` be added in? > > > > Thanks, > > Bo > > > > Yunze Xu <y...@streamnative.io.invalid> 于2022年12月18日周日 14:23写道： > > > > > > 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 > > > consumer. 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]. > > > > > > After both PIP-224 and PIP-229 are approved, the `TopicMessageIdImpl` > > > will implement both `PulsarApiMessageId` and `TopicMessageId` > > > interfaces. Other `MessageId` implementations only need to implement > > > `PulsarApiMessageId`. > > > > > > BTW, PIP-224 mainly solves two problems: > > > 1. When a multi-topics consumer acknowledges a `MessageId` that is not > > > a `TopicMessageId`, a `PulsarClientException.NotAllowedException` will > > > be thrown in synchronous methods. The asynchronous methods should not > > > throw an exception. > > > 2. For a multi-topics consumer, support seeking with a `TopicMessageId`. > > > > > > PIP-224 is designed for application users to specify an associated > > > topic name when using a `MessageId` in `seek` or `acknowledge` on a > > > multi-topics consumer. PIP-229 is more like a refactoring to allow the > > > experienced developers access the fields of `MessageIdData` via a > > > standard interface. > > > > > > [1] https://github.com/apache/pulsar/issues/18616#issuecomment-1328609346 > > > [2] https://lists.apache.org/thread/g8o0qtljllxnvck69dn36205xg5xr8cc > > > > > > Thanks, > > > Yunze > > > > > > > > > On Fri, Dec 16, 2022 at 8:50 PM 丛搏 <bog...@apache.org> wrote: > > > > > > > > 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 <y...@streamnative.io.invalid> 于2022年12月16日周五 15:59写道： > > > > > > > > > > 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 problem and allows more loose type > > > > > assertions when using `seek` and `acknowledge`. > > > > > > > > > > You can also see the demo for reference: > > > > > https://github.com/BewareMyPower/pulsar/pull/11 > > > > > > > > > > (Sorry I forgot to add the [DISCUSS] prefix again in the previous > > > > > email, let's continue the discussion here) > > > > > > > > > > Thanks, > > > > > Yunze
