You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jason Gustafson <ja...@confluent.io> on 2015/07/09 22:23:09 UTC
Review Request 36368: Patch for KAFKA-1740
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36368/
-----------------------------------------------------------
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
Posted by Jason Gustafson <ja...@confluent.io>.
-----------------------------------------------------------
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
Re: Review Request 36368: Patch for KAFKA-1740
Posted by Jason Gustafson <ja...@confluent.io>.
> 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
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
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
Posted by Onur Karaman <ok...@linkedin.com>.
-----------------------------------------------------------
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
>
>