Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][client] PIP-224: Add getLastMessageIds API #20040

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Apr 7, 2023

Master Issue: #18616

Fixes #4940

NOTE: This implementation is different from the original design of PIP-224 that the method name is getLastMessageIds instead of getLastTopicMessageId.

Motivation

When a multi-topics consumer calls getLastMessageId, a MultiMessageIdImpl instance will be returned. It contains a map of the topic name and the latest message id of the topic. However, the MultiMessageIdImpl cannot be used in any place of the API that accepts a MessageId because all methods of the MessageId interface are not implemented, including compareTo and toByteArray.

Therefore, users cannot do anything on such a MessageId implementation except casting MessageId to MultiMessageIdImpl and get the internal map.

Modifications

  • Add the getLastMessageIds methods to Consumer. It returns a list of TopicMessageId instances, each of them represents the last message id of the owner topic.
  • Mark the getLastMessageId APIs as deprecated.

Verifications

  • Modify the TopicsConsumerImplTest#testGetLastMessageId to test the getLastMessageIds for a multi-topics consumer.
  • Modify the TopicReaderTest#testHasMessageAvailable to test the getLastMessageIds for a single topic consumer.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: BewareMyPower#25

…LastMessageId

Master Issue: apache#18616

Fixes apache#4940

NOTE: This implementation is different from the original design of
PIP-224 that the method name is `getLastMessageIds` instead of
`getLastTopicMessageId`.

### Motivation

When a multi-topics consumer calls `getLastMessageId`, a
`MultiMessageIdImpl` instance will be returned. It contains a map of the
topic name and the latest message id of the topic. However, the
`MultiMessageIdImpl` cannot be used in any place of the API that accepts
a `MessageId` because all methods of the `MessageId` interface are not
implemented, including `compareTo` and `toByteArray`.

Therefore, users cannot do anything on such a `MessageId` implementation
except casting `MessageId` to `MultiMessageIdImpl` and get the internal
map.

### Modifications

- Throw an exception when calling `getLastMessageId` on a multi-topics
  consumer instead of returning a `MultiMessageIdImpl`.
- Remove the `MultiMessageIdImpl` implementation and its related tests.
- Add the `getLastMessageIds` methods to `Consumer`. It returns a list
  of `TopicMessageId` instances, each of them represents the last
  message id of the owner topic.
- Mark the `getLastMessageId` API as deprecated.

### Verifications

- Modify the `TopicsConsumerImplTest#testGetLastMessageId` to test the
  `getLastMessageIds` for a multi-topics consumer.
- Modify the `TopicReaderTest#testHasMessageAvailable` to test the
  `getLastMessageIds` for a single topic consumer.
@BewareMyPower BewareMyPower added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/client labels Apr 7, 2023
@BewareMyPower BewareMyPower added this to the 3.0.0 milestone Apr 7, 2023
@BewareMyPower BewareMyPower self-assigned this Apr 7, 2023
@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Apr 7, 2023
@BewareMyPower BewareMyPower force-pushed the bewaremypower/pip-224-get-last-msg-id branch 2 times, most recently from b6e118a to fd129bb Compare April 10, 2023 13:08
@BewareMyPower BewareMyPower force-pushed the bewaremypower/pip-224-get-last-msg-id branch from fd129bb to 0960047 Compare April 10, 2023 13:09
Co-authored-by: Baodi Shi <[email protected]>
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@BewareMyPower BewareMyPower merged commit a918651 into apache:master Apr 11, 2023
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Apr 11, 2023
@BewareMyPower BewareMyPower deleted the bewaremypower/pip-224-get-last-msg-id branch March 5, 2024 05:59
RobertIndie pushed a commit to apache/pulsar-client-go that referenced this pull request May 28, 2024
### Motivation
To keep consistent with the Java client.

Releted PR: apache/pulsar#20040

### Modifications
- Add `GetLastMessageIDs`api for consumer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. ready-to-test type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Serialization and Deserialization for MultiMessageIdImpl
4 participants