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
