Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Mar 1, 2016

What changes were proposed in this pull request?

In order to tell OutputStream that the task has failed or not, we should call the failure callbacks BEFORE calling writer.close().

How was this patch tested?

Added new unit tests.

@davies
Copy link
Contributor Author

davies commented Mar 1, 2016

cc @rxin @yhuai

@SparkQA
Copy link

SparkQA commented Mar 1, 2016

Test build #52255 has finished for PR 11450 at commit 64786b5.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 2, 2016

Test build #52258 has finished for PR 11450 at commit abfa099.

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

@rxin
Copy link
Contributor

rxin commented Mar 2, 2016

Thanks for doing this. I took a look and I think we also need to update a few of the places in WriterContainer.scala, because the SQL file-based data sources don't actually go through this pair rdd path.

@davies
Copy link
Contributor Author

davies commented Mar 2, 2016

@rxin Added.

* exception from the original `out.write` call.
*/
def tryWithSafeFinallyAndFailureCallbacks[T](block: => T)(finallyBlock: => Unit): T = {
// It would be nice to find a method on Try that did this
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this comment

@rxin
Copy link
Contributor

rxin commented Mar 2, 2016

LGTM pending tests.

Can you just remove that comment when you merge it?

@SparkQA
Copy link

SparkQA commented Mar 2, 2016

Test build #52335 has finished for PR 11450 at commit e9c60a8.

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

@davies
Copy link
Contributor Author

davies commented Mar 2, 2016

Merging this into master (comments removed), will cherry-picked into 1.6 later.

@asfgit asfgit closed this in b5a59a0 Mar 2, 2016
@SparkQA
Copy link

SparkQA commented Mar 3, 2016

Test build #52341 has finished for PR 11450 at commit 8f09258.

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

asfgit pushed a commit that referenced this pull request Mar 3, 2016
In order to tell OutputStream that the task has failed or not, we should call the failure callbacks BEFORE calling writer.close().

Added new unit tests.

Author: Davies Liu <[email protected]>

Closes #11450 from davies/callback.
roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
## What changes were proposed in this pull request?

In order to tell OutputStream that the task has failed or not, we should call the failure callbacks BEFORE calling writer.close().

## How was this patch tested?

Added new unit tests.

Author: Davies Liu <[email protected]>

Closes apache#11450 from davies/callback.
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