> On July 22, 2015, 10:40 a.m., Ismael Juma wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 467 > > <https://reviews.apache.org/r/36664/diff/2/?file=1018375#file1018375line467> > > > > As far as I can see `ClosedChannelException`, `IllegalStateException` > > and `IllegalArgumentException` are enough? Also, you would it be better to > > use `IOException` instead of `ClosedChannelException`? > > > > What happens if other exceptions are thrown? Will we still have a > > socket leak? > > Gwen Shapira wrote: > Yeah, perhaps in addition to listing the expected cases, we should also > handle nonFatal(e)? > (https://tersesystems.com/2012/12/27/error-handling-in-scala/) > > Ismael Juma wrote: > If we handle `NonFatal(e)`, it may be that we don't want to list all the > other exceptions as it's just boilerplate.
OK, this is how it was done in the end. Good. :) - Ismael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36664/#review92574 ----------------------------------------------------------- On July 23, 2015, 12:51 a.m., Jiangjie Qin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36664/ > ----------------------------------------------------------- > > (Updated July 23, 2015, 12:51 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2353 > https://issues.apache.org/jira/browse/KAFKA-2353 > > > Repository: kafka > > > Description > ------- > > Addressed Gwen's comments > > > Addressed Gwen's comments. > > > Diffs > ----- > > core/src/main/scala/kafka/network/SocketServer.scala > 91319fa010b140cca632e5fa8050509bd2295fc9 > > Diff: https://reviews.apache.org/r/36664/diff/ > > > Testing > ------- > > > Thanks, > > Jiangjie Qin > >
