Re: Review Request 36368: Patch for KAFKA-1740
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/#review91184 --- Ship it! Ship It! - Onur Karaman On July 9, 2015, 8:23 p.m., Jason Gustafson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/ --- (Updated July 9, 2015, 8:23 p.m.) Review request for kafka. Bugs: KAFKA-1740 https://issues.apache.org/jira/browse/KAFKA-1740 Repository: kafka Description --- KAFKA-1740; heartbeat should return illegal generation if rebalancing Diffs - core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e Diff: https://reviews.apache.org/r/36368/diff/ Testing --- Thanks, Jason Gustafson
Re: Review Request 36368: Patch for KAFKA-1740
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/#review91187 --- Could you add some unit test for this fix? core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala (line 358) https://reviews.apache.org/r/36368/#comment144470 I feel it is helpful to add add the new members list in this info entry as well. I can do that upon checkin if people are OK with it? - Guozhang Wang On July 9, 2015, 8:23 p.m., Jason Gustafson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/ --- (Updated July 9, 2015, 8:23 p.m.) Review request for kafka. Bugs: KAFKA-1740 https://issues.apache.org/jira/browse/KAFKA-1740 Repository: kafka Description --- KAFKA-1740; heartbeat should return illegal generation if rebalancing Diffs - core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e Diff: https://reviews.apache.org/r/36368/diff/ Testing --- Thanks, Jason Gustafson
Re: Review Request 36368: Patch for KAFKA-1740
On July 9, 2015, 8:40 p.m., Guozhang Wang wrote: Could you add some unit test for this fix? Done. The test case depends on timing like the other response tests, but it should test what we want most of the time and succeed even if there is an unexpected delay in execution. On July 9, 2015, 8:40 p.m., Guozhang Wang wrote: core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala, line 358 https://reviews.apache.org/r/36368/diff/1/?file=1003888#file1003888line358 I feel it is helpful to add add the new members list in this info entry as well. I can do that upon checkin if people are OK with it? Sounds reasonable to me. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/#review91187 --- On July 9, 2015, 9:41 p.m., Jason Gustafson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/ --- (Updated July 9, 2015, 9:41 p.m.) Review request for kafka. Bugs: KAFKA-1740 https://issues.apache.org/jira/browse/KAFKA-1740 Repository: kafka Description --- KAFKA-1740; add unit test for heartbeat during rebalance Diffs - core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e core/src/test/scala/unit/kafka/coordinator/ConsumerCoordinatorResponseTest.scala 3cd726d291d0b1f9dd30d499c204e40961eb2c41 Diff: https://reviews.apache.org/r/36368/diff/ Testing --- Thanks, Jason Gustafson
Re: Review Request 36368: Patch for KAFKA-1740
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36368/ --- (Updated July 9, 2015, 9:41 p.m.) Review request for kafka. Bugs: KAFKA-1740 https://issues.apache.org/jira/browse/KAFKA-1740 Repository: kafka Description (updated) --- KAFKA-1740; add unit test for heartbeat during rebalance Diffs (updated) - core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 476973b2c551db5be3f1c54f94990f0dd15ff65e core/src/test/scala/unit/kafka/coordinator/ConsumerCoordinatorResponseTest.scala 3cd726d291d0b1f9dd30d499c204e40961eb2c41 Diff: https://reviews.apache.org/r/36368/diff/ Testing --- Thanks, Jason Gustafson
