[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...

[zhangminglei]

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5449#discussion_r172004051
  
--- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/net/ConnectionUtilsTest.java
 ---
@@ -61,7 +61,7 @@ public void testReturnLocalHostAddressUsingHeuristics() 
throws Exception {
assertNotNull(add);
 
// make sure that we returned the 
InetAddress.getLocalHost as a heuristic
-   assertEquals(InetAddress.getLocalHost(), add);
+   assertEquals(InetAddress.getByName("localhost"), add);
--- End diff --

Or, we can also still use ```InetAddress.getByName("localhost")```. Both 
```null``` and ```InetAddress.getByName("localhost")``` are OKay.


---

[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...

[zhangminglei]

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5449#discussion_r172004016
  
--- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/net/ConnectionUtilsTest.java
 ---
@@ -61,7 +61,7 @@ public void testReturnLocalHostAddressUsingHeuristics() 
throws Exception {
assertNotNull(add);
 
// make sure that we returned the 
InetAddress.getLocalHost as a heuristic
-   assertEquals(InetAddress.getLocalHost(), add);
+   assertEquals(InetAddress.getByName("localhost"), add);
--- End diff --

Thanks @NicoK review. Yes. I think this is a better way to use ```null``` 
instead of ```InetAddress.getLocalHost()``` to prevent multiple adapters cache 
expires from getting a  another result in the future. I will give a quick fix 
for that.


---

[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...

[NicoK]

Github user NicoK commented on a diff in the pull request:

https://github.com/apache/flink/pull/5449#discussion_r171858504
  
--- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/net/ConnectionUtilsTest.java
 ---
@@ -61,7 +61,7 @@ public void testReturnLocalHostAddressUsingHeuristics() 
throws Exception {
assertNotNull(add);
 
// make sure that we returned the 
InetAddress.getLocalHost as a heuristic
-   assertEquals(InetAddress.getLocalHost(), add);
+   assertEquals(InetAddress.getByName("localhost"), add);
--- End diff --

How about also changing the blocker above to accept connections on any 
interface just to rule accidental successful resolve attempts out, too, i.e. 
using `ServerSocket blocker = new ServerSocket(0, 1, null)`?


---

[GitHub] flink pull request #5449: [FLINK-8623] ConnectionUtilsTest.testReturnLocalHo...

[zhangminglei]

GitHub user zhangminglei opened a pull request:

https://github.com/apache/flink/pull/5449

[FLINK-8623] ConnectionUtilsTest.testReturnLocalHostAddressUsingHeuri…

## What is the purpose of the change
Try to fix ConnectionUtilsTest.testReturnLocalHostAddressUsingHeuristics 
unstable on Travis. 

## Brief change log
Change the behavior get localhost ```InetAddress``` to 
```InetAddress.getByName("localhost")``` instead of 
```InetAddress.getLocalHost()```, as the latter one will return the actual IP 
of one of your network adapters.
And in general ```InetAddress.getLocalHost()``` should be avoided.

## Verifying this change
This change is already covered by existing tests, is 
```testReturnLocalHostAddressUsingHeuristics``` in 
```ConnectionUtilsTest.java```.

## Does this pull request potentially affect one of the following parts:

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: (no)
  - The serializers: (no)
  - The runtime per-record code paths (performance sensitive): (no )
  - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  - The S3 file system connector: (no)

## Documentation

  - Does this pull request introduce a new feature? (no)
  - If yes, how is the feature documented? (not documented)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zhangminglei/flink flink-8623

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/5449.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #5449


commit 67e70d558c7d2f47ed3f7a407ad250518a2f683c
Author: zhangminglei 
Date:   2018-02-10T07:16:09Z

[FLINK-8623] ConnectionUtilsTest.testReturnLocalHostAddressUsingHeuristics 
unstable on Travis




---