-
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
[improve][broker] PIP-307: Add proxy support for Java client #21789
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@dragosvictor Please add the following content to your PR description and select a checkbox:
|
github-actions
bot
added
doc-not-needed
Your PR changes do not impact docs
and removed
doc-label-missing
labels
Dec 21, 2023
dragosvictor
changed the title
PIP-307: Add proxy support for Java client
[improve][broker] PIP-307: Add proxy support for Java client
Dec 21, 2023
heesung-sn
reviewed
Dec 22, 2023
pulsar-broker/src/test/java/org/apache/pulsar/client/impl/PulsarTestClient.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpLookupService.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
Outdated
Show resolved
Hide resolved
...r-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithExtensibleLoadManagerTest.java
Show resolved
Hide resolved
dragosvictor
force-pushed
the
pip-307-proxy
branch
from
December 22, 2023 21:29
02af0c3
to
09f25d1
Compare
heesung-sn
approved these changes
Dec 22, 2023
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #21789 +/- ##
============================================
- Coverage 73.42% 73.39% -0.04%
- Complexity 32795 32797 +2
============================================
Files 1897 1898 +1
Lines 140656 140668 +12
Branches 15491 15492 +1
============================================
- Hits 103282 103239 -43
- Misses 29297 29353 +56
+ Partials 8077 8076 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
merlimat
approved these changes
Dec 23, 2023
Closed
3 tasks
Closed
2 tasks
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PIP-307
Motivation
PRs #21408 and #21682 implemented producer/consumer support as per the new load balancer transfer mechanism, described by PIP-307, but both assumed that the clients were directly connected to the brokers. This PR implements the respective functionality when connections are made via the Pulsar proxy.
Modifications
LookupService.getBroker
implementations but is not forwarded to the outside callers. The PR changes this method's signature to return a newLookupTopicResult
object, that contains all the pertinent information and allows for ease of extension in the future.ConnectionHandler
and use it when the connection is closed with a broker URL redirection.Verifying this change
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
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: (dragosvictor#3)