Github user kayousterhout commented on a diff in the pull request:
https://github.com/apache/spark/pull/12775#discussion_r81837726
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/TaskResultGetter.scala ---
@@ -135,8 +135,9 @@ private[spark] class TaskResultGetter(sparkEnv:
SparkEnv, scheduler: TaskSchedul
logError(
"Could not deserialize TaskEndReason: ClassNotFound with
classloader " + loader)
case ex: Exception => // No-op
+ } finally {
--- End diff --
Usually it's a bad idea to catch all exceptions / errors, but in this case
I think it would be better to change line 137 to instead catch all remaining
error exceptions (so "case ex: _") and log them as an error (similar to line
135). I think this is less confusing to the user than the default
"logUncaughtExceptions" logging that will happen otherwise, and it doesn't make
any difference in terms of user functionality. If you do that too, I'd also
add a comment with something like "In general we avoid catching all throwables,
but in this case, the Runnable is about to complete anyway, so it's better to
catch remaining errors so that we can log a sensible message to the user."
@markhamstra does this seem reasonable?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
