-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix][client] Fix retry logic for BinaryProtoLookupService.getTopicsUnderNamespace method #24974
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
base: master
Are you sure you want to change the base?
Conversation
…Cnx to ease unit testing
2da65d4 to
2562c19
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24974 +/- ##
=============================================
+ Coverage 38.69% 74.28% +35.58%
- Complexity 13362 34017 +20655
=============================================
Files 1863 1920 +57
Lines 145975 150236 +4261
Branches 16928 17428 +500
=============================================
+ Hits 56487 111604 +55117
+ Misses 81858 29719 -52139
- Partials 7630 8913 +1283
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| private final Queue<Object> queue = Queues.newArrayDeque(); | ||
|
|
||
| public ClientChannelHelper() { | ||
| int maxMessageSize = 5 * 1024 * 1024; |
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.
This can be removed.
| }, lookupPinnedExecutor).whenComplete((r, t) -> { | ||
| if (t != null) { | ||
| Throwable cause = FutureUtil.unwrapCompletionException(t); | ||
| if (cause instanceof PulsarClientException && !PulsarClientException.isRetriableError(cause)) { |
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.
Is it covered in the test here?
Motivation
BinaryProtoLookupService.getTopicsUnderNamespace method contains retry logic, but it only applies to exceptions that occur when acquiring the connection fails.
A getTopicsUnderNamespace might fail later if the connection gets closed before the response has been successfully received. That's why it's better that the retry logic would also cover the actual operation to be effective.
Modifications
Documentation
docdoc-requireddoc-not-neededdoc-complete