Penghui,

Il giorno ven 21 ott 2022 alle ore 02:04 PengHui Li
<peng...@apache.org> ha scritto:
>
> > while (true) {
>     Messages<String> messages = consumer.batchReceive();
>     Message<String> finalMessage = null;
>     for (Message<String> message : messages) {
>         process(message);
>         finalMessage = message;
>     }
>     if (finalMessage != null) {
>         consumer.acknowledgeCumulativeAsync(finalMessage);
>     }
> }
> ```
>
> I think this could be a good idea for improving the user's experience of
> cumulative ack.
> For one batch receive operation, users can only receive messages from one
> partition/topic.
> The cumulative ack becomes more assured and understandable.

I can agree on this idea, but we must state it cleanly in the javadocs
and we cannot change the behaviour in the future.

This proposal is backward compatible so we could do it but if in the
future we want to go back to
2.10-like behaviour it won't be possible anymore.

It is an important choice, because we are going to change the meaning
of an existing
API.

The very best way is to have a new method, this way no one will have
surprises, no and in the future.


>
> For the performance issue. The worst case is degenerate into the single
> message receiving.
> Users are mostly using the single message receiving for now.
>
> Regarding the issue of batch receive itself, I think we should discuss it
> separately since currently, we are
> split the batch first and then create a new list for users with the split
> messages. This can be improved
> if we do some optimization for this part.

Sure

Enrico

>
> Thanks,
> Penghui
>
> On Thu, Oct 20, 2022 at 7:14 PM 丛搏 <bog...@apache.org> wrote:
>
> > Hi, Enrico
> > I think we temporarily don't add the new API, it will make the users
> > very hard to use. we can try to improve the performance first.
> >
> > the performance can improve:
> > 1. create multi consumer clients to consume the multi-partition topic.
> > 2. change the MultiTopicsConsumerImpl incoming queue, using
> > ConsumerImpl.batchReceive() directly when use
> > MultiTopicsConsumerImpl.batchReceive()
> >
> > Thanks,
> > bo
> >
> > Enrico Olivelli <eolive...@gmail.com> 于2022年10月20日周四 17:37写道:
> > >
> > > Il giorno gio 20 ott 2022 alle ore 11:09 丛搏 <bog...@apache.org> ha
> > scritto:
> > > >
> > > > Hi,
> > > >
> > > > I have another solution, we only guarantee the invoke batch receive
> > > > returns the messages are the same topic message.
> > >
> > > This is going to reduce performances of applications that consume from
> > > a partitioned topic or using a pattern or multiple topics, the
> > > application will receive smaller batches.
> > > Also, in the current API you don't have this assumption.
> > >
> > > I think that if we want to go this way we have to add a new
> > > batchReceive API that guarantees that all the messages are from the
> > > same message
> > >
> > > batchReceiveSingleTopic()
> > >
> > > Enrico
> > >
> > >
> > > In this way, we don't
> > > > need to add the new API for ack. use example is:
> > > > ```
> > > > // consumer the multi-partition messages
> > > > while (true) {
> > > >     Messages<String> messages = consumer.batchReceive();
> > > >     Message<String> finalMessage = null;
> > > >     for (Message<String> message : messages) {
> > > >         process(message);
> > > >         finalMessage = message;
> > > >     }
> > > >     if (finalMessage != null) {
> > > >         consumer.acknowledgeCumulativeAsync(finalMessage);
> > > >     }
> > > > }
> > > > ```
> > > > implementation pr: https://github.com/congbobo184/pulsar/pull/5
> > > >
> > > > Thanks,
> > > > bo
> > > >
> > > > Tarun Annapareddy <tarunannapareddy1...@gmail.com> 于2022年9月14日周三
> > 17:46写道:
> > > > >
> > > > > Hi Enrico,
> > > > >        I can understand the concern about the validation of the topic
> > > > > string key in the map we are using. In this case is it good to have
> > > > > Map<String, Message> topicToMessage? It will help us to validate
> > that the
> > > > > message truly belongs to the topic/partition.
> > > > >
> > > > > Thank you,
> > > > > Tarun.
> > > > >
> > > > > On Wed, 14 Sept 2022 at 07:50, Yunze Xu <y...@streamnative.io.invalid
> > >
> > > > > wrote:
> > > > >
> > > > > > Hi Enrico,
> > > > > >
> > > > > > > There is no way to confirm that the MessageId really belongs to
> > the
> > > > > > > partition and you can pass whatever you want
> > > > > >
> > > > > > True. But still, the existing `acknowledgeCumulative` API is not
> > > > > > convenient to use. The original purpose for this new overload is
> > > > > > allowing users to maintain each partition's latest message ID to
> > avoid
> > > > > > frequent ACK commands, then they can flush all these message IDs
> > once.
> > > > > >
> > > > > > Though the keys are actually not used, a natural way is to
> > maintain a
> > > > > > map, not a list or set. For a multi-topics consumer, we can also
> > > > > > add a constraint that the key must exist in an internal consumer.
> > For
> > > > > > a single topic consumer, the key must be the topic name.
> > > > > >
> > > > > > Without the new API, users have to configure
> > `acknowledgmentGroupTime`
> > > > > > and call `acknowledgeCumulative` for each message received.
> > However,
> > > > > > the ACK grouping tracker is very simple that it can only configure
> > the
> > > > > > grouping timeout. I think users need a way to control precisely
> > for when
> > > > > > to send the ACK commands.
> > > > > >
> > > > > > In my design, `acknowledgeCumulative` that accepts a single
> > message ID
> > > > > > will go through the ACK grouping tracker, while
> > > > > > `acknowledgeCumulative` that accepts a map should send ACK commands
> > > > > > immediately because it represents users cache the pending message
> > IDs
> > > > > > by themselves.
> > > > > >
> > > > > > Thanks,
> > > > > > Yunze
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > On Sep 13, 2022, at 18:36, Enrico Olivelli <eolive...@gmail.com>
> > wrote:
> > > > > > >
> > > > > > > -1
> > > > > > > I am not sure this is much useful in this form.
> > > > > > > I had also commented on the PR: We pass a Map<String, MessageId>
> > but
> > > > > > > actually the keys of the map will not be used.
> > > > > > >
> > > > > > > I understand that the current API is not user-friendly, but I
> > don't
> > > > > > > think that adding a dummy parameter to the API will really
> > > > > > > help.
> > > > > > > There is no way to confirm that the MessageId really belongs to
> > the
> > > > > > > partition and you can pass whatever you want
> > > > > > >
> > > > > > > Enrico
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Il giorno mar 13 set 2022 alle ore 12:24 Xiangying Meng
> > > > > > > <xiangy...@apache.org> ha scritto:
> > > > > > >>
> > > > > > >> +1 (non-binding)
> > > > > > >> Good work!
> > > > > > >>
> > > > > > >> Sincerely,
> > > > > > >> Xiangying
> > > > > > >>
> > > > > > >> On Tue, Sep 13, 2022 at 6:15 PM Tarun Annapareddy <
> > > > > > >> tarunannapareddy1...@gmail.com> wrote:
> > > > > > >>
> > > > > > >>> Hi devs,
> > > > > > >>>    This is the official thread to vote for Java Client support
> > to
> > > > > > >>> Cumulative Acknowledge messages for multiple partitions or
> > topics
> > > > > > >>>
> > > > > > >>> PIP Issue: https://github.com/apache/pulsar/issues/17574
> > > > > > >>> Discussion thread:
> > > > > > >>>
> > https://lists.apache.org/thread/k090ftlqc149yr1cnprxb29vxg160131
> > > > > > >>> PR: https://github.com/apache/pulsar/pull/17577
> > > > > > >>>
> > > > > > >>> Thank you,
> > > > > > >>> Tarun.
> > > > > > >>>
> > > > > >
> > > > > >
> >

Reply via email to