Skip to content

[SPARK-14547] Avoid DNS resolution for reusing connections#12315

Closed
rxin wants to merge 2 commits intoapache:masterfrom
rxin:SPARK-14547
Closed

[SPARK-14547] Avoid DNS resolution for reusing connections#12315
rxin wants to merge 2 commits intoapache:masterfrom
rxin:SPARK-14547

Conversation

@rxin
Copy link
Contributor

@rxin rxin commented Apr 12, 2016

What changes were proposed in this pull request?

This patch changes the connection creation logic in the network client module to avoid DNS resolution when reusing connections.

How was this patch tested?

Testing in production. This is too difficult to test in isolation (for high fidelity unit tests, we'd need to change the DNS resolution behavior in the JVM).

@rxin
Copy link
Contributor Author

rxin commented Apr 12, 2016

cc @nongli @zsxwing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I downgraded this logging level from info to trace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's more useful to log a warning if this time is greater than say 2 seconds.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@SparkQA
Copy link

SparkQA commented Apr 12, 2016

Test build #55566 has finished for PR 12315 at commit 70b5e82.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 12, 2016

Test build #2774 has finished for PR 12315 at commit 70b5e82.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 12, 2016

Test build #2775 has finished for PR 12315 at commit 70b5e82.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@nongli
Copy link
Contributor

nongli commented Apr 12, 2016

lgtm

@rxin
Copy link
Contributor Author

rxin commented Apr 12, 2016

OK updated.

@SparkQA
Copy link

SparkQA commented Apr 12, 2016

Test build #55625 has finished for PR 12315 at commit 886ab09.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 12, 2016

Test build #2780 has finished for PR 12315 at commit 886ab09.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor Author

rxin commented Apr 12, 2016

Alright going to merge this.

@asfgit asfgit closed this in c439d88 Apr 12, 2016
zzcclp pushed a commit to zzcclp/spark that referenced this pull request Apr 13, 2016
final long preResolveHost = System.nanoTime();
final InetSocketAddress resolvedAddress = new InetSocketAddress(remoteHost, remotePort);
final long hostResolveTimeMs = (System.nanoTime() - preResolveHost) / 1000000;
if (hostResolveTimeMs > 2000) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the value of 2000 be configurable ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants