Make sense. +1
> 2022年11月29日 22:50，Yunze Xu <y...@streamnative.io.INVALID> 写道： > > Hi Baodi, > >> `negativeAcknowledge` also needs to add the same checks as the new >> acknowledge interface > > Good suggestion. I will add it. > >> This can users to clearly associate the use cases of MultiTopicConsumer and >> TopicMessageId. > > IMO. Users should not care about the MultiTopicsConsumer and > TopicMessageId when using `receive` and `acknowledge` APIs. We only > need to guarantee the following code works. > > ```java > var msg = consumer.receive(); > consumer.acknowledge(msg.getMessageId()); > ``` > > Currently, `acknowledge` works well but there is no standard to handle > the exceptional case, e.g. a multi-topics consumer tries to > acknowledge a message id from a single-topic consumer. This proposal > makes the behavior clear that > PulsarClientException.NotAllowedException will be thrown in this case. > In addition, for these users, they can use `TopicMessageId#create` > after this proposal. > > ```java > var msg = singleTopicConsumer.receive(); > multiTopicsConsumer.acknowledge(TopicMessageId.create(msg.getMessageId())); > ``` > > However, this solution is just for experienced Pulsar developers. They > don't need to use `TopicMessageIdImpl` anymore. But for normal users, > adding overloads could make it more complicated. They have to know > what the TopicMessageId is when using the `acknowledge` methods. > > Thanks, > Yunze > > On Tue, Nov 29, 2022 at 10:12 PM Baodi Shi <baodi....@icloud.com.invalid> > wrote: >> >> Hi, Yunze: >> >> Thanks for your proposal. That Looks good to me. >> >> `negativeAcknowledge` also needs to add the same checks as the new >> acknowledge interface. >> >>> This interface doesn't add any acknowledge overload because the overloads >>> are already too many. But it will make the behavior clear. >> I think since we exposed the TopicMessageId, it would be better to add >> overloaded interfaces (even if the overloads are a lot). This can users to >> clearly associate the use cases of MultiTopicConsumer and TopicMessageId. >> >> Also, while it's okay to use TopicMessageId param on a single consumer, I >> guess we shouldn't allow users to use it. >> >> In this way, users are clearly aware that TopicMessageId is used when using >> MultiTopicConsumer and MessageId is used when using >> SingleTopicConsumer.(Maybe it's not a good idea) >> >> >> Thanks, >> Baodi Shi >> >>> 2022年11月29日 15:57，Yunze Xu <y...@streamnative.io.INVALID> 写道： >>> >>>> Is there a case where the user uses the messageId returned by the >>> producer to seek in the consumer? Is this a good behavior? >>> >>> Yes. I think it should be acceptable. To correct my previous point, >>> now I think the MessageId returned by send should also be able to be >>> applied for seek or acknowledge. >>> >>>> even with the >>> current proposal, it may return null when getting the topic from >>> TopicMessageId for backward compatibility. >>> >>> No. It may return null just because Java doesn't allow a non-null >>> returned value. The internal implementations of >>> TopicMessageId#getOwerTopic should return a non-null topic name to >>> avoid null check. >>> >>> When I mentioned **the implementation of getTopicName() must return >>> null**, the assumption is that MessageId#toByteArray serializes the >>> topic name if adding the `getTopicName()` method. However, in this >>> proposal, `TopicMessageId#toByteArray` won't. See the implementation >>> of `TopicMessageId#create`. It's only a wrapper for an arbitrary >>> MessageId implementation. >>> >>> Thanks, >>> Yunze >>> >>> On Tue, Nov 29, 2022 at 2:47 PM Zike Yang <z...@apache.org> wrote: >>>> >>>> Hi Yunze, >>>> >>>> Thanks for your proposal. Quoted from your GitHub comments[0]: >>>> >>>>> There is also a case when MessageId is returned from Producer#send. In >>>>> this case, the returned MessageId should only used for serialization >>>> >>>> Is there a case where the user uses the messageId returned by the >>>> producer to seek in the consumer? Is this a good behavior? >>>> >>>>> If we added the method directly to MessageId, to keep the backward >>>>> compatibility, the implementation of getTopicName() must return null, >>>>> which is not a good design. >>>> >>>> I think it's a trade-off. If I understand correctly, even with the >>>> current proposal, it may return null when getting the topic from >>>> TopicMessageId for backward compatibility. The current >>>> TopicMessageIdImpl doesn't serialize the topic information. >>>> >>>> >>>> [0] https://github.com/apache/pulsar/issues/18616#issuecomment-1328609346 >>>> >>>> Thanks, >>>> Zike Yang >>>> >>>> On Mon, Nov 28, 2022 at 12:22 PM Yunze Xu <y...@streamnative.io.invalid> >>>> wrote: >>>>> >>>>> Hi all, >>>>> >>>>> I've opened a PIP to discuss: >>>>> https://github.com/apache/pulsar/issues/18616. >>>>> >>>>> The consumer's MessageId related APIs have some hidden requirements >>>>> and flakiness and some behaviors are not documented well. This >>>>> proposal will introduce a TopicMessageId interface that exposes a >>>>> method to get a message's owner topic. >>>>> >>>>> P.S. There was an email [1] that didn't add the "[DISCUSS]" label, >>>>> which might be a little confusing. So I sent the email again for >>>>> discussion. Please do not reply to the previous email. >>>>> >>>>> [1] https://lists.apache.org/thread/6gj16pmrjk6ncsd30xrl20pr5ng6t61o >>>>> >>>>> Thanks, >>>>> Yunze >>
