Skip to content

Conversation

@viadea
Copy link
Contributor

@viadea viadea commented Aug 10, 2015

Spark streaming deletes the temp file and backup files without checking if they exist or not

Spark streaming deletes the temp file and backup files without checking if they exist or not
@tdas
Copy link
Contributor

tdas commented Aug 10, 2015

Please set the PR title as the problem or fix this PR has. Something like "Check if file exists before deleting temporary files."

@tdas
Copy link
Contributor

tdas commented Aug 10, 2015

LGTM. Will merge when tests pass and after you update the title.

@tdas
Copy link
Contributor

tdas commented Aug 10, 2015

This is ok to test

@viadea viadea changed the title [SPARK-9801][Streaming] [SPARK-9801][Streaming]Check if file exists before deleting temporary files. Aug 10, 2015
@viadea
Copy link
Contributor Author

viadea commented Aug 10, 2015

Done changing the title.

@viadea
Copy link
Contributor Author

viadea commented Aug 10, 2015

Hi Team,

I forgot to add one check, I wanted to add the check for backup file also:
fs.delete(backupFile, true) // just in case it exists
Should I file a new PR or edit this PR?

@viadea
Copy link
Contributor Author

viadea commented Aug 10, 2015

Hi Team,

I just added another commit to this PR.

@SparkQA
Copy link

SparkQA commented Aug 10, 2015

Test build #1424 has finished for PR 8082 at commit 087daf0.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not if this is necessary. These files that are being deleted came from a recent listing, so they are very very likely to exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agreed and I just made the change. Can you start another build and test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually from the error message in Spark-9801, the only 2 checks I wanted to add were:
tempFile
backupFile

So I am fine to ignore the check for "file". Also for performance consideration.

@SparkQA
Copy link

SparkQA commented Aug 11, 2015

Test build #1425 has finished for PR 8082 at commit fd143f2.

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

@SparkQA
Copy link

SparkQA commented Aug 11, 2015

Test build #40347 has finished for PR 8082 at commit 242d05f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class SpecificMutableProjection extends $

@tdas
Copy link
Contributor

tdas commented Aug 11, 2015

All right, test on the latest commit has passed. I am merging this to master and branch 1.5. Thanks!

asfgit pushed a commit that referenced this pull request Aug 11, 2015
…ry files.

Spark streaming deletes the temp file and backup files without checking if they exist or not

Author: Hao Zhu <[email protected]>

Closes #8082 from viadea/master and squashes the following commits:

242d05f [Hao Zhu] [SPARK-9801][Streaming]No need to check the existence of those files
fd143f2 [Hao Zhu] [SPARK-9801][Streaming]Check if backupFile exists before deleting backupFile files.
087daf0 [Hao Zhu] SPARK-9801

(cherry picked from commit 3c9802d)
Signed-off-by: Tathagata Das <[email protected]>
asfgit pushed a commit that referenced this pull request Aug 11, 2015
…ry files.

Spark streaming deletes the temp file and backup files without checking if they exist or not

Author: Hao Zhu <[email protected]>

Closes #8082 from viadea/master and squashes the following commits:

242d05f [Hao Zhu] [SPARK-9801][Streaming]No need to check the existence of those files
fd143f2 [Hao Zhu] [SPARK-9801][Streaming]Check if backupFile exists before deleting backupFile files.
087daf0 [Hao Zhu] SPARK-9801

(cherry picked from commit 3c9802d)
Signed-off-by: Tathagata Das <[email protected]>
asfgit pushed a commit that referenced this pull request Aug 11, 2015
…ry files.

Spark streaming deletes the temp file and backup files without checking if they exist or not

Author: Hao Zhu <[email protected]>

Closes #8082 from viadea/master and squashes the following commits:

242d05f [Hao Zhu] [SPARK-9801][Streaming]No need to check the existence of those files
fd143f2 [Hao Zhu] [SPARK-9801][Streaming]Check if backupFile exists before deleting backupFile files.
087daf0 [Hao Zhu] SPARK-9801

(cherry picked from commit 3c9802d)
Signed-off-by: Tathagata Das <[email protected]>
@tdas
Copy link
Contributor

tdas commented Aug 11, 2015

Actually, I merged it to 1.4 and 1.3 branches as well.

@asfgit asfgit closed this in 3c9802d Aug 11, 2015
CodingCat pushed a commit to CodingCat/spark that referenced this pull request Aug 17, 2015
…ry files.

Spark streaming deletes the temp file and backup files without checking if they exist or not

Author: Hao Zhu <[email protected]>

Closes apache#8082 from viadea/master and squashes the following commits:

242d05f [Hao Zhu] [SPARK-9801][Streaming]No need to check the existence of those files
fd143f2 [Hao Zhu] [SPARK-9801][Streaming]Check if backupFile exists before deleting backupFile files.
087daf0 [Hao Zhu] SPARK-9801
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants