Skip to content

Conversation

@zsxwing
Copy link
Member

@zsxwing zsxwing commented Feb 10, 2017

What changes were proposed in this pull request?

When a query uses a temp checkpoint dir, it's better to delete it if it's stopped without errors.

How was this patch tested?

New unit tests.

@SparkQA
Copy link

SparkQA commented Feb 10, 2017

Test build #72680 has finished for PR 16880 at commit 22e02b9.

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

@SparkQA
Copy link

SparkQA commented Feb 10, 2017

Test build #72682 has finished for PR 16880 at commit e5fa7d7.

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

val fs = checkpointPath.getFileSystem(sparkSession.sessionState.newHadoopConf())
fs.delete(checkpointPath, true)
} catch {
case e: IOException =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so, but can we get any other exception here? It could be bad if it bubbles up to an uncaught exception handler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I changed it to NonFatal.

@brkyvz
Copy link
Contributor

brkyvz commented Feb 10, 2017

LGTM. I left one question, but then checked and found the answer.

@SparkQA
Copy link

SparkQA commented Feb 11, 2017

Test build #72721 has finished for PR 16880 at commit cae981f.

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

@brkyvz
Copy link
Contributor

brkyvz commented Feb 13, 2017

LGTM! Merging to master and 2.1.

asfgit pushed a commit that referenced this pull request Feb 13, 2017
…thout errors

## What changes were proposed in this pull request?

When a query uses a temp checkpoint dir, it's better to delete it if it's stopped without errors.

## How was this patch tested?

New unit tests.

Author: Shixiong Zhu <[email protected]>

Closes #16880 from zsxwing/delete-temp-checkpoint.

(cherry picked from commit 3dbff9b)
Signed-off-by: Burak Yavuz <[email protected]>
@asfgit asfgit closed this in 3dbff9b Feb 13, 2017
@zsxwing zsxwing deleted the delete-temp-checkpoint branch February 13, 2017 19:59
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…thout errors

## What changes were proposed in this pull request?

When a query uses a temp checkpoint dir, it's better to delete it if it's stopped without errors.

## How was this patch tested?

New unit tests.

Author: Shixiong Zhu <[email protected]>

Closes apache#16880 from zsxwing/delete-temp-checkpoint.
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