> On July 9, 2015, 7:19 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, line 76 > > <https://reviews.apache.org/r/34965/diff/2/?file=977751#file977751line76> > > > > You could get around the above by retaining this call to > > simpleConsumer.close (although it would be mostly redundant). However this > > is still not ideal, since it is a caveat that the user of the (public) > > forceClose API needs to be aware of. > > Dong Lin wrote: > I agree. I have updated the code and comments to hopefully avoid any > confusion to user. > > Joel Koshy wrote: > Would it work to just modify what you had before in `forceClose` to: > ``` > disconnect(); > close(); > ```
I think that won't work. The event sequence you described will still cause problem. The following sequence of events may happen: - the forceClose() as well as close() is executed by thread 1 - thread 2 calls sendRequest(). blockingChannel.send(request) will throw ClosedChannelException which triggers reconnect(). It is possible to make this work by changing the way sendRequest() handles ClosedChannelException. But I find the API in the second patch is better. Which solution do you prefer? - Dong ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34965/#review91159 ----------------------------------------------------------- On July 9, 2015, 10:35 p.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34965/ > ----------------------------------------------------------- > > (Updated July 9, 2015, 10:35 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2241 > https://issues.apache.org/jira/browse/KAFKA-2241 > > > Repository: kafka > > > Description > ------- > > KAFKA-2241; AbstractFetcherThread.shutdown() should not block on > ReadableByteChannel.read(buffer) > > > Diffs > ----- > > core/src/main/scala/kafka/consumer/SimpleConsumer.scala > c16f7edd322709060e54c77eb505c44cbd77a4ec > core/src/main/scala/kafka/server/AbstractFetcherThread.scala > 83fc47417dd7fe4edf030217fa7fd69d99b170b0 > > Diff: https://reviews.apache.org/r/34965/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
