[GitHub] spark pull request #18838: [SPARK-21632] There is no need to make attempts f...
Github user liu-zhaokun closed the pull request at: https://github.com/apache/spark/pull/18838 --- 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 #18838: [SPARK-21632] There is no need to make attempts f...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/18838#discussion_r131328303 --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala --- @@ -294,9 +294,14 @@ private[spark] object Utils extends Logging { } try { dir = new File(root, namePrefix + "-" + UUID.randomUUID.toString) -if (dir.exists() || !dir.mkdirs()) { +if (!dir.mkdirs()) { dir = null } +if (dir.exists()) { + logInfo(s"$dir exists,can't create a new directory.") + dir = null + return dir.getCanonicalFile --- End diff -- And here it looks always NPE. --- 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 #18838: [SPARK-21632] There is no need to make attempts f...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/18838#discussion_r131326197 --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala --- @@ -294,9 +294,14 @@ private[spark] object Utils extends Logging { } try { dir = new File(root, namePrefix + "-" + UUID.randomUUID.toString) -if (dir.exists() || !dir.mkdirs()) { --- End diff -- I think we intendedly try to return a non-existent and a newly created directory. It looks not a hot path and performance here looks not a big deal. --- 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 #18838: [SPARK-21632] There is no need to make attempts f...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/18838#discussion_r131326239 --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala --- @@ -294,9 +294,14 @@ private[spark] object Utils extends Logging { } try { dir = new File(root, namePrefix + "-" + UUID.randomUUID.toString) -if (dir.exists() || !dir.mkdirs()) { +if (!dir.mkdirs()) { dir = null } +if (dir.exists()) { --- End diff -- I guess it throws NPE if we failed to create a dir? --- 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 #18838: [SPARK-21632] There is no need to make attempts f...
GitHub user liu-zhaokun opened a pull request: https://github.com/apache/spark/pull/18838 [SPARK-21632] There is no need to make attempts for createDirectory if the dir had existed [https://issues.apache.org/jira/browse/SPARK-21632](https://issues.apache.org/jira/browse/SPARK-21632) There is no need to make attempts for createDirectory if the dir had existed.So I think we should log it,and Jump out of the loop You can merge this pull request into a Git repository by running: $ git pull https://github.com/liu-zhaokun/spark master08040833 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/18838.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 #18838 commit e3e36a1b920890f2e1873790775d4327cf9ee4fa Author: liuzhaokun Date: 2017-08-04T01:39:51Z [SPARK-21632] There is no need to make attempts for createDirectory if the dir had existed --- 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