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

[junit-runner] cache localhost lookups to ease OSX/JDK DNS issues #5660

Merged
merged 2 commits into from Apr 5, 2018

Conversation

Projects
None yet
3 participants
@baroquebobcat
Copy link
Contributor

baroquebobcat commented Apr 5, 2018

Problem

OSX's DNS service's handling of localhost names changed, I think with 10.12, in a way that combined with JDK releases after a particular point release of 1.8 causes looking up localhost's name to take on the order of seconds for some folks. https://bugs.openjdk.java.net/browse/JDK-8170910

Solution

This patch memoizes the localhost lookup used by the xml listener of Pants' junit runner so that it won't be re-looked up per test suite entry.

As a workaround for the problem, OSX users can add an additional name in their /etc/hosts file for 127.0.0.1, eg

127.0.0.1       localhost My-MacBook-Pro
[junit-runner] cache localhost lookups to ease OSX/JDK DNS issues
OSX's DNS service's handling of localhost names changed, I think with 10.12, in a way that combined with JDK releases after a particular point release of 1.8 causes looking up localhost's name to take on the order of seconds for some folks. https://bugs.openjdk.java.net/browse/JDK-8170910

This patch memoizes the localhost lookup used by the xml listener of Pants' junit runner so that it won't be re-looked up per test suite entry.

@baroquebobcat baroquebobcat requested review from jsirois and stuhood Apr 5, 2018

@stuhood

stuhood approved these changes Apr 5, 2018

// NB: Cache the localhost name as a workaround for an issue where localhost resolution on OSX
// may take seconds.
// See: https://bugs.openjdk.java.net/browse/JDK-8170910
private static String localHostName = null;

This comment has been minimized.

@stuhood

stuhood Apr 5, 2018

Member

Is it worth eagerly initializing this, or are there cases where laziness will avoid actually needing to construct it?

This comment has been minimized.

@baroquebobcat

baroquebobcat Apr 5, 2018

Contributor

Good question. I think I'm going to adjust some things. Since this is only necessary when the listener is used, I think I can hoist this into the listener's constructor instead and pass the hostname from the listener to the inner class that uses it on initialization.

@baroquebobcat baroquebobcat merged commit 7db1588 into pantsbuild:master Apr 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@baroquebobcat baroquebobcat deleted the baroquebobcat:nhoward/cache_localhost branch Apr 5, 2018

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