You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by CodingCat <gi...@git.apache.org> on 2014/02/15 00:33:58 UTC

[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

GitHub user CodingCat opened a pull request:

    https://github.com/apache/incubator-spark/pull/602

    [SPARK-1092] remove SPARK_MEM usage in sparkcontext.scala

    https://spark-project.atlassian.net/browse/SPARK-1092?jql=project%20%3D%20SPARK
    
    Currently, users will usually set SPARK_MEM to control the memory usage of driver programs, (in spark-class)
    91 JAVA_OPTS="$OUR_JAVA_OPTS"
    92 JAVA_OPTS="$JAVA_OPTS -Djava.library.path=$SPARK_LIBRARY_PATH"
    93 JAVA_OPTS="$JAVA_OPTS -Xms$SPARK_MEM -Xmx$SPARK_MEM"
    if they didn't set spark.executor.memory, the value in this environment variable will also affect the memory usage of executors, because the following lines in SparkContext
    privatespark val executorMemory = conf.getOption("spark.executor.memory")
    .orElse(Option(System.getenv("SPARK_MEM")))
    .map(Utils.memoryStringToMb)
    .getOrElse(512)
    also
    since SPARK_MEM has been (proposed to) deprecated in SPARK-929 (https://spark-project.atlassian.net/browse/SPARK-929) and the corresponding PR (https://github.com/apache/incubator-spark/pull/104)
    we should remove this line

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

    $ git pull https://github.com/apache/incubator-spark clean_spark_mem

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

    https://github.com/apache/incubator-spark/pull/602.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 #602
    
----
commit 31ea1cfe4231c7ff14f2721fa2be99baa43c29d0
Author: CodingCat <zh...@gmail.com>
Date:   2014-02-14T23:31:03Z

    remove SPARK_MEM usage in sparkcontext.scala

----


[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35207540
  
    @aarondav just fixed


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35208278
  
    Merged build started.


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#discussion_r9778280
  
    typo: using => use


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35138743
  
    We can't accept this patch because it will regress behavior for existing users. See this JIRA for a proposal relating to SPARK_MEM: https://spark-project.atlassian.net/browse/SPARK-929


[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35139930
  
    @pwendell @aarondav can any of you help to close the related JIRA?
    
    https://spark-project.atlassian.net/browse/SPARK-1092
    
    Thank you


[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35141292
  
    Hi, @pwendell @aarondav, I reopened it and changed my commit to printing warning information
    
    I just think that executor and driver share the same env variable is not so good 


[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35208277
  
     Merged build triggered.


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35209157
  
    Merged build finished.


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35211178
  
    Neato, Jenkins actually listened to me. Merged into master, thanks!


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35208183
  
    Jenkins doesn't like me at the moment, @rxin is trying to get that fixed. In the meantime, can someone with permissions say "Jenkins, test this please." ?


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35209158
  
    All automated tests passed.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12732/


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35141094
  
    Can one of the admins verify this patch?


[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35157378
  
    Hi, @mridulm , I think it will be used in local, mesos, and standalone mode
    
    1. local
    
    case LOCAL_CLUSTER_REGEX(numSlaves, coresPerSlave, memoryPerSlave) =>
            // Check to make sure memory requested <= memoryPerSlave. Otherwise Spark will just hang.
            val memoryPerSlaveInt = memoryPerSlave.toInt
            if (sc.executorMemory > memoryPerSlaveInt) {
              throw new SparkException(
                "Asked to launch cluster with %d MB RAM / worker but requested %d MB/worker".format(
                  memoryPerSlaveInt, sc.executorMemory))
            }
    
            val scheduler = new TaskSchedulerImpl(sc)
            val localCluster = new LocalSparkCluster(
              numSlaves.toInt, coresPerSlave.toInt, memoryPerSlaveInt)
            val masterUrls = localCluster.start()
            val backend = new SparkDeploySchedulerBackend(scheduler, sc, masterUrls, appName)
            scheduler.initialize(backend)
            backend.shutdownCallback = (backend: SparkDeploySchedulerBackend) => {
              localCluster.stop()
            }
            scheduler
    
    2. standalone (SparkDeployClusterBackend.scala)
    
    override def start() {
        super.start()
    
        // The endpoint for executors to talk to us
        val driverUrl = "akka.tcp://spark@%s:%s/user/%s".format(
          conf.get("spark.driver.host"),  conf.get("spark.driver.port"),
          CoarseGrainedSchedulerBackend.ACTOR_NAME)
        val args = Seq(driverUrl, "{{EXECUTOR_ID}}", "{{HOSTNAME}}", "{{CORES}}", "{{WORKER_URL}}")
        val command = Command(
          "org.apache.spark.executor.CoarseGrainedExecutorBackend", args, sc.executorEnvs)
        val sparkHome = sc.getSparkHome()
        val appDesc = new ApplicationDescription(appName, maxCores, sc.executorMemory, command,
          sparkHome, "http://" + sc.ui.appUIAddress)
    
        client = new AppClient(sc.env.actorSystem, masters, appDesc, this, conf)
        client.start()
      }
    
    3. CoarseMesosSchedulerBackend.scala
    
    resourceOffers(d: SchedulerDriver, offers: JList[Offer])


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by CodingCat <gi...@git.apache.org>.
GitHub user CodingCat reopened a pull request:

    https://github.com/apache/incubator-spark/pull/602

    [SPARK-1092] remove SPARK_MEM usage in sparkcontext.scala

    https://spark-project.atlassian.net/browse/SPARK-1092?jql=project%20%3D%20SPARK
    
    Currently, users will usually set SPARK_MEM to control the memory usage of driver programs, (in spark-class)
    91 JAVA_OPTS="$OUR_JAVA_OPTS"
    92 JAVA_OPTS="$JAVA_OPTS -Djava.library.path=$SPARK_LIBRARY_PATH"
    93 JAVA_OPTS="$JAVA_OPTS -Xms$SPARK_MEM -Xmx$SPARK_MEM"
    if they didn't set spark.executor.memory, the value in this environment variable will also affect the memory usage of executors, because the following lines in SparkContext
    privatespark val executorMemory = conf.getOption("spark.executor.memory")
    .orElse(Option(System.getenv("SPARK_MEM")))
    .map(Utils.memoryStringToMb)
    .getOrElse(512)
    also
    since SPARK_MEM has been (proposed to) deprecated in SPARK-929 (https://spark-project.atlassian.net/browse/SPARK-929) and the corresponding PR (https://github.com/apache/incubator-spark/pull/104)
    we should remove this line

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

    $ git pull https://github.com/apache/incubator-spark clean_spark_mem

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

    https://github.com/apache/incubator-spark/pull/602.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 #602
    
----
commit 31ea1cfe4231c7ff14f2721fa2be99baa43c29d0
Author: CodingCat <zh...@gmail.com>
Date:   2014-02-14T23:31:03Z

    remove SPARK_MEM usage in sparkcontext.scala

commit c9b4872d366a5d41400eacd8d2feef0020d0c109
Author: CodingCat <zh...@gmail.com>
Date:   2014-02-15T00:14:25Z

    print warning information if user use SPARK_MEM to regulate executor memory usage

----


[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35139787
  
    @pwendell @aarondav thanks for pointing this out
    
    agree, closed


[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35138646
  
    Can one of the admins verify this patch?


[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/incubator-spark/pull/602#issuecomment-35207406
  
    Looks good to me, just fix the little typo and I'll merge it in. (I would do it myself but I'm not certain how to amend the commit while keeping you as the original author.)
    
    @mridulm You make a good point about YARN mode -- this warning may not be applicable there. I don't think we need to move the warning in this PR, though, since SPARK_MEM will soon be generally deprecated and shouldn't be used directly by anyone.


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] print warning informati...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat closed the pull request at:

    https://github.com/apache/incubator-spark/pull/602


If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.

[GitHub] incubator-spark pull request: [SPARK-1092] remove SPARK_MEM usage ...

Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat closed the pull request at:

    https://github.com/apache/incubator-spark/pull/602