[SPARK-30310] [Core] Resolve missing match case in SparkUncaughtExceptionHandler and added tests #26955

tinhto-000 · 2019-12-19T21:53:59Z

What changes were proposed in this pull request?

Added missing match case to SparkUncaughtExceptionHandler, so that it would not halt the process when the exception doesn't match any of the match case statements.
Added log message before halting process. During debugging it wasn't obvious why the Worker process would DEAD (until we set SPARK_NO_DAEMONIZE=1) due to the shell-scripts puts the process into background and essentially absorbs the exit code.
Added SparkUncaughtExceptionHandlerSuite. Basically we create a Spark exception-throwing application with SparkUncaughtExceptionHandler and then check its exit code.

Why are the changes needed?

SPARK-30310, because the process would halt unexpectedly.

How was this patch tested?

All unit tests (mvn test) were ran and OK.

added SparkUncaughtExceptionHandlerSuite

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala

core/src/test/scala/org/apache/spark/util/SparkUncaughtExceptionHandlerSuite.scala

SparkUncaughtExceptionHandlerSuite had wrong name use toBoolean/toString for converting string to/from boolean exception throwers now check args.length before using args(0)

SparkQA · 2019-12-24T15:44:17Z

Test build #4975 has finished for PR 26955 at commit 724e254.

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

srowen · 2019-12-26T16:00:28Z

Jenkins retest this please

SparkQA · 2019-12-26T18:20:27Z

Test build #115817 has finished for PR 26955 at commit 724e254.

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

core/src/test/scala/org/apache/spark/util/SparkUncaughtExceptionHandlerSuite.scala

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala

HeartSaVioR · 2019-12-27T04:45:44Z

cc. @gatorsmile @zsxwing

srowen · 2019-12-31T15:35:40Z

Ping @tinhto-000

use Seq().foreach() to launch the test-cases

HeartSaVioR

LGTM except a nit

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala

HeartSaVioR · 2020-01-09T22:54:51Z

@srowen Would you mind if I ask about revisiting this? Maybe with "ok to test". Thanks in advance!

srowen · 2020-01-09T22:58:47Z

Jenkins, test this please

SparkQA · 2020-01-10T01:26:26Z

Test build #116422 has finished for PR 26955 at commit 3f9e96f.

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

srowen · 2020-01-10T01:29:04Z

Jenkins, test this please

HeartSaVioR · 2020-01-10T01:35:05Z

It's known failure failing consistently from yesterday. I pinged @HyukjinKwon to kindly ask about taking a look (as I don't have a context on pyspark) but it seems to be better to disable the test first as multiple PRs are affected.

HyukjinKwon · 2020-01-10T01:56:39Z

Sorry I missed pings @HeartSaVioR - I am currently stuck at some works :(.

HeartSaVioR · 2020-01-10T01:58:38Z

Oh OK. Never mind about missing ping. What about disabling the test and file an issue then? I've submitted #27158 to disable the test.

SparkQA · 2020-01-10T03:29:31Z

Test build #116438 has finished for PR 26955 at commit 3f9e96f.

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

### What changes were proposed in this pull request? This patch increases the memory limit in the test 'test_memory_limit' from 1m to 8m. Credit to srowen and HyukjinKwon to provide the idea of suspicion and guide how to fix. ### Why are the changes needed? We observed consistent Pyspark test failures on multiple PRs (apache#26955, apache#26201, apache#27064) which block the PR builds whenever the test is included. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Jenkins builds passed in WIP PR (apache#27159) Closes apache#27162 from HeartSaVioR/SPARK-30480. Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

HyukjinKwon · 2020-01-10T06:32:50Z

retest this please

SparkQA · 2020-01-10T08:05:02Z

Test build #116454 has finished for PR 26955 at commit 3f9e96f.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR · 2020-01-10T08:15:48Z

retest this, please

srowen · 2020-01-13T14:25:48Z

Jenkins, retest this please

SparkQA · 2020-01-13T17:26:10Z

Test build #116645 has finished for PR 26955 at commit 3f9e96f.

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

srowen · 2020-01-17T15:46:35Z

Merged to master

tinhto-000 added 2 commits December 19, 2019 16:36

fix missing match case in SparkUncaughtExceptionHandler

f7c9956

added SparkUncaughtExceptionHandlerSuite

put new line at end of SparkUncaughtExceptionHandlerSuite.scala

5d671b0

seanli-rallyhealth reviewed Dec 19, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala Outdated Show resolved Hide resolved

seanli-rallyhealth reviewed Dec 19, 2019

View reviewed changes

core/src/test/scala/org/apache/spark/util/SparkUncaughtExceptionHandlerSuite.scala Outdated Show resolved Hide resolved

seanli-rallyhealth reviewed Dec 19, 2019

View reviewed changes

core/src/test/scala/org/apache/spark/util/SparkUncaughtExceptionHandlerSuite.scala Outdated Show resolved Hide resolved

use string interpolation in log messages

724e254

SparkUncaughtExceptionHandlerSuite had wrong name use toBoolean/toString for converting string to/from boolean exception throwers now check args.length before using args(0)

seanli-rallyhealth approved these changes Dec 23, 2019

View reviewed changes

HeartSaVioR reviewed Dec 27, 2019

View reviewed changes

core/src/test/scala/org/apache/spark/util/SparkUncaughtExceptionHandlerSuite.scala Outdated Show resolved Hide resolved

HeartSaVioR reviewed Dec 27, 2019

View reviewed changes

core/src/test/scala/org/apache/spark/util/SparkUncaughtExceptionHandlerSuite.scala Outdated Show resolved Hide resolved

HeartSaVioR reviewed Dec 27, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala Outdated Show resolved Hide resolved

tinhto-000 added 2 commits January 7, 2020 16:36

consolidated the thrower classes into one

48abea3

use Seq().foreach() to launch the test-cases

add try-catch around the second logError

c59f93c

HeartSaVioR approved these changes Jan 8, 2020

View reviewed changes

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala Outdated Show resolved Hide resolved

modify the try-block to non-oneliner.

b3da69d

srowen reviewed Jan 9, 2020

View reviewed changes

core/src/main/scala/org/apache/spark/util/SparkUncaughtExceptionHandler.scala Outdated Show resolved Hide resolved

pull catch statement up one line

3f9e96f

HeartSaVioR mentioned this pull request Jan 10, 2020

[MINOR][BUILD][PYSPARK] Disable test_memory_limit test temporarily #27158

Closed

HeartSaVioR mentioned this pull request Jan 10, 2020

[SPARK-30480][PYSPARK][TESTS] Fix 'test_memory_limit' on pyspark test #27162

Closed

srowen approved these changes Jan 16, 2020

View reviewed changes

srowen closed this in f5f05d5 Jan 17, 2020

n-marion mentioned this pull request Jan 29, 2020

[SPARK-30310][CORE][2.4] Resolve missing match case in SparkUncaughtExceptionHandler and added tests #27384

Closed

[SPARK-30310] [Core] Resolve missing match case in SparkUncaughtExceptionHandler and added tests #26955

[SPARK-30310] [Core] Resolve missing match case in SparkUncaughtExceptionHandler and added tests #26955

Uh oh!

Conversation

tinhto-000 commented Dec 19, 2019

What changes were proposed in this pull request?

Why are the changes needed?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Dec 24, 2019

Uh oh!

srowen commented Dec 26, 2019

Uh oh!

SparkQA commented Dec 26, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HeartSaVioR commented Dec 27, 2019

Uh oh!

srowen commented Dec 31, 2019

Uh oh!

HeartSaVioR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HeartSaVioR commented Jan 9, 2020

Uh oh!

srowen commented Jan 9, 2020

Uh oh!

SparkQA commented Jan 10, 2020

Uh oh!

srowen commented Jan 10, 2020

Uh oh!

HeartSaVioR commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyukjinKwon commented Jan 10, 2020

Uh oh!

HeartSaVioR commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jan 10, 2020

Uh oh!

HyukjinKwon commented Jan 10, 2020

Uh oh!

SparkQA commented Jan 10, 2020

Uh oh!

HeartSaVioR commented Jan 10, 2020

Uh oh!

srowen commented Jan 13, 2020

Uh oh!

SparkQA commented Jan 13, 2020

Uh oh!

srowen commented Jan 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

HeartSaVioR commented Jan 10, 2020 •

edited

Loading

HeartSaVioR commented Jan 10, 2020 •

edited

Loading