You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by skyluc <gi...@git.apache.org> on 2016/04/22 17:18:28 UTC
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
GitHub user skyluc opened a pull request:
https://github.com/apache/spark/pull/12613
[SPARK-14849][CORE]Always set an address for the executor
As specified in [SPARK-14849](https://issues.apache.org/jira/browse/SPARK-14849), the `address` in the `NettyRpcEnv` for the executor is not set when running in standalone mode. To compensate, the driver try to guess the IP address of the executor during registration. But in a NAT situation, the IP address visible in the connection is not the one that should be used.
This address is sent back to the executor, and used later to describe the location of blocks.
The change is too always set the address as the `host` value, and to remove the code on the driver side which is trying to guess the right IP address. This kind of guessing is wrong in a NAT configuration, a possibly others.
Manually tested on the configuration describe on the ticket, and on a standard Mesos cluster.
Testing on a Yarn cluster would be useful, to check that there is no unexpected effects.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/skyluc/spark issue/nat-standalone
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12613.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 #12613
----
commit 1cf83220fa9e826a1c15708ed990103182b1f403
Author: Luc Bourlier <lu...@typesafe.com>
Date: 2016-04-08T12:36:19Z
Always set an address for the executor
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215147616
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57133/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215081590
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215081239
**[Test build #57125 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57125/consoleFull)** for PR 12613 at commit [`0624898`](https://github.com/apache/spark/commit/0624898fde153b6d19968efb6b501ad8bcdcf3c0).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12613#discussion_r61022118
--- Diff: core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala ---
@@ -122,7 +122,7 @@ private[netty] class NettyRpcEnv(
@Nullable
override lazy val address: RpcAddress = {
- if (server != null) RpcAddress(host, server.getPort()) else null
+ if (server != null) RpcAddress(host, server.getPort()) else RpcAddress(host, -1)
--- End diff --
while you are at it, we should probably document when server is null and explain the choices here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-213511310
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215147612
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by skyluc <gi...@git.apache.org>.
Github user skyluc commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-214336966
Are the test failures real, or due to flaky tests?
I tried to reproduced the failures locally, but `core/test` passes most of the time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12613: [SPARK-14849][CORE]Always set an address for the executo...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:
https://github.com/apache/spark/pull/12613
@skyluc could you close this one, please? You can submit a new PR when you have a better idea. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215117480
**[Test build #57133 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57133/consoleFull)** for PR 12613 at commit [`baaef11`](https://github.com/apache/spark/commit/baaef115dda8472eb2be7b0b23b47150079e6a2e).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #12613: [SPARK-14849][CORE]Always set an address for the ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12613
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/12613#discussion_r61776444
--- Diff: core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala ---
@@ -120,9 +120,15 @@ private[netty] class NettyRpcEnv(
RpcEndpointVerifier.NAME, new RpcEndpointVerifier(this, dispatcher))
}
- @Nullable
override lazy val address: RpcAddress = {
- if (server != null) RpcAddress(host, server.getPort()) else null
+ // When running in client mode (i.e.: not opening a listening port, but connecting to a
+ // 'server' with an open port), the server value is null.
+ // But a valid host still has to be provided, as it is needed later on.
+ // Previously, the server side of the connection was attempting to guess the client
+ // hostname, using the connection information. But the value generated is wrong when
+ // the connection is NATed.
+ // [SPARK-14849]
+ if (server != null) RpcAddress(host, server.getPort()) else RpcAddress(host, -1)
--- End diff --
`-1` is actually confusing. All of executors in the same node will have the same address.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-216313536
The host is easy to fix. However, the hard part is `port`. The executor cannot know the port until it sends a message. In addition, since the executor runs in the client mode, it may have multiple clients/ports.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by skyluc <gi...@git.apache.org>.
Github user skyluc commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215080708
Added comments, updated checks for cases when running in client mode, removed sending back the hostname to the executor.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215147386
**[Test build #57133 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57133/consoleFull)** for PR 12613 at commit [`baaef11`](https://github.com/apache/spark/commit/baaef115dda8472eb2be7b0b23b47150079e6a2e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-214591914
I think there are also some code in SparkEnv that deals with this?
```
if (isDriver) {
conf.set("spark.driver.port", rpcEnv.address.port.toString)
} else if (rpcEnv.address != null) {
conf.set("spark.executor.port", rpcEnv.address.port.toString)
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215081584
**[Test build #57125 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57125/consoleFull)** for PR 12613 at commit [`0624898`](https://github.com/apache/spark/commit/0624898fde153b6d19968efb6b501ad8bcdcf3c0).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-213471010
**[Test build #56705 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56705/consoleFull)** for PR 12613 at commit [`1cf8322`](https://github.com/apache/spark/commit/1cf83220fa9e826a1c15708ed990103182b1f403).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12613#discussion_r61022100
--- Diff: core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala ---
@@ -122,7 +122,7 @@ private[netty] class NettyRpcEnv(
@Nullable
--- End diff --
this is no longer nullable
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-213798398
cc @zsxwing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-213510771
**[Test build #56705 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56705/consoleFull)** for PR 12613 at commit [`1cf8322`](https://github.com/apache/spark/commit/1cf83220fa9e826a1c15708ed990103182b1f403).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-215081593
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57125/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14849][CORE]Always set an address for t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12613#issuecomment-213511313
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56705/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org