> 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(); > ``` > > Dong Lin wrote: > 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?
True - that won't work. Another option may be to change `connect` to throw a `afkaException` if `isClosed` is true. Your latest patch may be better though since that avoids modification of the existing API (and only adds to it) - although I think naming it `interruptConsumer` may be better. The javadoc can clearly state that it actually disconnects the consumer due to the JVM bug (and link to the stackoverflow question). - Joel ----------------------------------------------------------- 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 > >
