----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36664/#review92496 -----------------------------------------------------------
Thanks for looking into that. Exception handling was the most challenging part of rewriting SocketServer, so I'm glad to see more eyes on this implementation. I have a concern regarding the right way to handle an unexpected exceptions. core/src/main/scala/kafka/network/SocketServer.scala (line 400) <https://reviews.apache.org/r/36664/#comment146707> So in case of unexpected exception, we log an error and keep running? Isn't it better to kill the processor, since we don't know what's the state of the system? If the acceptor keeps placing messages in the queue for a dead processor, isn't it a separate issue? core/src/main/scala/kafka/network/SocketServer.scala (line 461) <https://reviews.apache.org/r/36664/#comment146708> Turns out that catching Throwable is a really bad idea: https://www.sumologic.com/2014/05/05/why-you-should-never-catch-throwable-in-scala/ - Gwen Shapira On July 21, 2015, 11:03 p.m., Jiangjie Qin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36664/ > ----------------------------------------------------------- > > (Updated July 21, 2015, 11:03 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2353 > https://issues.apache.org/jira/browse/KAFKA-2353 > > > Repository: kafka > > > Description > ------- > > Catch exception in kafka.network.Processor to avoid socket leak and exiting > unexpectedly. > > > Diffs > ----- > > core/src/main/scala/kafka/network/SocketServer.scala > 91319fa010b140cca632e5fa8050509bd2295fc9 > > Diff: https://reviews.apache.org/r/36664/diff/ > > > Testing > ------- > > > Thanks, > > Jiangjie Qin > >
