samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aleksandar Bircakovic" <a.bircako...@levi9.com>
Subject Re: Review Request 37604: SAMZA-760 Samza Container should catch Throwables instead of just catching Exceptions
Date Tue, 06 Oct 2015 14:04:12 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message