Hi Baodi,

+1
This breaking change looks good to me. We should ensure the Go
client's behavior is as consistent as possible with the Java client's.

Thanks,
Zike Yang

On Thu, Mar 6, 2025 at 11:04 PM Yunze Xu <x...@apache.org> wrote:
>
> Hi Baodi,
>
> There is a similar discussion before:
> https://lists.apache.org/thread/ryn7pq0ctfd5372ww1gsplkplz2sjc0d
>
> Since the PR is ready to change the behavior, let's discuss again to
> make more people know. IMO, it's +1 to me if there is no complaint
> from the community (the replies previous mail list are all +1).
>
> To correct one point, after this PR, configuring
> `StartMessageIDInclusive=true` is still inconsistent with previous
> behavior. For example, given 2 messages 0 and 1,
> - Before: start exclusive
> - Now (StartMessageIDInclusive): start inclusive
> The behaviors are the same only when you seek to a message ID.
>
> Thanks,
> Yunze
>
> On Thu, Mar 6, 2025 at 10:16 PM Baodi Shi <ba...@apache.org> wrote:
> >
> > https://github.com/apache/pulsar-client-go/pull/1340
> >
> > In this PR, I fixed the inconsistency in the Go client regarding the
> > behavior of `HasNext, Next, and receive` after `seek`. However,
> > there's a point of contention due to a breaking change.
> >
> > Refer to comment:
> > https://github.com/apache/pulsar-client-go/pull/1340#discussion_r1978554860
> >
> > Currently, the Go client doesn't consider StartMessageIDInclusive for
> > seek. The current behavior is:
> > ```
> > consumer.seek(msgid-5)
> > consumer.receive()
> > // will receives from msgid-5
> > ```
> >
> > After this PR, user need to set StartMessageIDInclusive=true to
> > maintain the original behavior (which is consistent with the current
> > Java client implementation; you can refer to the test:
> > https://github.com/apache/pulsar/blob/a1f3322ed358ab6841f0d3e43f2afcc54788b887/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/SubscriptionSeekTest.java#L142-L165).
> >
> > In the Java client, StartMessageIDInclusive does affect seek. Perhaps
> > the name of this configuration wasn't ideal initially, but there is a
> > comment in the Java client:
> > https://github.com/apache/pulsar/blob/f199e8805f517373dbcc0f0c4a132218ecc24f0a/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ConsumerBuilder.java#L699
> >
> > Regardless, it changes the existing behavior of the Go client but
> > aligns it with the Java client.
> >
> > I hope the PR gets merged. I've updated the documentation and will add
> > a label to this PR so it can be included in the release notes for the
> > next release to inform users.
> >
> > I would like to hear your suggestions.
> >
> > Thanks,
> > Baodi Shi

Reply via email to