Skip to content

[fix] [broker] make rest api PersistentTopic#getLastMessageId consistent with binary api ServerCnx#handleGetLastMessageId#21967

Closed
thetumbled wants to merge 4 commits intoapache:masterfrom
thetumbled:Fix_MakeRestApiConsistent
Closed

[fix] [broker] make rest api PersistentTopic#getLastMessageId consistent with binary api ServerCnx#handleGetLastMessageId#21967
thetumbled wants to merge 4 commits intoapache:masterfrom
thetumbled:Fix_MakeRestApiConsistent

Conversation

@thetumbled
Copy link
Copy Markdown
Member

@thetumbled thetumbled commented Jan 25, 2024

Fixes #21968

Motivation

The implementation of org.apache.pulsar.broker.service.persistent.PersistentTopic#getLastMessageId is not consistent with org.apache.pulsar.broker.service.ServerCnx#handleGetLastMessageId, for example, ServerCnx#handleGetLastMessageId may get last message id from the compacted topic, while PersistentTopic#getLastMessageId don't.

And flink-pulsar-connector also rely on the rest api PersistentTopic#getLastMessageId, we have better fix this problem.
Older version of flink-pulsar-connector use it, latest version change to call consumer.getLastMessageId();.

Modifications

Refactor PersistentTopic#getLastMessageId to be simillar with ServerCnx#handleGetLastMessageId.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

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: thetumbled#37

@thetumbled
Copy link
Copy Markdown
Member Author

As #21969 could retrieve the last message id from the compacted topic when consumer is in read compact mode, i wonder should we add a param readCompact to simulate this behavior, or we can just let these two api inconsistent?
@coderzc @Technoboy- @codelipenghui @liangyepianzhou @poorbarcode

@thetumbled thetumbled closed this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [broker] incorrect last message id from rest api

1 participant