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

Skip unnecessary DNS resolution when creating AuthenticationDataHttp instance #15221

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Apr 19, 2022

Motivation

Using the DNS resolution is a blocking operation in Java. In this case it's completely unnecessary to do a reverse lookup for the client's IP address.

Modifications

  • Use Netty's utility class to create a InetAddress instance from the IP address string without using DNS resolution.

@lhotari lhotari added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker doc-not-needed Your PR changes do not impact docs release/2.9.3 release/2.7.6 release/2.8.4 release/2.10.1 labels Apr 19, 2022
@lhotari lhotari added this to the 2.11.0 milestone Apr 19, 2022
@lhotari lhotari self-assigned this Apr 19, 2022
Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari lhotari merged commit 14991c9 into apache:master Apr 20, 2022
lhotari added a commit to lhotari/pulsar that referenced this pull request Apr 20, 2022
…tp instance

- improves solution added in apache#15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution
@lhotari
Copy link
Member Author

lhotari commented Apr 20, 2022

I created a follow up PR #15228 since in this case, the use of InetSocketAddress.createUnresolved method would be the correct solution.

lhotari added a commit that referenced this pull request Apr 20, 2022
…tp instance (#15228)

- improves solution added in #15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 20, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 20, 2022
…tp instance (apache#15228)

- improves solution added in apache#15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 21, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 21, 2022
…tp instance (apache#15228)

- improves solution added in apache#15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 21, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 21, 2022
…tp instance (apache#15228)

- improves solution added in apache#15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
@nicoloboschi
Copy link
Contributor

@lhotari This and the follow up commits are labelled with 2.10 but I don't see the related changes on branch-2.10 https://github.com/apache/pulsar/blob/branch-2.10/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationDataHttp.java

codelipenghui pushed a commit that referenced this pull request Apr 28, 2022
codelipenghui pushed a commit that referenced this pull request Apr 28, 2022
…tp instance (#15228)

- improves solution added in #15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Apr 28, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Apr 28, 2022
…tp instance (apache#15228)

- improves solution added in apache#15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
codelipenghui pushed a commit that referenced this pull request Apr 28, 2022
codelipenghui pushed a commit that referenced this pull request Apr 28, 2022
…tp instance (#15228)

- improves solution added in #15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Apr 28, 2022
codelipenghui pushed a commit that referenced this pull request Apr 29, 2022
codelipenghui pushed a commit that referenced this pull request Apr 29, 2022
…tp instance (#15228)

- improves solution added in #15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Apr 29, 2022
Jason918 pushed a commit that referenced this pull request Jul 29, 2022
Jason918 pushed a commit that referenced this pull request Jul 29, 2022
…tp instance (#15228)

- improves solution added in #15221
  - It's better to use the JDK provided InetSocketAddress.createUnresolved method
    to prevent unnecessary DNS resolution

(cherry picked from commit e71b98a)
@Jason918 Jason918 added cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.5 and removed release/2.7.6 labels Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker 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/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants