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

[fix][broker] Fix getMessageById throws 500 #21919

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

nodece
Copy link
Member

@nodece nodece commented Jan 18, 2024

Motivation

When using bin/pulsar-admin topics get-message-by-id public/default/my-topic-1 -e 0 -l 1024 with a non-existent ledger, I expect to throw the 400 error instead of 500.

CLI log:

$ bin/pulsar-admin topics get-message-by-id public/default/my-topic-1 -e 0 -l 1024

 --- An unexpected error occurred in the server ---

Message: Message not found, the ledgerId does not belong to this topic or has been deleted

Stacktrace:

org.apache.bookkeeper.mledger.ManagedLedgerException$LedgerNotExistException: Message not found, the ledgerId does not belong to this topic or has been deleted


Reason: 
 --- An unexpected error occurred in the server ---

Message: Message not found, the ledgerId does not belong to this topic or has been deleted

Stacktrace:

org.apache.bookkeeper.mledger.ManagedLedgerException$LedgerNotExistException: Message not found, the ledgerId does not belong to this topic or has been deleted

Broker log:

2024-01-18T18:03:46,824+0800 [pulsar-web-47-14] INFO  org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [18/1月/2024:18:03:46 +0800] "GET /admin/v2/persistent/public/default/my-topic-1/ledger/1024/entry/0 HTTP/1.1" 500 340 "-" "Pulsar-Java-v3.2.0" 29

Modifications

  • In the internalGetMessageById, when readEntryFailed returns LedgerNotExitException, convert this exception to 400 error.
  • When the admin receives the 400 error, skip handle recovery.

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 18, 2024
@nodece nodece closed this Jan 18, 2024
@nodece nodece reopened this Jan 18, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece force-pushed the fix-get-messages-by-id-500 branch from 7000cf4 to 3efd4a6 Compare January 18, 2024 11:12
Copy link
Member

@dao-jun dao-jun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nodece
Copy link
Member Author

nodece commented Jan 18, 2024

/pulsarbot rerun-failure-checks

@nodece nodece added this to the 3.3.0 milestone Jan 18, 2024
@nodece nodece merged commit 55520bd into apache:master Jan 19, 2024
49 of 51 checks passed
@Technoboy- Technoboy- modified the milestones: 3.3.0, 3.2.0 Jan 22, 2024
Technoboy- pushed a commit that referenced this pull request Jan 22, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Technoboy- pushed a commit that referenced this pull request Jan 31, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Technoboy- pushed a commit that referenced this pull request Feb 5, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 6f7b9d9)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 6, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 6f7b9d9)
nodece added a commit to ascentstream/pulsar that referenced this pull request Dec 18, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 55520bd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants