You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com> on 2015/10/01 21:44:04 UTC
Re: Review Request 37604: SAMZA-760 Samza Container should catch
Throwables instead of just catching Exceptions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37604/#review101272
-----------------------------------------------------------
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (line 582)
<https://reviews.apache.org/r/37604/#comment158652>
The bug description states that making the Throwable as a cause of the SamzaException, I would perfer to use SamzaException(String s, Throwable t) s.t. more detailed cause info would be print out.
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (line 599)
<https://reviews.apache.org/r/37604/#comment158651>
Forgive me on ignorance on this ControlThrowable. Why should we skip logging this one? You mentioned in the RB description that there are some nuance involved by catching all Throwables. Could you elaborate a bit more here?
Thanks!
- Yi Pan (Data Infrastructure)
On Aug. 25, 2015, 11:48 a.m., Aleksandar Bircakovic wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37604/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 11:48 a.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> Added a catch for Throwables in Samza container. Catching Throwables can cause problems in specific situations so I also added a partial function 'safely' that should take care of that specific situations.
>
>
> Diffs
> -----
>
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 85b012b
> samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 4db6d5c
>
> Diff: https://reviews.apache.org/r/37604/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Aleksandar Bircakovic
>
>
Re: Review Request 37604: SAMZA-760 Samza Container should catch
Throwables instead of just catching Exceptions
Posted by Aleksandar Bircakovic <a....@levi9.com>.
> On Oct. 1, 2015, 7:44 p.m., Yi Pan (Data Infrastructure) wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, line 599
> > <https://reviews.apache.org/r/37604/diff/2/?file=1051095#file1051095line599>
> >
> > Forgive me on ignorance on this ControlThrowable. Why should we skip logging this one? You mentioned in the RB description that there are some nuance involved by catching all Throwables. Could you elaborate a bit more here?
> >
> > Thanks!
Thank you for review. I found some articles saying that catching Throwables in Scala isn't so wise (like this one https://www.sumologic.com/2014/05/05/why-you-should-never-catch-throwable-in-scala/). They say it can have negative impact on JVM.
> On Oct. 1, 2015, 7:44 p.m., Yi Pan (Data Infrastructure) wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, line 582
> > <https://reviews.apache.org/r/37604/diff/2/?file=1051095#file1051095line582>
> >
> > The bug description states that making the Throwable as a cause of the SamzaException, I would perfer to use SamzaException(String s, Throwable t) s.t. more detailed cause info would be print out.
Agree with that.
- Aleksandar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37604/#review101272
-----------------------------------------------------------
On Aug. 25, 2015, 11:48 a.m., Aleksandar Bircakovic wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37604/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2015, 11:48 a.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> Added a catch for Throwables in Samza container. Catching Throwables can cause problems in specific situations so I also added a partial function 'safely' that should take care of that specific situations.
>
>
> Diffs
> -----
>
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 85b012b
> samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 4db6d5c
>
> Diff: https://reviews.apache.org/r/37604/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Aleksandar Bircakovic
>
>