pulsar-client-cpp icon indicating copy to clipboard operation
pulsar-client-cpp copied to clipboard

Fix broken main branch by providing correct getBitSet implementation

Open BewareMyPower opened this issue 2 years ago • 1 comments

Motivation

Currently the main branch is broken by the concurrent merge of https://github.com/apache/pulsar-client-cpp/pull/153 and https://github.com/apache/pulsar-client-cpp/pull/151. It's because when a batched message id is constructed from deserialization, there is no getBitSet implementation of the internal acker.

Modifications

Add a bool parameter to MessageIdImpl::getBitSet to indicate whether the message ID is batched. The logic is similar with

https://github.com/apache/pulsar/blob/299bd70fdfa023768e94a8ee4347d39337b6cbd4/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java#L325-L327

and

https://github.com/apache/pulsar/blob/299bd70fdfa023768e94a8ee4347d39337b6cbd4/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java#L345-L347

Add a testMessageIdFromBuild to test the acknowledgment for a message ID without an acker could succeed for a consumer that enables batch index ACK.

TODO

In future, https://github.com/apache/pulsar/pull/19031 might be migrated into the C++ client to fix the consumer that disables batch index ACK.

Documentation

  • [ ] doc-required (Your PR needs to update docs and you will update later)

  • [x] doc-not-needed (Please explain why)

  • [ ] doc (Your PR contains doc changes)

  • [ ] doc-complete (Docs have been already added)

BewareMyPower avatar Jan 17 '23 11:01 BewareMyPower

I have a question:

It's because when a batched message id is constructed from deserialization, there is no getBitSet implementation of the internal acker.

Is the code related to this passage here?

https://github.com/apache/pulsar-client-cpp/blob/d03ff20a96de695a6c2837d62862569faf11876b/lib/MessageIdBuilder.cc#L45-L47

Is setting bitSize equal to 0 for lazy initialization? To save memory?

If bitSite is initialized here, subsequent operations such as ack can reuse the logic here. Wouldn't it be better?

https://github.com/apache/pulsar-client-cpp/blob/f860566c05634dd097d8941ea38a2a939fcfd26e/lib/BatchedMessageIdImpl.h#L38-L40

shibd avatar Jan 17 '23 15:01 shibd