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

Jump to bottom

[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler #15415

Merged
merged 2 commits into from May 3, 2022
Merged

[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler #15415

merged 2 commits into from May 3, 2022

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented May 3, 2022 •
edited

Motivation

  • Calling new InetSocketAddress(brokerURI.getHost(), brokerURI.getPort()) does a blocking DNS lookup. This should be avoided since Pulsar's ConnectionPool uses Netty's non-blocking DNS lookup. Unresolved addresses are expected as input to ConnectionPool.

Modifications

replace new InetSocketAddress with InetSocketAddress.createUnresolved by using existing helper method to create the InetSocketAddress instance

@lhotari lhotari added this to the 2.11.0 milestone May 3, 2022
@lhotari lhotari self-assigned this May 3, 2022
@github-actions
Copy link

github-actions bot commented May 3, 2022

@lhotari:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@lhotari lhotari added the doc-not-needed Your PR changes do not impact docs label May 3, 2022
@github-actions
Copy link

github-actions bot commented May 3, 2022

@lhotari:Thanks for providing doc info!

@lhotari lhotari marked this pull request as draft May 3, 2022 15:26
poorbarcode
poorbarcode approved these changes May 3, 2022
View reviewed changes
Copy link
Contributor

@poorbarcode poorbarcode left a comment

Choose a reason for hiding this comment

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

After this patch, addr.getAddress() will return null, but I can't find a place to do that.

LGTM

@lhotari lhotari marked this pull request as ready for review May 3, 2022 16:45
eolivelli
eolivelli approved these changes May 3, 2022
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Good catch

@lhotari lhotari merged commit 7373a51 into apache:master May 3, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request May 4, 2022
@lhotari
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (a…
032be96 
…pache#15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
lhotari added a commit to datastax/pulsar that referenced this pull request May 4, 2022
@lhotari
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (a…
85f0767 
…pache#15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
lhotari added a commit to datastax/pulsar that referenced this pull request May 4, 2022
@lhotari
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (a…
5980cdc 
…pache#15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
@michaeljmarshall
Copy link
Member

LGTM.

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 10, 2022
@lhotari @nicoloboschi
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (a…
d0105df 
…pache#15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
(cherry picked from commit 85f0767)
codelipenghui pushed a commit that referenced this pull request May 20, 2022
@lhotari @codelipenghui
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (#…
fd44570 
…15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label May 20, 2022
codelipenghui pushed a commit that referenced this pull request May 20, 2022
@lhotari @codelipenghui
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (#…
d776e17 
…15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
mattisonchao pushed a commit that referenced this pull request May 25, 2022
@lhotari @mattisonchao
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (#…
9955b0a 
…15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
@mattisonchao mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label May 25, 2022
lhotari added a commit that referenced this pull request Jun 1, 2022
@lhotari
[Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler (#…
462d44f 
…15415)

* [Proxy] Remove unnecessary blocking DNS lookup in LookupProxyHandler

* Use existing code pattern for creating address

(cherry picked from commit 7373a51)
(cherry picked from commit 5980cdc)
@lhotari lhotari added cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.5 and removed release/2.7.6 labels Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merlimat merlimat merlimat approved these changes

@HQebupt HQebupt HQebupt approved these changes

@eolivelli eolivelli eolivelli approved these changes

@nicoloboschi nicoloboschi nicoloboschi approved these changes

@poorbarcode poorbarcode poorbarcode approved these changes

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

Assignees

@lhotari lhotari

Labels
area/proxy cherry-picked/branch-2.7 Archived: 2.7 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.7.5 release/2.8.4 release/2.9.3 release/2.10.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
2.11.0
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@lhotari @michaeljmarshall @merlimat @HQebupt @eolivelli @nicoloboschi @poorbarcode @codelipenghui @mattisonchao