Skip to content

Conversation

@shibd
Copy link
Member

@shibd shibd commented Oct 7, 2022

Motivation

#90

Modifications

  • Add acknowledge message id list API.

Documentation

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

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

Matching PR in forked repository

shibd#3

@shibd
Copy link
Member Author

shibd commented Nov 1, 2022

@BewareMyPower
Copy link
Contributor

Overall LGTM except the comments I've mentioned.

@BewareMyPower BewareMyPower merged commit 2980bbe into apache:main Nov 2, 2022
@RobertIndie RobertIndie added this to the 3.1.0 milestone Nov 24, 2022
BewareMyPower referenced this pull request in BewareMyPower/pulsar-client-cpp Nov 29, 2022
…meMs is 0

### Motivation

When `ackGroupingTimeMs` is 0, `AckGroupingTrackerDisabled` will be
created to acknowledge messages immediately. However, since the
`addAcknowledgeList` method is not implemented in #23, it doesn't work.

### Modifications

Implement `AckGroupingTrackerDisabled::addAcknowledgeList`. Move the
protocol version validation from `AckGroupingTrackerEnabled` to the base
class `AckGroupingTracker` so that the `doImmediateAck` overload that
accepts `std::set<MessageId>` can be reused.

Add testing params to `testAckMsgList` and
`testAckMsgListWithMultiConsumer` so that various `ackGroupingTimeMs`
can be verified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants