You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sarutak <gi...@git.apache.org> on 2014/07/24 21:24:49 UTC
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
GitHub user sarutak opened a pull request:
https://github.com/apache/spark/pull/1580
[SPARK-2671] BlockObjectWriter should create parent directory when the directory doesn't exist
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sarutak/spark SPARK-2671
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1580.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 #1580
----
commit 43e3aad847ebe040eca5aac93386847a36009b8d
Author: Kousuke Saruta <sa...@oss.nttdata.co.jp>
Date: 2014-07-24T19:23:40Z
Modified DiskBlockObjectWriter#open to be able to recoer from non-existent parent directory
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50230269
I think, it's sometimes depends on the kinds of error.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50065852
Can one of the admins verify this patch?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by colorant <gi...@git.apache.org>.
Github user colorant commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50098663
Just curious, In which case this non exist directory might happen in current spark code path? ;)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50215042
Parent directories named spark-local-* were deleted before shuffle, you can see the stack trace like this.
java.io.FileOutputStream.open(Native Method)
java.io.FileOutputStream.<init>(FileOutputStream.java:221)
org.apache.spark.storage.DiskBlockObjectWriter.open(BlockObjectWriter.scala:116)
org.apache.spark.storage.DiskBlockObjectWriter.write(BlockObjectWriter.scala:177)
org.apache.spark.shuffle.hash.HashShuffleWriter$$anonfun$write$1.apply(HashShuffleWriter.scala:59)
org.apache.spark.shuffle.hash.HashShuffleWriter$$anonfun$write$1.apply(HashShuffleWriter.scala:57)
scala.collection.Iterator$class.foreach(Iterator.scala:727)
org.apache.spark.util.collection.AppendOnlyMap$$anon$1.foreach(AppendOnlyMap.scala:159)
org.apache.spark.shuffle.hash.HashShuffleWriter.write(HashShuffleWriter.scala:57)
org.apache.spark.scheduler.ShuffleMapTask.runTask(ShuffleMapTask.scala:147)
org.apache.spark.scheduler.ShuffleMapTask.runTask(ShuffleMapTask.scala:97)
org.apache.spark.scheduler.Task.run(Task.scala:51)
org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:189)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
java.lang.Thread.run(Thread.java:745)
As you say, getFile creates parent directories but I think this path doesn't call getFile.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by sarutak <gi...@git.apache.org>.
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50174987
@colorant @jerryshao Normally, non exist directory doesn't happen. I aim at recovering rom operational miss or some accidents.
In my experience, system operator sometimes do what we can't imagine. Accidentally, operator may delete directories.
And some hand-made scripts written by user may delete directories because of bug.
I know, those cases happen rarely but can happen.
IMO, if it can recover, Spark should try to recover.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by sarutak <gi...@git.apache.org>.
Github user sarutak closed the pull request at:
https://github.com/apache/spark/pull/1580
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50184666
But the point above was that the code that creates this object goes through DiskBlockManager.getFile, which already creates any non-existent directories. So I don't think this will be a problem, unless a directory is deleted exactly in the instant when we return a File and we start writing.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50246267
Actually we have also seen this happen multiple times.
A few have them have been fixed, but not all have been identified.
For example, there is incorrect DCL for directory creation in spark.
The tricky bit is preventing creation of directories when shutdown is
happening (either via exit or via driver message).
The consolidated shuffle bug fix includes some attempts to resolve it : but
since we could not identify/nail down all cases.
We introduced something similar : retry in case there is a
FileNotFoundException creating file stream but prevent in case we were
going to shutdown (so that we dont mess up shutdown hooks trying to remove
directories !).
Regards,
Mridul
On Fri, Jul 25, 2014 at 11:41 PM, Matei Zaharia <no...@github.com>
wrote:
> But the point above was that the code that creates this object goes
> through DiskBlockManager.getFile, which already creates any non-existent
> directories. So I don't think this will be a problem, unless a directory is
> deleted exactly in the instant when we return a File and we start writing.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/1580#issuecomment-50184666>.
>
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50226383
IMHO, I think it would be better to let it fail when this situation happens, fast fail is better than trying to recover, I think :).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: [SPARK-2671] BlockObjectWriter should create p...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the pull request:
https://github.com/apache/spark/pull/1580#issuecomment-50109613
Usually File is gotten through `DiskBlockManger/getFile()`, so parent directory will be create in `getFile()`, I think you needn't worry about the parent directory if you follow the Spark's convention.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---