[SPARK-7503][YARN] Resources in .sparkStaging directory can't be cleaned up on error #6026

sarutak · 2015-05-09T10:06:57Z

When we run applications on YARN with cluster mode, uploaded resources on .sparkStaging directory can't be cleaned up in case of failure of uploading local resources.

You can see this issue by running following command.

bin/spark-submit --master yarn --deploy-mode cluster --class <someClassName> <non-existing-jar>

AmplabJenkins · 2015-05-09T10:07:10Z

Merged build triggered.

AmplabJenkins · 2015-05-09T10:07:17Z

Merged build started.

SparkQA · 2015-05-09T10:09:05Z

Test build #32299 has started for PR 6026 at commit f61071b.

srowen · 2015-05-09T10:50:14Z

yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala

+      localResources = prepareLocalResources(appStagingDir)
+    } catch {
+      case e: Throwable =>
+        var stagingDirPath: Path = null


I don't think this variable is needed? appStagingDir can be used everywhere except where it needs to be wrapped for FileSystem. Can localResources remain a val and receive the value of the try block?

srowen · 2015-05-09T10:52:23Z

To be more specific, you are pointing out that the staging dir is set up before the AM runs. The AM also does this cleanup, but if it fails to start, nothing cleans this up.

I agree, but this it a band-aid that doesn't catch most error cases and would still leave this dir lying around.

Can the staging files be set up much later in this initialization? that would greatly narrow the problem

SparkQA · 2015-05-09T11:25:17Z

Test build #32299 has finished for PR 6026 at commit f61071b.

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

AmplabJenkins · 2015-05-09T11:25:23Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-09T11:25:23Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32299/
Test FAILed.

sarutak · 2015-05-10T10:06:54Z

Thank you for your comment @srowen .
Yeah, as you mentioned, staging dir is not cleaned up when ApplicationMaster fails to start.
But I think it's difficult to setup local resources after we know that ApplicationMaster start.
I'll investigate another way.

srowen · 2015-05-10T20:33:09Z

At least can it be one of the last things that happens before starting? that would tighten this up.

tgravescs · 2015-05-11T14:42:42Z

How about we just wrap submitApplication is a try catch block and check to see if we need to cleanup the staging directory if it fails. that way it should cover more failure scenarios.

You have to do certain things in certain order in order to submit. We create the launch context (which includes preparing the resources, create the submission context and submit to yarn. Personally I like having the resources setup before setting it in the context.

vanzin · 2015-05-12T17:04:10Z

How about we just wrap submitApplication is a try catch block and check to see if we need to cleanup the staging directory if it fails.

+1

sarutak · 2015-05-12T17:32:07Z

Thanks for all of your advice.
Yeah, wrapping submitApplication by try/catch block is good idea.
But, what if the last app attempt fails before ApplicationMaster starts and after submitApplication returns successfully.
Should we check the host name of last ApplicationMaster by ApplicationReport#getHost ? In this case, the host name should be "N/A".

vanzin · 2015-05-12T17:36:31Z

But, what if the last app attempt fails before ApplicationMaster starts and after submitApplication returns successfully.

That requires the launcher to still be around to be fixed, which is not guaranteed (use can just ctrl-c out of the launcher, it's not required after the app starts).

If the launcher is still around, though, it can probably just do a final check for the presence of the staging dir after the app finishes, regardless of final status.

…uploaded-resources-on-error

sarutak · 2015-05-13T02:12:17Z

O.K, I'll wrap submitApplication with a try/catch block.

…lete resources on error

AmplabJenkins · 2015-05-13T02:47:11Z

Merged build triggered.

AmplabJenkins · 2015-05-13T02:47:18Z

Merged build started.

SparkQA · 2015-05-13T02:47:59Z

Test build #32570 has started for PR 6026 at commit 882f921.

SparkQA · 2015-05-13T02:49:36Z

Test build #32570 has finished for PR 6026 at commit 882f921.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- final class OneVsRest extends Estimator[OneVsRestModel] with OneVsRestParams
- decisionTreeCode = '''class DecisionTreeParams(Params):
- class DecisionTreeParams(Params):
- class LinearRegression(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol, HasMaxIter,
- class LinearRegressionModel(JavaModel):
- class TreeRegressorParams(object):
- class RandomForestParams(object):
- class GBTParams(object):
- class DecisionTreeRegressor(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol,
- class DecisionTreeRegressionModel(JavaModel):
- class RandomForestRegressor(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol, HasSeed,
- class RandomForestRegressionModel(JavaModel):
- class GBTRegressor(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol, HasMaxIter,
- class GBTRegressionModel(JavaModel):
- s"FileOutputCommitter or its subclass is expected, but got a $
- trait FSBasedRelationProvider
- abstract class OutputWriter

AmplabJenkins · 2015-05-13T02:49:37Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-13T02:49:38Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32570/
Test FAILed.

AmplabJenkins · 2015-05-13T02:52:11Z

Merged build triggered.

AmplabJenkins · 2015-05-13T02:52:18Z

Merged build started.

SparkQA · 2015-05-13T02:52:36Z

Test build #32571 has started for PR 6026 at commit caef9f4.

SparkQA · 2015-05-13T03:02:16Z

Test build #32571 has finished for PR 6026 at commit caef9f4.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-05-13T03:02:20Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-13T03:02:20Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32571/
Test FAILed.

sarutak · 2015-05-13T03:20:29Z

retest this please.

AmplabJenkins · 2015-05-13T03:22:10Z

Merged build triggered.

AmplabJenkins · 2015-05-13T03:22:18Z

Merged build started.

SparkQA · 2015-05-13T03:24:11Z

Test build #32573 has started for PR 6026 at commit caef9f4.

SparkQA · 2015-05-13T05:06:49Z

Test build #32573 has finished for PR 6026 at commit caef9f4.

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

AmplabJenkins · 2015-05-13T05:06:53Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-13T05:06:53Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32573/
Test PASSed.

srowen · 2015-05-13T15:02:13Z

yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala

+      appId
+    } catch {
+      case e: Throwable =>
+        if (appId != null) {


I suppose there's no great way to share this with similar code in ApplicationMaster.scala? maybe not. This could be made into a private method, as it is in the other similar code block, but it's not a big deal. LGTM.

…aned up on error When we run applications on YARN with cluster mode, uploaded resources on .sparkStaging directory can't be cleaned up in case of failure of uploading local resources. You can see this issue by running following command. ``` bin/spark-submit --master yarn --deploy-mode cluster --class <someClassName> <non-existing-jar> ``` Author: Kousuke Saruta <[email protected]> Closes #6026 from sarutak/delete-uploaded-resources-on-error and squashes the following commits: caef9f4 [Kousuke Saruta] Fixed style 882f921 [Kousuke Saruta] Wrapped Client#submitApplication with try/catch blocks in order to delete resources on error 1786ca4 [Kousuke Saruta] Merge branch 'master' of https://github.com/apache/spark into delete-uploaded-resources-on-error f61071b [Kousuke Saruta] Fixed cleanup problem (cherry picked from commit c64ff80) Signed-off-by: Sean Owen <[email protected]>

…aned up on error When we run applications on YARN with cluster mode, uploaded resources on .sparkStaging directory can't be cleaned up in case of failure of uploading local resources. You can see this issue by running following command. ``` bin/spark-submit --master yarn --deploy-mode cluster --class <someClassName> <non-existing-jar> ``` Author: Kousuke Saruta <[email protected]> Closes apache#6026 from sarutak/delete-uploaded-resources-on-error and squashes the following commits: caef9f4 [Kousuke Saruta] Fixed style 882f921 [Kousuke Saruta] Wrapped Client#submitApplication with try/catch blocks in order to delete resources on error 1786ca4 [Kousuke Saruta] Merge branch 'master' of https://github.com/apache/spark into delete-uploaded-resources-on-error f61071b [Kousuke Saruta] Fixed cleanup problem

Fixed cleanup problem

f61071b

srowen reviewed May 9, 2015
View reviewed changes

Merge branch 'master' of https://github.com/apache/spark into delete-…

1786ca4

…uploaded-resources-on-error

Wrapped Client#submitApplication with try/catch blocks in order to de…

882f921

…lete resources on error

Fixed style

caef9f4

srowen reviewed May 13, 2015
View reviewed changes

asfgit closed this in c64ff80 May 15, 2015

sarutak deleted the delete-uploaded-resources-on-error branch May 18, 2015 01:52

[SPARK-7503][YARN] Resources in .sparkStaging directory can't be cleaned up on error #6026

[SPARK-7503][YARN] Resources in .sparkStaging directory can't be cleaned up on error #6026

Conversation

sarutak commented May 9, 2015

AmplabJenkins commented May 9, 2015

AmplabJenkins commented May 9, 2015

SparkQA commented May 9, 2015

srowen May 9, 2015

Choose a reason for hiding this comment

srowen commented May 9, 2015

SparkQA commented May 9, 2015

AmplabJenkins commented May 9, 2015

AmplabJenkins commented May 9, 2015

sarutak commented May 10, 2015

srowen commented May 10, 2015

tgravescs commented May 11, 2015

vanzin commented May 12, 2015

sarutak commented May 12, 2015

vanzin commented May 12, 2015

sarutak commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

SparkQA commented May 13, 2015

SparkQA commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

SparkQA commented May 13, 2015

SparkQA commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

sarutak commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

SparkQA commented May 13, 2015

SparkQA commented May 13, 2015

AmplabJenkins commented May 13, 2015

AmplabJenkins commented May 13, 2015

srowen May 13, 2015

Choose a reason for hiding this comment