-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Issue 14013][broker]make internalPeekNthMessage method async #14039
Conversation
2410170
to
768ce72
Compare
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Outdated
Show resolved
Hide resolved
Thank you @codelipenghui PLAT |
759504d
to
abec3f1
Compare
/pulsarbot rerun-failure-checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another, I have two suggestions:
- Remove asyncResponse from the internal method, and unified exception and result handling are performed at the controller layer, which makes the internal method purer.
- Have you checked to see if there is a test for the modified method?
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Outdated
Show resolved
Hide resolved
/pulsarbot rerun-failure-checks |
1 similar comment
/pulsarbot rerun-failure-checks |
/pulsarbot rerun-failure-checks |
/pulsarbot rerun-failure-checks |
The pr had no activity for 30 days, mark with Stale label. |
The pr had no activity for 30 days, mark with Stale label. |
Master Issue: #14013
Motivation
See #14013
Verifying this change
Make sure that the change passes the CI checks.
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
no-need-doc