[SPARK-48325][CORE] Always specify messages in ExecutorRunner.killProcess #46641
Conversation
There was a problem hiding this comment.
Looks reasonable to me, but I will let @Ngone51 or @jiangxb1987 take a look at it.
| case e: Exception => | ||
| logError("Error running executor", e) | ||
| state = ExecutorState.FAILED | ||
| killProcess(Some(e.toString)) | ||
| killProcess(s"Error running executor: $e") |
There was a problem hiding this comment.
Could you dump the full stacktrace for the error message?
There was a problem hiding this comment.
The message will be sent to Master and Driver. Do you think including the full stacktrace might make it too large? How about logging it in the Worker log here?
There was a problem hiding this comment.
logError("Error running executor", e)
This ^ should already log the full stack in the worker log.
My concern is that the exception that throw here could be rare case and full stack will help the debug. Since worker already logs it, I'm fine to return a short error message to master.
|
Merged to master for Apache Spark 4.0.0. Thank you, @bozhang2820 and all. |
What changes were proposed in this pull request?
This change is to always specify the message in
ExecutorRunner.killProcess.Why are the changes needed?
This is to get the occurrence rate for different cases when killing the executor process, in order to analyze executor running stability.
Does this PR introduce any user-facing change?
No
How was this patch tested?
N/A
Was this patch authored or co-authored using generative AI tooling?
No