> On June 23, 2015, 5:59 p.m., Jason Gustafson wrote: > > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line > > 282 > > <https://reviews.apache.org/r/35791/diff/1/?file=990592#file990592line282> > > > > Do you think we should just move the disconnected.add() into the close > > method? > > Dong Lin wrote: > I have thought about it as well. But probabaly no. Because in > Selector.send() we put failed destinationId is put in failedSends rather than > disconnected. The reason we use failedSends is because send() and poll() in > Selector will be called asynchronously by different threads. > > Jason Gustafson wrote: > Yeah, that makes sense. I wonder if we should be using a Set internally > instead of a List? Then we wouldn't need to worry about adding to > disconnected multiple times. Guess there might be some performance impact, > but we would have an easier time ensuring correctness. > > Dong Lin wrote: > Yeah that can be useful. I am not sure if currently some id may be added > to disconnected multiple times. Even it does, this should be not a problme in > NetworkClient.handleDisconnections(). I personally prefer to keep existing > code unless there is good reason (i.e. performance or functionality) for > change. Not sure what others think about this change.
Seems unnecessary now that I think about it. In fact, it wouldn't hurt to do the disconnected.add() into close() as long as we also add it to failedSends, but that might make the code tougher to follow. Anyway, I think the patch is fine as it is. I was just wondering if there was an easy way to prevent this issue from popping up again in the future. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35791/#review89010 ----------------------------------------------------------- On June 23, 2015, 5:41 p.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35791/ > ----------------------------------------------------------- > > (Updated June 23, 2015, 5:41 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2298 > https://issues.apache.org/jira/browse/KAFKA-2298 > > > Repository: kafka > > > Description > ------- > > KAFKA-2290; Client Selector can drop connections on InvalidReceiveException > without notifying NetworkClient > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 4aee214b24fd990be003adc36d675f015bf22fe6 > > Diff: https://reviews.apache.org/r/35791/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
