Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-19026]SPARK_LOCAL_DIRS(multiple directories on different disks) cannot be deleted #16439

Closed
wants to merge 3 commits into from

Conversation

zuotingbing
Copy link

@zuotingbing zuotingbing commented Dec 30, 2016

JIRA Issue: https://issues.apache.org/jira/browse/SPARK-19026

SPARK_LOCAL_DIRS (Standalone) can be a comma-separated list of multiple directories on different disks, e.g. SPARK_LOCAL_DIRS=/dir1,/dir2,/dir3, if there is a IOExecption when create sub directory on dir3 , the sub directory which have been created successfully on dir1 and dir2 cannot be deleted anymore when the application finishes.
So we should catch the IOExecption at Utils.createDirectory , otherwise the variable "appDirectories(appId)" which the function maybeCleanupApplication calls will not be set then dir1 and dir2 will not be cleaned up .

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before continuing though, hm, shouldn't this be a fatal error? I don't see a great deal of value in silently ignoring complete failure to write to a configured volume.

Some(appDir.getAbsolutePath())
} catch {
case e: IOException =>
logError(s"${e.getMessage}. Ignoring this directory.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review, i will merge it.

None
}
}.toSeq
if (dirs.length <= 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isEmpty

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i will merge it.

@zuotingbing
Copy link
Author

@srowen i do not think there should be a fatal error since some of SPARK_LOCAL_DIRS can be written successfully, even there is only one of SPARK_LOCAL_DIRS can be written, the application is able to run successfully.

@srowen
Copy link
Member

srowen commented Jan 4, 2017

Would it be common to configure some directories for use but not mind if they're unwritable? It seems like this is an explicit setting and if the explicit setting can't be honored, feels like an error to me. What's the use case?

@vanzin
Copy link
Contributor

vanzin commented Jan 4, 2017

but not mind if they're unwritable

I guess you wouldn't configure an unwritable directory, but it can become unwritable for other reasons (e.g. disk fills up).

@zuotingbing
Copy link
Author

zuotingbing commented Jan 5, 2017

@srowen Thanks for your advice, as @vanzin said I did not configure an unwritable directory, but it can become unwritable for other reasons (e.g. disk fills up or disk suddenly become damaged etc.).

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see the logic. Let's test it.

@SparkQA
Copy link

SparkQA commented Jan 5, 2017

Test build #3521 has finished for PR 16439 at commit 61cf467.

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

}
}.toSeq
if (dirs.isEmpty) {
throw new IOException("None subfolder can be created in " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"No subfolder..."

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks vanzin. i will commit it.

}.toSeq
if (dirs.isEmpty) {
throw new IOException("None subfolder can be created in " +
s"${Utils.getOrCreateLocalRootDirs(conf).mkString(",")}.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd stash Utils.getOrCreateLocalRootDirs(conf) in a variable since it's also used above in L449.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks vanzin. i will commit it

@vanzin
Copy link
Contributor

vanzin commented Jan 6, 2017

retest this please

@SparkQA
Copy link

SparkQA commented Jan 6, 2017

Test build #70993 has finished for PR 16439 at commit 61cf467.

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

@srowen
Copy link
Member

srowen commented Jan 8, 2017

Merged to master

@asfgit asfgit closed this in cd1d00a Jan 8, 2017
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Jan 9, 2017
…s) cannot be deleted

JIRA Issue: https://issues.apache.org/jira/browse/SPARK-19026

SPARK_LOCAL_DIRS (Standalone) can  be a comma-separated list of multiple directories on different disks, e.g. SPARK_LOCAL_DIRS=/dir1,/dir2,/dir3, if there is a IOExecption when create sub directory on dir3 , the sub directory which have been created successfully on dir1 and dir2 cannot be deleted anymore when the application finishes.
So we should catch the IOExecption at Utils.createDirectory  , otherwise the variable "appDirectories(appId)" which the function maybeCleanupApplication calls will not be set then dir1 and dir2 will not be cleaned up .

Author: zuotingbing <zuo.tingbing9@zte.com.cn>

Closes apache#16439 from zuotingbing/master.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…s) cannot be deleted

JIRA Issue: https://issues.apache.org/jira/browse/SPARK-19026

SPARK_LOCAL_DIRS (Standalone) can  be a comma-separated list of multiple directories on different disks, e.g. SPARK_LOCAL_DIRS=/dir1,/dir2,/dir3, if there is a IOExecption when create sub directory on dir3 , the sub directory which have been created successfully on dir1 and dir2 cannot be deleted anymore when the application finishes.
So we should catch the IOExecption at Utils.createDirectory  , otherwise the variable "appDirectories(appId)" which the function maybeCleanupApplication calls will not be set then dir1 and dir2 will not be cleaned up .

Author: zuotingbing <zuo.tingbing9@zte.com.cn>

Closes apache#16439 from zuotingbing/master.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants