Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-19113][SS][Tests]Set UncaughtExceptionHandler in onQueryStarted to ensure catching fatal errors during query initialization #16492

Closed
wants to merge 3 commits into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Jan 6, 2017

What changes were proposed in this pull request?

StreamTest sets UncaughtExceptionHandler after starting the query now. It may not be able to catch fatal errors during query initialization. This PR uses onQueryStarted callback to fix it.

How was this patch tested?

Jenkins

@SparkQA
Copy link

SparkQA commented Jan 7, 2017

Test build #70995 has finished for PR 16492 at commit 080a269.

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

@SparkQA
Copy link

SparkQA commented Jan 7, 2017

Test build #70998 has finished for PR 16492 at commit 59a1161.

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

@@ -238,7 +238,7 @@ class StreamSuite extends StreamTest {
}
}

testQuietly("fatal errors from a source should be sent to the user") {
testQuietly("handle fatal errors thrown from the stream thread correctly") {
Copy link
Contributor

Choose a reason for hiding this comment

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

correctly is superfluous and could be removed.

@@ -235,7 +235,10 @@ trait StreamTest extends QueryTest with SharedSQLContext with Timeouts {
*/
def testStream(
_stream: Dataset[_],
outputMode: OutputMode = OutputMode.Append)(actions: StreamAction*): Unit = {
outputMode: OutputMode = OutputMode.Append)(actions: StreamAction*): Unit = synchronized {
// `synchronized` is added to prevent the user from calling `testStream` concurrently because
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate why StreamingQueryListener can be used concurrently? I'm reading the comment: "it does not work because something else does not work". Why? What's wrong with StreamingQueryListener?

@SparkQA
Copy link

SparkQA commented Jan 9, 2017

Test build #71086 has finished for PR 16492 at commit fca0424.

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

@srowen
Copy link
Member

srowen commented Jan 10, 2017

Merged to master

@asfgit asfgit closed this in 3ef183a Jan 10, 2017
asfgit pushed a commit that referenced this pull request Jan 10, 2017
…ed to ensure catching fatal errors during query initialization

## What changes were proposed in this pull request?

StreamTest sets `UncaughtExceptionHandler` after starting the query now. It may not be able to catch fatal errors during query initialization. This PR uses `onQueryStarted` callback to fix it.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes #16492 from zsxwing/SPARK-19113.
@zsxwing
Copy link
Member Author

zsxwing commented Jan 10, 2017

I also cherry-picked to 2.1 since Jenkins also run 2.1 tests.

@zsxwing zsxwing deleted the SPARK-19113 branch January 10, 2017 19:51
asfgit pushed a commit that referenced this pull request Jan 18, 2017
…waitInitialization to avoid breaking tests

## What changes were proposed in this pull request?

#16492 missed one race condition: `StreamExecution.awaitInitialization` may throw fatal errors and fail the test. This PR just ignores `StreamingQueryException` thrown from `awaitInitialization` so that we can verify the exception in the `ExpectFailure` action later. It's fine since `StopStream` or `ExpectFailure` will catch `StreamingQueryException` as well.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes #16567 from zsxwing/SPARK-19113-2.

(cherry picked from commit c050c12)
Signed-off-by: Shixiong Zhu <[email protected]>
ghost pushed a commit to dbtsai/spark that referenced this pull request Jan 18, 2017
…waitInitialization to avoid breaking tests

## What changes were proposed in this pull request?

apache#16492 missed one race condition: `StreamExecution.awaitInitialization` may throw fatal errors and fail the test. This PR just ignores `StreamingQueryException` thrown from `awaitInitialization` so that we can verify the exception in the `ExpectFailure` action later. It's fine since `StopStream` or `ExpectFailure` will catch `StreamingQueryException` as well.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#16567 from zsxwing/SPARK-19113-2.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…ed to ensure catching fatal errors during query initialization

## What changes were proposed in this pull request?

StreamTest sets `UncaughtExceptionHandler` after starting the query now. It may not be able to catch fatal errors during query initialization. This PR uses `onQueryStarted` callback to fix it.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#16492 from zsxwing/SPARK-19113.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…waitInitialization to avoid breaking tests

## What changes were proposed in this pull request?

apache#16492 missed one race condition: `StreamExecution.awaitInitialization` may throw fatal errors and fail the test. This PR just ignores `StreamingQueryException` thrown from `awaitInitialization` so that we can verify the exception in the `ExpectFailure` action later. It's fine since `StopStream` or `ExpectFailure` will catch `StreamingQueryException` as well.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#16567 from zsxwing/SPARK-19113-2.
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…ed to ensure catching fatal errors during query initialization

## What changes were proposed in this pull request?

StreamTest sets `UncaughtExceptionHandler` after starting the query now. It may not be able to catch fatal errors during query initialization. This PR uses `onQueryStarted` callback to fix it.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#16492 from zsxwing/SPARK-19113.
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…waitInitialization to avoid breaking tests

## What changes were proposed in this pull request?

apache#16492 missed one race condition: `StreamExecution.awaitInitialization` may throw fatal errors and fail the test. This PR just ignores `StreamingQueryException` thrown from `awaitInitialization` so that we can verify the exception in the `ExpectFailure` action later. It's fine since `StopStream` or `ExpectFailure` will catch `StreamingQueryException` as well.

## How was this patch tested?

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#16567 from zsxwing/SPARK-19113-2.
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.

4 participants