You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by bomeng <gi...@git.apache.org> on 2016/06/07 16:05:40 UTC
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
GitHub user bomeng opened a pull request:
https://github.com/apache/spark/pull/13543
[SPARK-15806] [Documentation] update doc for SPARK_MASTER_IP
## What changes were proposed in this pull request?
SPARK_MASTER_IP is a deprecated environment variable. It is replaced by SPARK_MASTER_HOST according to MasterArguments.scala.
## How was this patch tested?
Manually verified.
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bomeng/spark SPARK-15806
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13543.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 #13543
----
commit 239cdfc08e5ad28864574f9ddbcf8240dd5a51ff
Author: bomeng <bm...@us.ibm.com>
Date: 2016-06-07T16:03:19Z
update doc
----
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66490508
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
Yeah, the env param already controls the value of `--host` from the script, so I think this is redundant. Right? Although I don't feel strongly about the second point, I thought it might be tidier to handle the env param in one place rather than two. Env variables do feel like something more from bash-land than Scala-land.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66669128
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
But Master.scala doesn't use SPARK_MASTER_HOST. Why does this matter?
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/13543
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Merged build finished. Test PASSed.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on the issue:
https://github.com/apache/spark/pull/13543
Please note that there are also some places still using SPARK_MASTER_IP, for example, start-master.sh, etc. I did not replace them, because it may break the current running script.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13543
Merged to master/2.0
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
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 issue #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on the issue:
https://github.com/apache/spark/pull/13543
Yes. I can add a warning if SPARK_MASTER_IP is set. Ideally we should use SPARK_MASTER_HOST in all places to avoid confusion.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60130 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60130/consoleFull)** for PR 13543 at commit [`239cdfc`](https://github.com/apache/spark/commit/239cdfc08e5ad28864574f9ddbcf8240dd5a51ff).
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on the issue:
https://github.com/apache/spark/pull/13543
Here is the link: [MasterArguments.scala](https://github.com/bomeng/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala#L56-L59)
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60142 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60142/consoleFull)** for PR 13543 at commit [`6f29181`](https://github.com/apache/spark/commit/6f29181d92141db2270a90c2315c0399060bc7d0).
* 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 issue #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Merged build finished. Test PASSed.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66208902
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,23 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
--- End diff --
Here I think we would also want to set host?
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66647339
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
As I found before, MasterArguments.scala is currently used by Master.scala, I think we need to keep SPARK_MASTER_HOST as for now. Please let me know how we should proceed for this one.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60130 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60130/consoleFull)** for PR 13543 at commit [`239cdfc`](https://github.com/apache/spark/commit/239cdfc08e5ad28864574f9ddbcf8240dd5a51ff).
* This patch passes all 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 issue #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60142 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60142/consoleFull)** for PR 13543 at commit [`6f29181`](https://github.com/apache/spark/commit/6f29181d92141db2270a90c2315c0399060bc7d0).
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13543
OK let's go with this. It's basically flip flopping the arguments so that the more natural HOST arg is preferred. Both still work.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66701686
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
Master.scala create an instance of MasterArguments (line 1008), and MasterArguments will read environment as its initial values (includes SPARK_MASTER_HOST), that is the original logic. User may not pass in --host and just use the SPARK_MASTER_HOST as its value.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66438856
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
This looks good to me. Now, one final thought. We are sort of moving away from env variables anyway. I think we could now remove the handling of `SPARK_MASTER_HOST` here since it's redundant with the scripts in `sbin/`.
And then beyond that, I wonder if we should just handle the deprecation warning in the same place, in the scripts, rather than code. What do you think?
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66488409
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
The code here is just to set its initial values and it may be changed by "--host" configuration. I think we should keep it there for now. For the warning message, we kind of always use logger, not sure it is a good idea to put into the script. I am open to your decision.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on the issue:
https://github.com/apache/spark/pull/13543
@srowen Thanks for merging.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66501686
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
The script sets --host to the value of SPARK_MASTER_HOST though, and I think that's considered the only valid way to start the master. It is a little behavior change but I was wondering if it's best to go ahead and move any unofficial use case away from env variables anyway.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60130/
Test PASSed.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60182 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60182/consoleFull)** for PR 13543 at commit [`03bb009`](https://github.com/apache/spark/commit/03bb009bab5322b1dc7479aad9267e7d10bd49a7).
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13543
More needs to change, like spark-env.sh.template and the scripts in sbin
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60182/
Test PASSed.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66703119
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
Yeah, but that would mean they're embedding master or running it without the script, in which case bets are off. This is what I mean we should not need to support anymore.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60146 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60146/consoleFull)** for PR 13543 at commit [`adcaaab`](https://github.com/apache/spark/commit/adcaaab52f711e9c9b0ad2f7fe7db374c7399064).
* This patch passes all 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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by bomeng <gi...@git.apache.org>.
Github user bomeng commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66493098
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala ---
@@ -20,18 +20,24 @@ package org.apache.spark.deploy.master
import scala.annotation.tailrec
import org.apache.spark.SparkConf
+import org.apache.spark.internal.Logging
import org.apache.spark.util.{IntParam, Utils}
/**
* Command-line parser for the master.
*/
-private[master] class MasterArguments(args: Array[String], conf: SparkConf) {
+private[master] class MasterArguments(args: Array[String], conf: SparkConf) extends Logging {
var host = Utils.localHostName()
var port = 7077
var webUiPort = 8080
var propertiesFile: String = null
// Check for settings in environment variables
+ if (System.getenv("SPARK_MASTER_IP") != null) {
+ logWarning("SPARK_MASTER_IP is deprecated, please use SPARK_MASTER_HOST")
+ host = System.getenv("SPARK_MASTER_IP")
+ }
+
if (System.getenv("SPARK_MASTER_HOST") != null) {
--- End diff --
MasterArguments.scala is used by Master.scala main() method, so there is a way to use `SPARK_MASTER_HOST
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13543
The thing is, `SPARK_MASTER_HOST` is not used anywhere in the code but this file. It's not documented. I would imagine it is the one that's deprecated, but, have a look at https://issues.apache.org/jira/browse/SPARK-3474 In any event, I wonder if we should finally standardize on SPARK_MASTER_HOST? It's as if these two should literally be flipped everywhere in the code, and then this line would generate a warning perhaps if SPARK_MASTER_IP is set? https://github.com/bomeng/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala#L35
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13543
Pardon, where do you see that SPARK_MASTER_IP is deprecated?
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60182 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60182/consoleFull)** for PR 13543 at commit [`03bb009`](https://github.com/apache/spark/commit/03bb009bab5322b1dc7479aad9267e7d10bd49a7).
* This patch passes all 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 issue #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13543
**[Test build #60146 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60146/consoleFull)** for PR 13543 at commit [`adcaaab`](https://github.com/apache/spark/commit/adcaaab52f711e9c9b0ad2f7fe7db374c7399064).
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13543#discussion_r66208925
--- Diff: docs/spark-standalone.md ---
@@ -94,8 +94,8 @@ You can optionally configure the cluster further by setting environment variable
<table class="table">
<tr><th style="width:21%">Environment Variable</th><th>Meaning</th></tr>
<tr>
- <td><code>SPARK_MASTER_IP</code></td>
- <td>Bind the master to a specific IP address, for example a public one.</td>
+ <td><code>SPARK_MASTER_HOST</code></td>
+ <td>Bind the master to a specific host name, for example a public one.</td>
--- End diff --
host name or IP 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 issue #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Merged build finished. Test PASSed.
---
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 #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60142/
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 issue #13543: [SPARK-15806] [Documentation] update doc for SPARK_MASTE...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13543
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60146/
Test PASSed.
---
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