[
https://issues.apache.org/jira/browse/SAMZA-178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13943201#comment-13943201
]
Chris Riccomini commented on SAMZA-178:
---------------------------------------
1. For SamzaContainer, let's catch exception there, too. I'm afraid catching
OOME and trying to shut stuff down, which is what the container does, will
cause the process to hang still.
2. Good catch on CheckpointSerde. Not sure why we weren't logging that.
+1 Everything else looks good. I think we're good to commit if you make the
change for (1) above.
> Review catching of Throwable
> ----------------------------
>
> Key: SAMZA-178
> URL: https://issues.apache.org/jira/browse/SAMZA-178
> Project: Samza
> Issue Type: Task
> Reporter: Martin Kleppmann
> Attachments: SAMZA-178.patch
>
>
> We have various places in the code where we do:
> {noformat}
> try { ... } catch { case _ => ... }
> try { ... } catch { case e: Throwable => ... }
> {noformat}
> That's potentially dangerous, because Throwable is the supertype for various
> things that really shouldn't be caught by normal code, such as
> java.lang.OutOfMemoryError or scala.runtime.NonLocalReturnControl (the latter
> is used internally by Scala for flow control).
> We should probably change most or all of those occurrences to catch Exception
> instead. But it's worth reviewing each case to make sure that it really
> doesn't need to catch Throwable after all.
> See also discussion on https://reviews.apache.org/r/18606/ (SAMZA-161).
--
This message was sent by Atlassian JIRA
(v6.2#6252)
