[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got #17768

HyukjinKwon · 2017-04-26T05:02:43Z

What changes were proposed in this pull request?

This PR proposes to throw an exception with better message rather than ArrayIndexOutOfBoundsException when temp directories could not be created.

Running the commands below:

./bin/spark-shell --conf spark.local.dir=/NONEXISTENT_DIR_ONE,/NONEXISTENT_DIR_TWO

produces ...

Before

Exception in thread "main" java.lang.ExceptionInInitializerError
        ...
Caused by: java.lang.ArrayIndexOutOfBoundsException: 0
        ...

After

Exception in thread "main" java.lang.ExceptionInInitializerError
        ...
Caused by: java.io.IOException: Failed to get a temp directory under [/NONEXISTENT_DIR_ONE,/NONEXISTENT_DIR_TWO].
        ...

How was this patch tested?

Unit tests in LocalDirsSuite.scala.

…when temp directories could not be got/created

SparkQA · 2017-04-26T05:07:35Z

Test build #76171 has started for PR 17768 at commit b9ce248.

SparkQA · 2017-04-26T05:12:37Z

Test build #76172 has started for PR 17768 at commit bf21e3b.

HyukjinKwon · 2017-04-26T05:16:16Z

This actually also happens when the directory exists but the user does not have the permission.

HyukjinKwon · 2017-04-26T05:47:51Z

@JoshRosen, could you take a look and see if it makes sense?

srowen

Seems reasonable to me vs failing with a less clear exception

srowen · 2017-04-26T08:58:53Z

core/src/test/scala/org/apache/spark/storage/LocalDirsSuite.scala

+    val message = intercept[IOException] {
+      Utils.getLocalDir(conf)
+    }.getMessage
+    assert(message ===


I would only suggest not testing the exact message here, but it's not a big deal. Maybe testing for the existence of the paths.

HyukjinKwon · 2017-04-26T09:34:08Z

Thank you @srowen.

SparkQA · 2017-04-26T11:44:21Z

Test build #76175 has finished for PR 17768 at commit 0f55c49.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2017-04-26T11:53:41Z

retest this please

HyukjinKwon · 2017-04-26T14:23:03Z

retest this please

SparkQA · 2017-04-26T16:07:36Z

Test build #76177 has finished for PR 17768 at commit 0f55c49.

This patch fails from timeout after a configured wait of `250m`.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-26T16:33:12Z

Test build #76186 has finished for PR 17768 at commit 0f55c49.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-26T20:25:37Z

Test build #3679 has finished for PR 17768 at commit 0f55c49.

This patch fails from timeout after a configured wait of `250m`.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2017-04-26T20:57:42Z

retest this please

SparkQA · 2017-04-26T23:11:02Z

Test build #76199 has finished for PR 17768 at commit 0f55c49.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2017-04-27T03:47:53Z

retest this please

HyukjinKwon · 2017-04-27T03:50:17Z

Current status: I don't know how using

val dirs = Array.empty[Int]
dirs.headOption.getOrElse {
  throw new Exception("")
}

instead of

val dirs = Array.empty[Int]
dirs(0)

could make the tests failed (given the observations above).
In the last commit, I gave a shot it with if-else as below:

val dirs = Array.empty[Int]
if (dirs.nonEmpty) {
  dirs(0)
} else {
  throw new Exception("")
}

in order to make sure logically this should not make other tests flaky.

SparkQA · 2017-04-27T04:44:54Z

Test build #76213 has finished for PR 17768 at commit b5c9839.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-27T05:52:41Z

Test build #76215 has finished for PR 17768 at commit b5c9839.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2017-04-27T12:22:56Z

It looks we should clean up the cached local dirs after the test. The newly added test sets it empty path and it seems affecting the tests afterwards. I changed it back to the original proposal with the cleanup.

HyukjinKwon · 2017-04-27T15:47:01Z

retest this please

SparkQA · 2017-04-27T16:32:37Z

Test build #76225 has finished for PR 17768 at commit 751f161.

This patch fails from timeout after a configured wait of `250m`.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-27T18:35:27Z

Test build #76231 has finished for PR 17768 at commit 751f161.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2017-04-28T07:54:26Z

Merged to master/2.2

… could not be got ## What changes were proposed in this pull request? This PR proposes to throw an exception with better message rather than `ArrayIndexOutOfBoundsException` when temp directories could not be created. Running the commands below: ```bash ./bin/spark-shell --conf spark.local.dir=/NONEXISTENT_DIR_ONE,/NONEXISTENT_DIR_TWO ``` produces ... **Before** ``` Exception in thread "main" java.lang.ExceptionInInitializerError ... Caused by: java.lang.ArrayIndexOutOfBoundsException: 0 ... ``` **After** ``` Exception in thread "main" java.lang.ExceptionInInitializerError ... Caused by: java.io.IOException: Failed to get a temp directory under [/NONEXISTENT_DIR_ONE,/NONEXISTENT_DIR_TWO]. ... ``` ## How was this patch tested? Unit tests in `LocalDirsSuite.scala`. Author: hyukjinkwon <gurwls223@gmail.com> Closes #17768 from HyukjinKwon/throws-temp-dir-exception. (cherry picked from commit 8c911ad) Signed-off-by: Sean Owen <sowen@cloudera.com>

HyukjinKwon added 2 commits April 26, 2017 13:56

Throws a proper exception rather than ArrayIndexOutOfBoundsException …

4500c2f

…when temp directories could not be got/created

Do not load default configuration to SparkConf

b9ce248

Fix typos in path names

bf21e3b

HyukjinKwon changed the title ~~[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got/created (rather than ArrayIndexOutOfBoundsException)~~ [SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got Apr 26, 2017

srowen approved these changes Apr 26, 2017

View reviewed changes

Address comment

0f55c49

HyukjinKwon changed the title ~~[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got~~ [WIP][SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got Apr 26, 2017

Try with if-else

b5c9839

HyukjinKwon changed the title ~~[WIP][SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got~~ [SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got Apr 27, 2017

HyukjinKwon changed the title ~~[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got~~ [WIP][SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got Apr 27, 2017

Clean up cached local dirs in tests

751f161

HyukjinKwon changed the title ~~[WIP][SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got~~ [SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got Apr 27, 2017

asfgit closed this in 8c911ad Apr 28, 2017

HyukjinKwon deleted the throws-temp-dir-exception branch January 2, 2018 03:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got #17768

[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got #17768

HyukjinKwon commented Apr 26, 2017 •

edited

Loading

SparkQA commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

srowen left a comment

srowen Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

SparkQA commented Apr 26, 2017

SparkQA commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 27, 2017

HyukjinKwon commented Apr 27, 2017 •

edited

Loading

SparkQA commented Apr 27, 2017

SparkQA commented Apr 27, 2017

HyukjinKwon commented Apr 27, 2017 •

edited

Loading

HyukjinKwon commented Apr 27, 2017

SparkQA commented Apr 27, 2017

SparkQA commented Apr 27, 2017

srowen commented Apr 28, 2017

[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got #17768

[SPARK-20465][CORE] Throws a proper exception when any temp directory could not be got #17768

Conversation

HyukjinKwon commented Apr 26, 2017 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

srowen left a comment

Choose a reason for hiding this comment

srowen Apr 26, 2017

Choose a reason for hiding this comment

HyukjinKwon commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

SparkQA commented Apr 26, 2017

SparkQA commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 26, 2017

SparkQA commented Apr 26, 2017

HyukjinKwon commented Apr 27, 2017

HyukjinKwon commented Apr 27, 2017 • edited Loading

SparkQA commented Apr 27, 2017

SparkQA commented Apr 27, 2017

HyukjinKwon commented Apr 27, 2017 • edited Loading

HyukjinKwon commented Apr 27, 2017

SparkQA commented Apr 27, 2017

SparkQA commented Apr 27, 2017

srowen commented Apr 28, 2017

HyukjinKwon commented Apr 26, 2017 •

edited

Loading

HyukjinKwon commented Apr 27, 2017 •

edited

Loading

HyukjinKwon commented Apr 27, 2017 •

edited

Loading