You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kelepi <gi...@git.apache.org> on 2014/10/09 09:34:32 UTC

[GitHub] spark pull request: Add spark.tmp.dir to set temp directory for sp...

GitHub user kelepi opened a pull request:

    https://github.com/apache/spark/pull/2729

    Add spark.tmp.dir to set temp directory for spark

    Related to SPARK-3875.


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

    $ git pull https://github.com/kelepi/spark master

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

    https://github.com/apache/spark/pull/2729.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 #2729
    
----
commit f14b68a4c98bf01db84afa55505868d42d8e4eb5
Author: yf_liu <yf...@ctrip.com>
Date:   2014-10-09T07:32:01Z

    Add spark.tmp.dir to set temp directory for spark

----


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729#issuecomment-59756273
  
    Yes, as @mridulm pointed out. This should not be settable by the users on yarn.  It should automatically use the yarn approved directories. We have logic in there for setting the java.io.tmpdir in ClientBase.  If this is added we would need to do something similar and not let the user override it.


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729#issuecomment-58479810
  
    There is a java property which controls this ... java.io.tmpdir
    On 09-Oct-2014 1:22 pm, "刘钰帆" <no...@github.com> wrote:
    
    > @mridulm <https://github.com/mridulm> Using std java property is fine.
    > Just add a more specific configuration argument.
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/2729#issuecomment-58474600>.
    >


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729#issuecomment-58473598
  
    `Utils.getLocalDir` already does basically this. I agree that files should not have to go to `/tmp` since this is rarely a good place for lots of stuff on servers. But I do wonder whether this local dir is in fact the standard and right place for all of these things rather than another temp dir setting. Distributions already correctly configure where `Utils.getLocalDir` looks so this is going to be much better if possible.


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729#issuecomment-58474600
  
    @mridulm Using std java property is fine. 
    Just add a more specific configuration argument.


---
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: Add spark.tmp.dir to set temp directory for sp...

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

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


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729#issuecomment-58473957
  
    At least for yarn, this will create issues if overridden from default.
    Not sure about mesos.
    
    Why not use std java property and define it for local and standalone mode where relevant.


---
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-3875] Add TEMP DIRECTORY configuration

Posted by kelepi <gi...@git.apache.org>.
Github user kelepi commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2729#discussion_r18630974
  
    --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
    @@ -233,7 +233,11 @@ private[spark] object Utils extends Logging {
       }
     
       /** Create a temporary directory inside the given parent directory */
    -  def createTempDir(root: String = System.getProperty("java.io.tmpdir")): File = {
    +  def createTempDir(): File = {
    +    val root: String = SparkEnv.get.conf.contains("spark.tmp.dir") match {
    --- End diff --
    
    Thanks @srowen for the comment. That's more clear!
    Currently I am running Spark 1.1 in Standalone mode. And I have set the 'SPARK_LOCAL_DIRS' to data disk,  which will store the broadcast Files in the target directory. However, the dependencies for executors will be fetched and stored in the /tmp/ directory.
    And the dependencies will not be removed.
    Like the snappy dependency, for example:
    "snappy-1.0.5.3-f4880c9f-95d9-4ab6-b1c8-8686d0b88f42-libsnappyjava.so"
    So the /tmp/ directory will grow large.


---
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-3875] Add TEMP DIRECTORY configuration

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

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


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729


---
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-3875] Add TEMP DIRECTORY configuration

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

    https://github.com/apache/spark/pull/2729#issuecomment-73221480
  
    It sounds like this is a wont-fix, given the discussion. Do you mind closing this PR?


---
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-3875] Add TEMP DIRECTORY configuration

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2729#discussion_r18630676
  
    --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
    @@ -233,7 +233,11 @@ private[spark] object Utils extends Logging {
       }
     
       /** Create a temporary directory inside the given parent directory */
    -  def createTempDir(root: String = System.getProperty("java.io.tmpdir")): File = {
    +  def createTempDir(): File = {
    +    val root: String = SparkEnv.get.conf.contains("spark.tmp.dir") match {
    --- End diff --
    
    PS this can all be one line if you set the default value to `... .get("spark.tmp.dir", System.getProperty("java.io.tmpdir"))`


---
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