[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/13543#discussion_r9128 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 Environment VariableMeaning -SPARK_MASTER_IP -Bind the master to a specific IP address, for example a public one. +SPARK_MASTER_HOST +Bind the master to a specific host name, for example a public one. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #13543: [SPARK-15806] [Documentation] update doc for SPAR...
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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[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 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org