[GitHub] spark pull request #18838: [SPARK-21632] There is no need to make attempts f...

2017-08-04 Thread liu-zhaokun
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...

2017-08-04 Thread HyukjinKwon
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...

2017-08-04 Thread HyukjinKwon
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...

2017-08-04 Thread HyukjinKwon
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...

2017-08-03 Thread liu-zhaokun
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