Skip to content
New issue

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

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

Already on GitHub? # to your account

[improve][admin] getMessageById gets message by batch index #21813

Closed
wants to merge 1 commit into from

Conversation

nodece
Copy link
Member

@nodece nodece commented Dec 28, 2023

Motivation

When the message is a batch message, the org.apache.pulsar.client.admin.Topics#getMessageByIdAsync(java.lang.String, long, long) only returns the first message from the batch message, this affects our troubleshooting.

Modifications

  • Add org.apache.pulsar.client.admin.Topics#getMessageByIdAsync(java.lang.String, long, long, int) instead of org.apache.pulsar.client.admin.Topics#getMessageByIdAsync(java.lang.String, long, long).
  • Add org.apache.pulsar.client.admin.Topics#getMessageById(java.lang.String, long, long, int) instead of org.apache.pulsar.client.admin.Topics#getMessageById(java.lang.String, long, long).
  • Update org.apache.pulsar.admin.cli.CmdPersistentTopics.GetMessageById#run to print all message.
  • Update org.apache.pulsar.admin.cli.CmdTopics.GetMessageById#run to print all message.

Verifying this change

Test added.

Documentation

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

Matching PR in forked repository

PR in forked repository:

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 28, 2023
@nodece
Copy link
Member Author

nodece commented Dec 28, 2023

Related PR: #19827

@nodece
Copy link
Member Author

nodece commented Dec 28, 2023

/pulsarbot rerun-failure-checks

1 similar comment
@nodece
Copy link
Member Author

nodece commented Dec 29, 2023

/pulsarbot rerun-failure-checks

@dao-jun
Copy link
Member

dao-jun commented Dec 29, 2023

Seems we need a PIP?

image

@BewareMyPower
Copy link
Contributor

@dao-jun Yeah I agree. And I left a comment my comment in the discussion: https://lists.apache.org/thread/olftk1n8ty05xwq59s1z75d72xjpj7no

@nodece
Copy link
Member Author

nodece commented Jan 8, 2024

Hi @dao-jun and @BewareMyPower, thank you for your feedback, I want to add getMessageByIdAsync(String) instead of the multiple parameters, the String format is ledgerId:entryId:batchIndex or ledgerId:entryId, what do you think?

@BewareMyPower
Copy link
Contributor

the String format is ledgerId:entryId:batchIndex or ledgerId:entryId, what do you think?

LGTM

@nodece
Copy link
Member Author

nodece commented Jan 22, 2024

Closed by #21918

@nodece nodece closed this Jan 22, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants