Re: Review Request 36652: Patch for KAFKA-2351

2015-08-25 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review96463 --- Ship it! Ship It! - Joel Koshy On Aug. 24, 2015, 10:50 p.m., May

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-24 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/ --- (Updated Aug. 24, 2015, 10:50 p.m.) Review request for kafka. Bugs: KAFKA-235

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-21 Thread Mayuresh Gharat
> On Aug. 21, 2015, 6:14 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 264 > > > > > > Yes that's exactly what we need. Any reason why we shouldn't use > > `NonFatal` as is?

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-21 Thread Ismael Juma
> On Aug. 21, 2015, 6:14 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 264 > > > > > > Yes that's exactly what we need. Any reason why we shouldn't use > > `NonFatal` as is?

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-21 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review96069 --- core/src/main/scala/kafka/network/SocketServer.scala (line 264)

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-21 Thread Ismael Juma
> On Aug. 21, 2015, 5:57 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 264 > > > > > > Thanks for the reference - we currently have this pattern all over the > > place. We ca

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-21 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review96061 --- core/src/main/scala/kafka/network/SocketServer.scala (line 264)

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-19 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review95868 --- core/src/main/scala/kafka/network/SocketServer.scala (line 264)

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-19 Thread Mayuresh Gharat
> On Aug. 19, 2015, 5:48 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 265 > > > > > > I'm also unclear at this point on what the right thing to do here would > > be - i.e.,

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-18 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review95824 --- Patch needs a rebase. core/src/main/scala/kafka/network/SocketServ

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-13 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/ --- (Updated Aug. 13, 2015, 8:10 p.m.) Review request for kafka. Bugs: KAFKA-2351

Re: Review Request 36652: Patch for KAFKA-2351

2015-08-05 Thread Jun Rao
> On July 24, 2015, 5:01 p.m., Mayuresh Gharat wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 264 > > > > > > Hi Jun, > > Cleaning up in finally is actually nice. I will make the necessary

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-27 Thread Jiangjie Qin
> On July 24, 2015, 4:13 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 264 > > > > > > Not sure if it's better to keep the thread alive on any throwable. For > > unexpected exce

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-24 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review92928 --- core/src/main/scala/kafka/network/SocketServer.scala (line 264)

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-24 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review92920 --- Thanks for the patch. A few comments below. core/src/main/scala/ka

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-23 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/ --- (Updated July 24, 2015, 4:36 a.m.) Review request for kafka. Bugs: KAFKA-2351

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review92488 --- Ship it! Latest patch looks good to me. - Jiangjie Qin On July 2

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/ --- (Updated July 21, 2015, 9:58 p.m.) Review request for kafka. Bugs: KAFKA-2351

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Grant Henke
> On July 21, 2015, 8:26 p.m., Grant Henke wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 266 > > > > > > What errors were seen that should be caught here? Can we catch a more > > specific exc

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Mayuresh Gharat
> On July 21, 2015, 8:26 p.m., Grant Henke wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 266 > > > > > > What errors were seen that should be caught here? Can we catch a more > > specific exc

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Grant Henke
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review92470 --- core/src/main/scala/kafka/network/SocketServer.scala (line 266)

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Mayuresh Gharat
On July 21, 2015, 8:18 p.m., Mayuresh Gharat wrote: > > T Yes. Got it, I thought that we should be catching all exceptions and exit. But doing the above will catch the exception and exit when its shutting down and thats the only thing that this ticket considers. - Mayuresh

Re: Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/#review92465 --- Thanks for the patch, some comments. core/src/main/scala/kafka/net

Review Request 36652: Patch for KAFKA-2351

2015-07-21 Thread Mayuresh Gharat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36652/ --- Review request for kafka. Bugs: KAFKA-2351 https://issues.apache.org/jira/b