Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

Streaming execution has a list of exceptions that means interruption, and handle them specially. WriteToDataSourceV2Exec should also respect this list and not wrap them with SparkException.

How was this patch tested?

existing test.

@cloud-fan
Copy link
Contributor Author

cc @zsxwing @jose-torres @mgaido91

@SparkQA
Copy link

SparkQA commented Feb 14, 2018

Test build #87437 has finished for PR 20605 at commit b91a3af.

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

@gatorsmile
Copy link
Member

Also cc @tdas @marmbrus

@mgaido91
Copy link
Contributor

This seems good to me. My only concern is that we still have to rely/maintain a list of exceptions thrown when the stop method is called... This approach seems not very robust to me, but I don't have a better suggestion.

@jose-torres
Copy link
Contributor

LGTM also. I'll still take it as an action item to try and think of a cleaner way to handle stopping a stream.

@zsxwing
Copy link
Member

zsxwing commented Feb 15, 2018

LGTM

@cloud-fan
Copy link
Contributor Author

thanks, merging to master/2.3! Since it's a bug fix so I included it in branch 2.3, please let me know if you have other concerns, cc @sameeragarwal @marmbrus

asfgit pushed a commit that referenced this pull request Feb 15, 2018
…row interruption exceptions directly

## What changes were proposed in this pull request?

Streaming execution has a list of exceptions that means interruption, and handle them specially. `WriteToDataSourceV2Exec` should also respect this list and not wrap them with `SparkException`.

## How was this patch tested?

existing test.

Author: Wenchen Fan <[email protected]>

Closes #20605 from cloud-fan/write.

(cherry picked from commit f38c760)
Signed-off-by: Wenchen Fan <[email protected]>
@asfgit asfgit closed this in f38c760 Feb 15, 2018
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.

6 participants