Skip to content

Conversation

@n-marion
Copy link
Contributor

@n-marion n-marion commented Jan 29, 2020

What changes were proposed in this pull request?

Backport of SPARK-30310 from master (f5f05d5)

  1. 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.
  2. 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.
  3. 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.

…ionHandler and added tests

1) 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.
2) 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.
3) Added SparkUncaughtExceptionHandlerSuite.  Basically we create a Spark exception-throwing application with SparkUncaughtExceptionHandler and then check its exit code.

SPARK-30310, because the process would halt unexpectedly.

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

Closes #26955 from tinhto-000/uncaught_exception_fix.

Authored-by: Tin Hang To <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-30310][CORE] Resolve missing match case in SparkUncaughtExcept… [SPARK-30310][CORE][2.4] Resolve missing match case in SparkUncaughtExceptionHandler and added tests Jan 29, 2020
@dongjoon-hyun
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Jan 29, 2020

Test build #117520 has finished for PR 27384 at commit 360623b.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @n-marion .
Merged to branch-2.4.

dongjoon-hyun pushed a commit that referenced this pull request Jan 29, 2020
…xceptionHandler and added tests

### What changes were proposed in this pull request?
Backport of SPARK-30310 from master (f5f05d5)

1) 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.
2) 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.
3) 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.

Closes #27384 from n-marion/branch-2.4_30310-backport.

Authored-by: git <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants