> On June 23, 2015, 4:20 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java, > > line 355 > > <https://reviews.apache.org/r/34789/diff/12/?file=990006#file990006line355> > > > > What's the logic to initiate connection to coordinator if the > > coordinator is not available during HB?
As it's currently written, we'd skip a heartbeat if we don't have an active connection to the coordinator. As long as the heartbeat frequency is 3 or more times per session timeout, this is probably ok, but we might want to handle it better if we end up exposing the heartbeat frequency in configuration (currently it's hard-coded). Perhaps we can fix this in a separate ticket? > On June 23, 2015, 4:20 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > lines 351-354 > > <https://reviews.apache.org/r/34789/diff/12/?file=990003#file990003line351> > > > > These seem redundant give the code below. I think it's still necessary to call wakeup to abort a long poll if you want to ensure timely shutdown. You could probably get away without the closed flag and just use the ConsumerWakeupException to close the consumer, but the explicit flag seems cleaner. > On June 23, 2015, 4:20 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 436 > > <https://reviews.apache.org/r/34789/diff/12/?file=990003#file990003line436> > > > > Should this be volatile so that different threads can see the latest > > value of refcount? I think you are right. Fixed in latest patch. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34789/#review88914 ----------------------------------------------------------- On June 23, 2015, 4:39 p.m., Jason Gustafson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34789/ > ----------------------------------------------------------- > > (Updated June 23, 2015, 4:39 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2168 > https://issues.apache.org/jira/browse/KAFKA-2168 > > > Repository: kafka > > > Description > ------- > > KAFKA-2168; make refcount in KafkaConsumer an AtomicInteger > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java > 9be8fbc648369ad9db1a7eea94bc1b9edbfdbfd7 > > Diff: https://reviews.apache.org/r/34789/diff/ > > > Testing > ------- > > > Thanks, > > Jason Gustafson > >
